Messages in this thread Patch in this message | | | Date | Tue, 21 Nov 2006 17:23:59 -0800 | From | Suzuki <> | Subject | Re: [BUG] Reiserfs: reiserfs_panic while running fs stress tests |
| |
Vladimir V. Saveliev wrote: > Hello > > On Fri, 2006-05-12 at 12:15 +0530, Suzuki wrote: > >>Randy.Dunlap wrote: >> >>>On Fri, 05 May 2006 10:22:21 +0530 Suzuki wrote: >>> >>> >>> >>>>Hi, >>>> >>>> >>>>I was working on a reiserfs panic with 2.6.17-rc3, while running fs >>>>stress tests. >>> >>> >>>Hi, >>>What test(s) do you use? >> >>The problem was initially hit while running the following tests >>simultaneously.. >>IOZone, bonnie++, dbench, fs_inod, fs_maim, fsstress, fsx_linux, >>postmark, tiobench. >> >>As I had mentioned in my post, I have a simple testcase to trigger the >>panic which can hit the code path explained below. >> >>The root cause of the problem is (as mentioned in the earlier post): >> >> Whenever there is an extending DIO write operation, the fs would >>create a safe link so as to ensure the file size consistent, if there is >>crash in between the DIO. This will be deleted once the write operation >>finishes. >> > > > I am not sure why safe link is needed for write. Maybe one who added > that still remembers why that was done and can explain, please? > > Suzuki, would you please try the attached patch?
We had no luck in reproducing the issue until recently. This was again hit with 2.6.18-rc7. The patch (with appropriate changes to match the current) has been verified to resolve the issue. Attached here is the patch which fits the current level.
Thanks,
Suzuki > > > > > ------------------------------------------------------------------------ > > > diff -puN fs/reiserfs/file.c~reiserfs-dont-use-safelink-on-write fs/reiserfs/file.c > --- linux-2.6.17-rc3/fs/reiserfs/file.c~reiserfs-dont-use-safelink-on-write 2006-05-12 11:28:16.000000000 +0400 > +++ linux-2.6.17-rc3-vs/fs/reiserfs/file.c 2006-05-12 11:29:16.000000000 +0400 > @@ -1305,54 +1305,7 @@ static ssize_t reiserfs_file_write(struc > } > > if (file->f_flags & O_DIRECT) { // Direct IO needs treatment > - ssize_t result, after_file_end = 0; > - if ((*ppos + count >= inode->i_size) > - || (file->f_flags & O_APPEND)) { > - /* If we are appending a file, we need to put this savelink in here. > - If we will crash while doing direct io, finish_unfinished will > - cut the garbage from the file end. */ > - reiserfs_write_lock(inode->i_sb); > - err = > - journal_begin(&th, inode->i_sb, > - JOURNAL_PER_BALANCE_CNT); > - if (err) { > - reiserfs_write_unlock(inode->i_sb); > - return err; > - } > - reiserfs_update_inode_transaction(inode); > - add_save_link(&th, inode, 1 /* Truncate */ ); > - after_file_end = 1; > - err = > - journal_end(&th, inode->i_sb, > - JOURNAL_PER_BALANCE_CNT); > - reiserfs_write_unlock(inode->i_sb); > - if (err) > - return err; > - } > - result = generic_file_write(file, buf, count, ppos); > - > - if (after_file_end) { /* Now update i_size and remove the savelink */ > - struct reiserfs_transaction_handle th; > - reiserfs_write_lock(inode->i_sb); > - err = journal_begin(&th, inode->i_sb, 1); > - if (err) { > - reiserfs_write_unlock(inode->i_sb); > - return err; > - } > - reiserfs_update_inode_transaction(inode); > - mark_inode_dirty(inode); > - err = journal_end(&th, inode->i_sb, 1); > - if (err) { > - reiserfs_write_unlock(inode->i_sb); > - return err; > - } > - err = remove_save_link(inode, 1 /* truncate */ ); > - reiserfs_write_unlock(inode->i_sb); > - if (err) > - return err; > - } > - > - return result; > + return generic_file_write(file, buf, count, ppos); > } > > if (unlikely((ssize_t) count < 0)) > > _
Index: linux-2.6.19-rc1/fs/reiserfs/file.c =================================================================== --- linux-2.6.19-rc1.orig/fs/reiserfs/file.c 2006-10-10 05:54:30.000000000 -0700 +++ linux-2.6.19-rc1/fs/reiserfs/file.c 2006-11-21 17:17:36.000000000 -0800 @@ -1306,56 +1306,8 @@ count = MAX_NON_LFS - (unsigned long)*ppos; } - if (file->f_flags & O_DIRECT) { // Direct IO needs treatment - ssize_t result, after_file_end = 0; - if ((*ppos + count >= inode->i_size) - || (file->f_flags & O_APPEND)) { - /* If we are appending a file, we need to put this savelink in here. - If we will crash while doing direct io, finish_unfinished will - cut the garbage from the file end. */ - reiserfs_write_lock(inode->i_sb); - err = - journal_begin(&th, inode->i_sb, - JOURNAL_PER_BALANCE_CNT); - if (err) { - reiserfs_write_unlock(inode->i_sb); - return err; - } - reiserfs_update_inode_transaction(inode); - add_save_link(&th, inode, 1 /* Truncate */ ); - after_file_end = 1; - err = - journal_end(&th, inode->i_sb, - JOURNAL_PER_BALANCE_CNT); - reiserfs_write_unlock(inode->i_sb); - if (err) - return err; - } - result = do_sync_write(file, buf, count, ppos); - - if (after_file_end) { /* Now update i_size and remove the savelink */ - struct reiserfs_transaction_handle th; - reiserfs_write_lock(inode->i_sb); - err = journal_begin(&th, inode->i_sb, 1); - if (err) { - reiserfs_write_unlock(inode->i_sb); - return err; - } - reiserfs_update_inode_transaction(inode); - mark_inode_dirty(inode); - err = journal_end(&th, inode->i_sb, 1); - if (err) { - reiserfs_write_unlock(inode->i_sb); - return err; - } - err = remove_save_link(inode, 1 /* truncate */ ); - reiserfs_write_unlock(inode->i_sb); - if (err) - return err; - } - - return result; - } + if (file->f_flags & O_DIRECT) + return do_sync_write(file, buf, count, ppos); if (unlikely((ssize_t) count < 0)) return -EINVAL; | |