| Message ID | 20200203175850.171689-7-preichl@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Remove wrappers for some semaphores | expand |
On Mon, Feb 03, 2020 at 06:58:49PM +0100, Pavel Reichl wrote: > Signed-off-by: Pavel Reichl <preichl@redhat.com> > Suggested-by: Dave Chinner <dchinner@redhat.com> > --- > fs/xfs/libxfs/xfs_bmap.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index c3638552b3c0..2d371f87e890 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -5829,7 +5829,8 @@ xfs_bmap_collapse_extents( > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL)); > + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) || > + xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); Hmmm. I think this is incorrect - a bug in the original code in that xfs_isilocked() will only check one lock type and so this never worked as intended. That is, we should have both the IOLOCK and the ILOCK held here. The IOLOCK is taken by the high level xfs_file_fallocate() code to lock out IO, while ILOCK is taken cwinside the transaction to make the metadata changes atomic. Hence I think this should actually end up as: ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL)); ASSERT(xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); > > if (!(ifp->if_flags & XFS_IFEXTENTS)) { > error = xfs_iread_extents(tp, ip, whichfork); > @@ -5946,7 +5947,8 @@ xfs_bmap_insert_extents( > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL)); > + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) || > + xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); Same here. Cheers, Dave.
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index c3638552b3c0..2d371f87e890 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -5829,7 +5829,8 @@ xfs_bmap_collapse_extents( if (XFS_FORCED_SHUTDOWN(mp)) return -EIO; - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL)); + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) || + xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); if (!(ifp->if_flags & XFS_IFEXTENTS)) { error = xfs_iread_extents(tp, ip, whichfork); @@ -5946,7 +5947,8 @@ xfs_bmap_insert_extents( if (XFS_FORCED_SHUTDOWN(mp)) return -EIO; - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL)); + ASSERT(xfs_is_iolocked(ip, XFS_IOLOCK_EXCL) || + xfs_is_ilocked(ip, XFS_ILOCK_EXCL)); if (!(ifp->if_flags & XFS_IFEXTENTS)) { error = xfs_iread_extents(tp, ip, whichfork);
Signed-off-by: Pavel Reichl <preichl@redhat.com> Suggested-by: Dave Chinner <dchinner@redhat.com> --- fs/xfs/libxfs/xfs_bmap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)