2010-08-17 08:26:22

by Marin Mitov

Subject: [BUG][reiserfs] page fault during kernel boot

Hi all,

The function: reiserfs_evict_inode() ends with:

<snip>
out:
end_writeback(inode); /* note this must go after the journal_end to prevent deadlock */
dquot_drop(inode);
inode->i_blocks = 0;
reiserfs_write_unlock_once(inode->i_sb, depth);

no_delete:
end_writeback(inode);
dquot_drop(inode);
}
<snip>

When goto out path is taken,

end_writeback(inode);
dquot_drop(inode);

are executed twice, leading to page fault (in my case) during the kernel boot.

Add return; before no_delete label (but I am not quite sure that this is correct :-).

Signed-off-by: Marin Mitov <[email protected]>

====================================================================
--- a/fs/reiserfs/inode.c 2010-08-17 09:51:27.000000000 +0300
+++ b/fs/reiserfs/inode.c 2010-08-17 10:45:20.000000000 +0300
@@ -78,11 +78,12 @@ void reiserfs_evict_inode(struct inode *
/* no object items are in the tree */
;
}
- out:
+out:
end_writeback(inode); /* note this must go after the journal_end to prevent deadlock */
dquot_drop(inode);
inode->i_blocks = 0;
reiserfs_write_unlock_once(inode->i_sb, depth);
+ return;

no_delete:
end_writeback(inode);

2010-08-17 22:20:25

by Frederic Weisbecker

Subject: Re: [BUG][reiserfs] page fault during kernel boot

On Tue, Aug 17, 2010 at 11:25:25AM +0300, Marin Mitov wrote:
> Hi all,
>
> The function: reiserfs_evict_inode() ends with:
>
> <snip>
> out:
> end_writeback(inode); /* note this must go after the journal_end to prevent deadlock */
> dquot_drop(inode);
> inode->i_blocks = 0;
> reiserfs_write_unlock_once(inode->i_sb, depth);
>
> no_delete:
> end_writeback(inode);
> dquot_drop(inode);
> }
> <snip>
>
> When goto out path is taken,
>
> end_writeback(inode);
> dquot_drop(inode);
>
> are executed twice, leading to page fault (in my case) during the kernel boot.


Indeed. More precisely it triggers a BUG in end_writeback():

BUG_ON(inode->i_state & I_CLEAR);

that because we call it twice.


> Add return; before no_delete label (but I am not quite sure that this is correct :-).



That looks correct.

Also Andrew Benton reported this issue and tested almost
the same patch and it seemed to solve the issue.

Thanks.



> Signed-off-by: Marin Mitov <[email protected]>
>
> ====================================================================
> --- a/fs/reiserfs/inode.c 2010-08-17 09:51:27.000000000 +0300
> +++ b/fs/reiserfs/inode.c 2010-08-17 10:45:20.000000000 +0300
> @@ -78,11 +78,12 @@ void reiserfs_evict_inode(struct inode *
> /* no object items are in the tree */
> ;
> }
> - out:
> +out:
> end_writeback(inode); /* note this must go after the journal_end to prevent deadlock */
> dquot_drop(inode);
> inode->i_blocks = 0;
> reiserfs_write_unlock_once(inode->i_sb, depth);
> + return;
>
> no_delete:
> end_writeback(inode);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2010-08-17 22:30:38

by Frederic Weisbecker

Subject: Re: [BUG][reiserfs] page fault during kernel boot

On Tue, Aug 17, 2010 at 11:25:25AM +0300, Marin Mitov wrote:
> Hi all,
>
> The function: reiserfs_evict_inode() ends with:
>
> <snip>
> out:
> end_writeback(inode); /* note this must go after the journal_end to prevent deadlock */
> dquot_drop(inode);
> inode->i_blocks = 0;
> reiserfs_write_unlock_once(inode->i_sb, depth);
>
> no_delete:
> end_writeback(inode);
> dquot_drop(inode);
> }
> <snip>
>
> When goto out path is taken,
>
> end_writeback(inode);
> dquot_drop(inode);
>
> are executed twice, leading to page fault (in my case) during the kernel boot.
>
> Add return; before no_delete label (but I am not quite sure that this is correct :-).
>
> Signed-off-by: Marin Mitov <[email protected]>


In fact the sam patch has been submitted and applied to the vfs tree already:

http://lkml.org/lkml/2010/8/11/98

The patch will probably reach mainline soon.

Thanks.