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

[Takenori Nagano <t-nagano@ah.jp.nec.com>]

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@ah.jp.nec.com>