xfs
[Top] [All Lists]

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

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [patch] Fix xfs_iunpin() sets I_DIRTY_SYNC after clear_inode().
From: Takenori Nagano <t-nagano@xxxxxxxxxxxxx>
Date: Mon, 23 Oct 2006 15:53:42 +0900
Cc: xfs@xxxxxxxxxxx
In-reply-to: <45384FC6.5040000@xxxxxxxxxxxxx>
References: <45237CCE.4010007@xxxxxxxxxxxxx> <20061006032617.GC11034@xxxxxxxxxxxxxxxxx> <20061011064357.GN19345@xxxxxxxxxxxxxxxxx> <452E32FF.8010109@xxxxxxxxxxxxx> <20061013014651.GC19345@xxxxxxxxxxxxxxxxx> <452F83BD.8050501@xxxxxxxxxxxxx> <20061017020218.GE8394166@xxxxxxxxxxxxxxxxx> <20061018023325.GL8394166@xxxxxxxxxxxxxxxxx> <20061018090701.GU11034@xxxxxxxxxxxxxxxxx> <4536E186.5040301@xxxxxxxxxxxxx> <20061019045851.GZ11034@xxxxxxxxxxxxxxxxx> <45384FC6.5040000@xxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 1.5.0.7 (Windows/20060909)
Hi David,

Your patches are surviving my testing. I have checked results of vmstat and
/var/log/messages, but I can't see any errors and degradation. I think they are
a viable fix, too.

Takenori Nagano wrote:
> Hi,
> 
> David Chinner wrote:
>> 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?
> 
> Sorry, you are right. I forgot xfs_iget_core() was modified that it don't 
> reuse
> xfs_inode while i_pincount > 0.
> 
>> 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.
> 
> Your patches have been working well for 20 hours. I intend to continue
> testing your patches until next Monday, and I'll report the result.
> 
> Best Regards,

-- 
Takenori Nagano <t-nagano@xxxxxxxxxxxxx>


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