[Top] [All Lists]

Re: [PATCH 3/3] free inodes using destroy_inode

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 3/3] free inodes using destroy_inode
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 22 Oct 2008 12:28:31 -0400
In-reply-to: <20081021210633.GM25906@disturbed>
References: <20081020222044.GC23662@xxxxxx> <20081021030726.GD18495@disturbed> <20081021090708.GA30463@xxxxxxxxxxxxx> <20081021210633.GM25906@disturbed>
User-agent: Mutt/1.5.18 (2008-05-17)
On Wed, Oct 22, 2008 at 08:06:33AM +1100, Dave Chinner wrote:
> Hmmmm - I still don't see that as possible. We don't set I_NEW until
> we are inside xfs_setup_inode(), which occurs after we insert
> the inode into the radix tree. xfs_setup_inode() also calls
> unlock_new_inode(), so the I_NEW flag is cleared before it returns,
> too. So I can't really see how this check in reclaim does anything....
> AFAICT, once we've inserted the new inode into the radix tree,
> we can't get an error before xfs_setup_inode() is called - even
> in the allocation case. Hence once we're in the radix tree,
> xfs_iput_new() should be called to cleanup.
> All the cases that xfs_destroy_inode() handles are before the inode
> is inserted into the radix tree, so marking the XFS inode XFS_IBAD
> in xfs_destroy_inode() is probably a much more reliable way to
> detect immediate destroy cases in the reclaim code than relying
> on I_NEW......

I've put in some instrumentation and we indeed newer hit the I_NEW
case.  I also can't reproduce the problems I saw before anymore -
in fact calling radix_tree_remove on an inode is explicitly fine
per documentation and does work.  I'll send a  reworked patch that
just uses make_inode_bad as this has passed xfsqa a few times now.

I've also removed patch 2 for now a it's not a bug fix an I'll put
it into my later series that cleans up a lot of mess in that area.

<Prev in Thread] Current Thread [Next in Thread>