Re: [PATCH] xfs: i_state of inode is changed after the inode is freed

[Masayuki Saito <m-saito@xxxxxxxxxxxxxx>]

Hi Nathan, David,

I had the vacation too.

David Chinner <dgc@xxxxxxx> wrote:
>Hmmm - Idon't think we should iput() before we wake up any pinned waiters.
>When we have a waiter on i_ipin_wait (called from xfs_iflush()), we have
>a thread sleeping with the inode locked.
>
>If we then call iput() and it drops the last reference, we can call back
>into the filesystem and start transactions. Those transactions will need
>to lock the inode. Hence I think the above can deadlock when racing against
>an inode flush. 
>
>The code should probably read:
>
>       if (dropped last pincount) {
>               int need_iput = 0;
>               struct inode *inode;
>
>               spin_lock(i_flags_lock)
>               if (!reclaimable) {
>                       if (!vp) {
>                               if (!(i_state & (NEW|CLEAR))) {
>                                       inode = igrab(inode)
>                                       if (inode) {
>                                               need_iput = 1   
>                                               mark_inode_dirty_sync(inode)
>                                       }
>                               }
>                       }
>               }
>               spin_unlock(i_flags_lock)
>               wake_up(&ip->i_ipin_wait)
>               if (need_iput)
>                       iput(inode);
>       }
>
>to avoid this possible deadlock.

OK, I see your point.  There is wait_event(in xfs_iunpin_wait) that should
be wake_up'ed(in xfs_iunpin), so deadlock can occur.

I'll update my patch and test it.  Please wait for a few moments.