lkml.org 
[lkml]   [2006]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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;
\
 
 \ /
  Last update: 2006-11-22 02:29    [W:0.094 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site