lkml.org 
[lkml]   [2018]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 10/11] xfs: prepare xfs_break_layouts() for another layout type
> 
> +int
> +xfs_break_layouts(
> + struct inode *inode,
> + uint *iolock,
> + unsigned long flags)
> +{
> + struct xfs_inode *ip = XFS_I(inode);
> + uint iolock_assert = 0;
> + int ret = 0;
> +
> + if (flags & XFS_BREAK_REMOTE)
> + iolock_assert |= XFS_IOLOCK_SHARED|XFS_IOLOCK_EXCL;
> + if (flags & XFS_BREAK_MAPS)
> + iolock_assert |= XFS_MMAPLOCK_EXCL;
> +
> + ASSERT(xfs_isilocked(ip, iolock_assert));
> +
> + if (flags & XFS_BREAK_REMOTE)
> + ret = xfs_break_leased_layouts(inode, iolock);
> + return ret;

This just looks weird as hell. We already pass in what to drop/reacquire
in the iolock argument. I don't think we need another argument controlled
by the same callers to assert it.

> @@ -768,7 +790,7 @@ xfs_file_fallocate(
> struct xfs_inode *ip = XFS_I(inode);
> long error;
> enum xfs_prealloc_flags flags = 0;
> - uint iolock = XFS_IOLOCK_EXCL;
> + uint iolock = XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL;

This is a behavior change that should not be in a patch titled
"prepare xfs_break_layouts() for another layout type" but in one
explicitly changing this and documenting why.

In summary: I think this should be replaced with a patch that
allows xfs_break_layouts to be called with the mmap lock held, and
change the callers that want the mmap lock to pass it with a good
explanation, and we should get rid of the XFS_BREAK_* flags here.
(need to check the next patch if there is any other good reason for
them to be added later, though).

\
 
 \ /
  Last update: 2018-03-10 10:52    [W:0.093 / U:2.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site