| Message ID | 20240115230113.4080105-7-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xfs: remove remaining kmem interfaces and GFP_NOFS usage | expand |
On Tue, Jan 16, 2024 at 09:59:44AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > We currently use a btree walk in the fstrim code. This requires a > btree cursor and btree cursors are only used inside transactions > except for the fstrim code. This means that all the btree operations > that allocate memory operate in both GFP_KERNEL and GFP_NOFS > contexts. > > This causes problems with lockdep being unable to determine the > difference between objects that are safe to lock both above and > below memory reclaim. Free space btree buffers are definitely locked > both above and below reclaim and that means we have to mark all > btree infrastructure allocations with GFP_NOFS to avoid potential > lockdep false positives. > > If we wrap this btree walk in an empty cursor, all btree walks are > now done under transaction context and so all allocations inherit > GFP_NOFS context from the tranaction. This enables us to move all > the btree allocations to GFP_KERNEL context and hence help remove > the explicit use of GFP_NOFS in XFS. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> LOL I just wrote this exact patch to shut up lockdep. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/xfs_discard.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c > index 8539f5c9a774..299b8f907292 100644 > --- a/fs/xfs/xfs_discard.c > +++ b/fs/xfs/xfs_discard.c > @@ -8,6 +8,7 @@ > #include "xfs_format.h" > #include "xfs_log_format.h" > #include "xfs_trans_resv.h" > +#include "xfs_trans.h" > #include "xfs_mount.h" > #include "xfs_btree.h" > #include "xfs_alloc_btree.h" > @@ -120,7 +121,7 @@ xfs_discard_extents( > error = __blkdev_issue_discard(mp->m_ddev_targp->bt_bdev, > XFS_AGB_TO_DADDR(mp, busyp->agno, busyp->bno), > XFS_FSB_TO_BB(mp, busyp->length), > - GFP_NOFS, &bio); > + GFP_KERNEL, &bio); > if (error && error != -EOPNOTSUPP) { > xfs_info(mp, > "discard failed for extent [0x%llx,%u], error %d", > @@ -155,6 +156,7 @@ xfs_trim_gather_extents( > uint64_t *blocks_trimmed) > { > struct xfs_mount *mp = pag->pag_mount; > + struct xfs_trans *tp; > struct xfs_btree_cur *cur; > struct xfs_buf *agbp; > int error; > @@ -168,11 +170,15 @@ xfs_trim_gather_extents( > */ > xfs_log_force(mp, XFS_LOG_SYNC); > > - error = xfs_alloc_read_agf(pag, NULL, 0, &agbp); > + error = xfs_trans_alloc_empty(mp, &tp); > if (error) > return error; > > - cur = xfs_allocbt_init_cursor(mp, NULL, agbp, pag, XFS_BTNUM_CNT); > + error = xfs_alloc_read_agf(pag, tp, 0, &agbp); > + if (error) > + goto out_trans_cancel; > + > + cur = xfs_allocbt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_CNT); > > /* > * Look up the extent length requested in the AGF and start with it. > @@ -279,7 +285,8 @@ xfs_trim_gather_extents( > xfs_extent_busy_clear(mp, &extents->extent_list, false); > out_del_cursor: > xfs_btree_del_cursor(cur, error); > - xfs_buf_relse(agbp); > +out_trans_cancel: > + xfs_trans_cancel(tp); > return error; > } > > -- > 2.43.0 > >
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index 8539f5c9a774..299b8f907292 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -8,6 +8,7 @@ #include "xfs_format.h" #include "xfs_log_format.h" #include "xfs_trans_resv.h" +#include "xfs_trans.h" #include "xfs_mount.h" #include "xfs_btree.h" #include "xfs_alloc_btree.h" @@ -120,7 +121,7 @@ xfs_discard_extents( error = __blkdev_issue_discard(mp->m_ddev_targp->bt_bdev, XFS_AGB_TO_DADDR(mp, busyp->agno, busyp->bno), XFS_FSB_TO_BB(mp, busyp->length), - GFP_NOFS, &bio); + GFP_KERNEL, &bio); if (error && error != -EOPNOTSUPP) { xfs_info(mp, "discard failed for extent [0x%llx,%u], error %d", @@ -155,6 +156,7 @@ xfs_trim_gather_extents( uint64_t *blocks_trimmed) { struct xfs_mount *mp = pag->pag_mount; + struct xfs_trans *tp; struct xfs_btree_cur *cur; struct xfs_buf *agbp; int error; @@ -168,11 +170,15 @@ xfs_trim_gather_extents( */ xfs_log_force(mp, XFS_LOG_SYNC); - error = xfs_alloc_read_agf(pag, NULL, 0, &agbp); + error = xfs_trans_alloc_empty(mp, &tp); if (error) return error; - cur = xfs_allocbt_init_cursor(mp, NULL, agbp, pag, XFS_BTNUM_CNT); + error = xfs_alloc_read_agf(pag, tp, 0, &agbp); + if (error) + goto out_trans_cancel; + + cur = xfs_allocbt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_CNT); /* * Look up the extent length requested in the AGF and start with it. @@ -279,7 +285,8 @@ xfs_trim_gather_extents( xfs_extent_busy_clear(mp, &extents->extent_list, false); out_del_cursor: xfs_btree_del_cursor(cur, error); - xfs_buf_relse(agbp); +out_trans_cancel: + xfs_trans_cancel(tp); return error; }