Re: RFC: Bug in generic_forget_inode() ?

[Russ Weight <rweight@us.ibm.com>]

On Fri, 2005-03-18 at 14:17 -0800, Andrew Morton wrote:
> Russ Weight <rweight@us.ibm.com> wrote:
> >
> > generic_forget_inode() is eventually called (within the context of
> >  iput), the inode is placed on the unused list, and the inode_lock is
> >  dropped.
> > 
> >  kswapd calls prune_icache(), locks the inode_lock, and pulls the same
> >  inode off of the unused list. Upon completion, prune_icache() calls
> >  dispose_list() for the inodes that it has collected.
> > 
> >  generic_forget_inode() calls write_inode_now(), which calls
> >  __writeback_single_inode() which calls __sync_single_inode().
> >  __sync_single_inode() panics when attempting to move the inode onto the
> >  unused list (the last call to list_move). This is due to the poison
> >  values that were previously loaded into the next and prev list pointers
> >  by list_del().
> 
> It's not clear what the actual bug is here.  When you say that
> __sync_single_inode() panics over the list pointers, who was it that
> poisoned them?  dispose_list()?
> 
> Certainly isofs_fill_super() could trivially be rewritten to not do the
> iget()/iput() but we should be sure that that's really the bug.  The inode
> lifetime management is rather messy, I'm afraid.

The pointers are poisoned by dispose_list(). The race condition is
between prune_icache() and generic_forget_inode().

When I suggested that a change to isofs_fill_super() might be
considered, I was speculating that isofs_fill_super() might be creating
an unexpected state by doing something unconventional in its usage of
iget() and iput(). This is something I had not investigated.

The problem is more likely in generic_forget_inode(). It releases the
inode_lock and then locks it again without doing anything to prevent the
inode from being stolen, and without checking to see if it has been
stolen. Likewise, write_inode_now() does not do any checks to see if the
inode has been stolen.

- Russ