lkml.org 
[lkml]   [2005]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: RFC: Bug in generic_forget_inode() ?
From
Date
On Fri, 2005-03-18 at 16:11 -0800, Andrew Morton wrote:
> Russ Weight <rweight@us.ibm.com> wrote:
> >
> > 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.
> >
>
> I dunno. This is really fiddly code and does like to blow up in your face.
> I doubt if the problem is in such a well-trodden path as
> generic_forget_inode().
>
> Perhaps isofs is breaking the rules by running iget() prior to setting
> MS_ACTIVE. (What rules, you ask? hah.)
>

isofs is, in fact, running iget() prior to setting MS_ACTIVE. I have
tested a fix for this and I will post a patch shortly.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-04-06 13:31    [W:0.035 / U:35.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site