Re: [PATCH] VFS bugfix: two read_inode() calles without clear_inode() call between

[Andrew Morton <akpm@osdl.org>]

David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Wed, 2005-05-04 at 13:04 -0700, Andrew Morton wrote:
> > This sounds more like a bug in the iget() caller to me.
> > 
> > Question is: if the inode has zero refcount and is unhashed then how
> > did the caller get its sticky paws onto the inode* in the first place?
> > 
> > If the caller had saved a copy of the inode* in local storage then the
> > caller should have taken a ref against the inode.
> > 
> > If the caller had just looked up the inode via hastable lookup via
> > iget_whatever() then again the caller will have a ref on the inode.
> > 
> > So.  Please tell us more about how the caller got into this situation.
> 
> I could explain in detail how JFFS2 garbage collection works, moving log
> entries out of the way by calling iget() on the inode to which they
> belong.... or I could just say "NFS".
> 

That doesn't really settle the question of whether the callers are broken,
whether they are doing something which the VFS really should support and
what need to be done to the VFS to support it properly.

Looking at the proposed patch: what happens if an inode is on its way to
dispose_list() and someone tries to do an iget() on it?  I don't think I_LOCK
is set, so __wait_on_freeing_inode() will no longer provide this guarantee:

/*
 * If we try to find an inode in the inode hash while it is being deleted, we
 * have to wait until the filesystem completes its deletion before reporting
 * that it isn't found.  This is because iget will immediately call
 * ->read_inode, and we want to be sure that evidence of the deletion is found
 * by ->read_inode.

Not only will we break the __wait_on_freeing_inode() guarantee, but we'll
break it in extremely rare circumstances.

And that's just from a quick peek.  There may be other problems.  Worried.