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> -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html