| Message ID | 20240115230113.4080105-11-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: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 > >
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 + * 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); } /*