[Cluster-devel] [PATCH 0/7] Allow lock dropping before waiting on inodes being freed

[Andreas Gruenbacher]

On Fri, May 13, 2016 at 7:42 PM, Andreas Gruenbacher
<agruenba at redhat.com> wrote:
> This patch set my Bob and me avoids a deadlock in gfs2 when an inode is being
> freed: normally, when find_inode finds an inode in I_FREEING or I_WILL_FREE
> state, it waits for the inode to go away before returning
> (__wait_on_freeing_inode).  Freeing an inode on gfs2 requires taking the
> inode's glock.  However, in gfs2_lookup_by_inum, we need to be holding the
> inode glock while doing lookups, so we can deadlock with processes concurrently
> freeing inodes.
>
> This patch set allows gfs2_lookup_by_inum to drop the inode glock before
> waiting for inodes to go away.  We could try non-blocking lookups and wait for
> an arbitrary amount of time before retrying the lookup instead as well, but
> waiting for the actual event (i.e., the inode being freed to likely have
> disappeared) makes more sense.

Another approach we have tried was to use iget5_locked and to drop the
glock in the test callback if necessary. However, we'd have to
atomically test for freeing inodes, release the glock, and cause
iget5_locked (actually find_inode) to wait and repeat. The test
callback can only return whether the inode matches the one we're
looking for though; it cannot return an error code like -EBUSY.

I've looked through the existing instances of iget5_locked. All test
callbacks only return 0 or 1, so we could theoretically change
iget5_locked to deal with a test result of -EBUSY or so as well. There
still doesn't seem to be a way of cleanly implementing the waiting for
freeing inodes with this approach though: the test callback is called
without i_lock held; it would have to __iget the inode itself to
prevent if from becoming freeing; find_inode would then __iget the
inode a second time. Yuck.

Andreas