Re: [PATCH] writeback: skip new or to-be-freed inodes

[Wu Fengguang <fengguang.wu@intel.com>]

Hi Artem,

On Mon, Jun 08, 2009 at 03:03:10PM +0800, Artem Bityutskiy wrote:
> Wu Fengguang wrote:
> > The above race and warning didn't turn up because writeback_inodes() holds
> > the s_umount lock, so generic_forget_inode() finds MS_ACTIVE and returns
> > early. But we are not sure the UBIFS calls and future callers will guarantee
> > that. So skip I_WILL_FREE inodes for the sake of safety.
>
> The inode states are a bit vague for me, but vs. UBIFS - feel
> free to ask questions.

Thank you. Basically I'm not sure if UBIFS guarantees it won't be
unmounted (hence the MS_ACTIVE bit is on) when calling
generic_sync_sb_inodes() in shrink_liability() and ubifs_sync_fs().

Thanks,
Fengguang

PS: our previous discussions

        > > Another possibility:
        > >
        > > generic_forget_inode
        > >   inode->i_state |= I_WILL_FREE;
        > >   spin_unlock(&inode_lock);
        > >                                                 generic_sync_sb_inodes()
        > >                                                   spin_lock(&inode_lock);
        > >                                                   __iget(inode);
        > >                                                   __writeback_single_inode
        > >                                                     // see non zero i_count
        > >                                                     WARN_ON(inode->i_state & I_WILL_FREE);
        > >
        > > I'm wondering why didn't we saw reports on the last WARN_ON()?
        > > Did we missed something?
        >   I meant the above race in my description ;-). Anyway, the race can happen
        > only if we are unmounting the filesystem (normally, we bail out on
        > sb->s_flags & MS_ACTIVE check - yes, it's a bit hidden and it also took me
        > a while to understand why we weren't seeing tons of warnings...).

        Ah OK. Just checked that all three callers of generic_sync_sb_inodes():
        - writeback_inodes(): umount prevented
        - pohmelfs_kill_super(): just before umount
        - ubifs calls: too complex to be obvious..
        At least the first two cases are safe, so we didn't see the error report ;)