On Tue, Jan 16, 2024 at 09:59:44AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
LOL I just wrote this exact patch to shut up lockdep.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--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
>
>
