Re: [PATCH v6 13/15] xfs: communicate lock drop events from xfs_break_layouts()

Christoph Hellwig
2018-03-16T19:08:04Z

On Thu, Mar 15, 2018 at 08:52:39AM -0700, Dan Williams wrote:
> In preparation for adding a new layout type, teach xfs_break_layouts()
> to return a positive number if it needed to drop locks while trying to
> break leases. For all layouts to be successfully broken each layout type
> needs to be able to assert that the layouts were broken with the locks
> held.
>
> The existing a xfs_break_layouts() is pushed down a level to
> xfs_break_leased_layouts() and the new xfs_break_layouts() will
> coordinate interpreting the return code from the low level 'break'
> helpers.

With that the subject line is rather confusing, given that the
externally visible xfs_break_layouts does not communicate the lock
drop events. So maybe this should just be titled something about
refactoring. Or just merged into the next patch which reshuffles
everything again anyway.

> int
> -xfs_break_layouts(
> +xfs_break_leased_layouts(
> struct inode *inode,
> uint *iolock)
> {
> struct xfs_inode *ip = XFS_I(inode);
> int error;
> -
> - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL
> - | XFS_MMAPLOCK_EXCL));
> + int did_unlock = 0;
>
> while ((error = break_layout(inode, false) == -EWOULDBLOCK)) {
> xfs_iunlock(ip, *iolock);
> + did_unlock = 1;
> error = break_layout(inode, true);
> *iolock &= ~XFS_IOLOCK_SHARED;
> *iolock |= XFS_IOLOCK_EXCL;
> xfs_ilock(ip, *iolock);
> }
>
> - return error;
> + if (error < 0)
> + return error;
> + return did_unlock;

And I suspect the cleaner interface would be to just pass a
bool *did_unlock argument.