Re: [PATCH 10/12] xfs: place the CIL under nofs allocation context

Darrick J. Wong
2024-01-18T23:41:12Z

On Tue, Jan 16, 2024 at 09:59:48AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> This is core code that needs to run in low memory conditions and
> can be triggered from memory reclaim. While it runs in a workqueue,
> it really shouldn't be recursing back into the filesystem during
> any memory allocation it needs to function.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_log_cil.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 815a2181004c..8c3b09777006 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -100,7 +100,7 @@ xlog_cil_ctx_alloc(void)
> {
> struct xfs_cil_ctx *ctx;
>
> - ctx = kzalloc(sizeof(*ctx), GFP_NOFS | __GFP_NOFAIL);
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL | __GFP_NOFAIL);
> INIT_LIST_HEAD(&ctx->committing);
> INIT_LIST_HEAD(&ctx->busy_extents.extent_list);
> INIT_LIST_HEAD(&ctx->log_items);
> @@ -1116,11 +1116,18 @@ xlog_cil_cleanup_whiteouts(
> * same sequence twice. If we get a race between multiple pushes for the same
> * sequence they will block on the first one and then abort, hence avoiding
> * needless pushes.
> + *
> + * This runs from a workqueue so it does not inherent any specific memory

inherit? ^^^^^^^^

If that change is correct,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> + * allocation context. However, we do not want to block on memory reclaim
> + * recursing back into the filesystem because this push may have been triggered
> + * by memory reclaim itself. Hence we really need to run under full GFP_NOFS
> + * contraints here.
> */
> static void
> xlog_cil_push_work(
> struct work_struct *work)
> {
> + unsigned int nofs_flags = memalloc_nofs_save();
> struct xfs_cil_ctx *ctx =
> container_of(work, struct xfs_cil_ctx, push_work);
> struct xfs_cil *cil = ctx->cil;
> @@ -1334,12 +1341,14 @@ xlog_cil_push_work(
> spin_unlock(&log->l_icloglock);
> xlog_cil_cleanup_whiteouts(&whiteouts);
> xfs_log_ticket_ungrant(log, ticket);
> + memalloc_nofs_restore(nofs_flags);
> return;
>
> out_skip:
> up_write(&cil->xc_ctx_lock);
> xfs_log_ticket_put(new_ctx->ticket);
> kfree(new_ctx);
> + memalloc_nofs_restore(nofs_flags);
> return;
>
> out_abort_free_ticket:
> @@ -1348,6 +1357,7 @@ xlog_cil_push_work(
> if (!ctx->commit_iclog) {
> xfs_log_ticket_ungrant(log, ctx->ticket);
> xlog_cil_committed(ctx);
> + memalloc_nofs_restore(nofs_flags);
> return;
> }
> spin_lock(&log->l_icloglock);
> @@ -1356,6 +1366,7 @@ xlog_cil_push_work(
> /* Not safe to reference ctx now! */
> spin_unlock(&log->l_icloglock);
> xfs_log_ticket_ungrant(log, ticket);
> + memalloc_nofs_restore(nofs_flags);
> }
>
> /*
> --
> 2.43.0
>
>