Messages in this thread | | | From | Dan Williams <> | Date | Thu, 22 Mar 2018 08:50:43 -0700 | Subject | Re: [PATCH v7 13/14] xfs: prepare xfs_break_layouts() for another layout type |
| |
On Thu, Mar 22, 2018 at 12:27 AM, Christoph Hellwig <hch@lst.de> 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@lst.de>
| |