Re: [patch] Fix xfs_iunpin() sets I_DIRTY_SYNC after clear_inode().

David Chinner
2006-10-19T04:59:57Z

On Thu, Oct 19, 2006 at 11:23:02AM +0900, Takenori Nagano wrote:
> Hi David,
>
> I'm testing your three patches.
> I am not seeing any degradation with your patches.

That is good to hear ;)

> But I think the patch that I attach to this mail is necessary.
> Isn't it?

I don't think so - in the lookup code where we find an existing
inode, we don't destroy the inode if XFS_IRECLAIMABLE is set.
Instead we do a log force and repeat the lookup. We only destroy
the inode in xfs_iget_core() if we raced with another thread
reading the inode in off disk after the cache lookup has
failed. In this case, we free the inode we read off disk which,
by definition, cannot be dirty or pinned at this point so we
don't need to wait for anything to be unpinned.

In the case of reclaim, when we flush a dirty inode we already
do a xfs_iunpin_wait() (xfs_finish_reclaim()->xfs_iflush()->wait)
so we should never get to the point of xfs_idestroy with an inode
that is still pinned.

Hence I don't think this is patch is necessary. Did I miss something
that I shouldn't have, Takenori?

FYI, the three patches have survived my testing for almost a day now,
so if they pass your testing I think we have a viable fix. I'll
sned out a set of updated patches later this afternoon.

Cheers,