On Thu, Mar 22, 2018 at 12:27 AM, Christoph Hellwig <hch@xxxxxx> wrote:
>> + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL
>> + | (reason == BREAK_UNMAPI
>> + ? XFS_MMAPLOCK_EXCL : 0)));
>
> please split the assert, e.g.:
>
> ASSERT(xfs_isilocked(ip, XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
>
> switch (reason) {
> + case BREAK_UNMAPI:
> ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
Sure, I was thinking to keep it out of the loop, but the common case
is that this loop only iterates once.
>> + /* fall through */
>> + case BREAK_WRITE:
>> + error = xfs_break_leased_layouts(inode, iolock, &did_unlock);
>> + break;
>> + default:
>> + error = -EINVAL;
>> + break;
>> + }
>> +
>> + return error;
>
> I have to say I'd prefer BREAK_UNMAP over BREAK_UNMAPI given that weird
> I suffix doesn't buy us anything, but that's just a minor issue.
Ok will do.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
