| Message ID | 20181123160533.21204-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] Btrfs: fix deadlock with memory reclaim during scrub | expand |
On 23.11.18 г. 18:05 ч., fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > When a transaction commit starts, it attempts to pause scrub and it blocks > until the scrub is paused. So while the transaction is blocked waiting for > scrub to pause, we can not do memory allocation with GFP_KERNEL while scrub > is running, we must use GFP_NOS to avoid deadlock with reclaim. Checking > for pause requests is done early in the while loop of scrub_stripe(), and > later in the loop, scrub_extent() is called, which in turns calls > scrub_pages(), which does memory allocations using GFP_KERNEL. So use > GFP_NOFS for the memory allocations because at any time a scrub pause > request can happen from another task that started to commit a transaction. > > Fixes: 58c4e173847a ("btrfs: scrub: use GFP_KERNEL on the submission path") > Signed-off-by: Filipe Manana <fdmanana@suse.com> > --- > > V2: Make using GFP_NOFS unconditionial. Previous version was racy, as pausing > requests migth happen just after we checked for them. > > fs/btrfs/scrub.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 3be1456b5116..0630ea0881bc 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2205,7 +2205,13 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, > struct scrub_block *sblock; > int index; > > - sblock = kzalloc(sizeof(*sblock), GFP_KERNEL); > + /* > + * In order to avoid deadlock with reclaim when there is a transaction > + * trying to pause scrub, use GFP_NOFS. The pausing request is done when > + * the transaction commit starts, and it blocks the transaction until > + * scrub is paused (done at specific points at scrub_stripe()). > + */ > + sblock = kzalloc(sizeof(*sblock), GFP_NOFS); Newer code shouldn't use GFP_NOFS, rather leave GFP_KERNEL as is and instead use the memaloc_nofs_save/memalloc_nofs_restore. For background information refer to: Documentation/core-api/gfp_mask-from-fs-io.rst > if (!sblock) { > spin_lock(&sctx->stat_lock); > sctx->stat.malloc_errors++; > @@ -2223,7 +2229,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, > struct scrub_page *spage; > u64 l = min_t(u64, len, PAGE_SIZE); > > - spage = kzalloc(sizeof(*spage), GFP_KERNEL); > + spage = kzalloc(sizeof(*spage), GFP_NOFS); > if (!spage) { > leave_nomem: > spin_lock(&sctx->stat_lock); > @@ -2250,7 +2256,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, > spage->have_csum = 0; > } > sblock->page_count++; > - spage->page = alloc_page(GFP_KERNEL); > + spage->page = alloc_page(GFP_NOFS); > if (!spage->page) > goto leave_nomem; > len -= l; >
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 3be1456b5116..0630ea0881bc 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -2205,7 +2205,13 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, struct scrub_block *sblock; int index; - sblock = kzalloc(sizeof(*sblock), GFP_KERNEL); + /* + * In order to avoid deadlock with reclaim when there is a transaction + * trying to pause scrub, use GFP_NOFS. The pausing request is done when + * the transaction commit starts, and it blocks the transaction until + * scrub is paused (done at specific points at scrub_stripe()). + */ + sblock = kzalloc(sizeof(*sblock), GFP_NOFS); if (!sblock) { spin_lock(&sctx->stat_lock); sctx->stat.malloc_errors++; @@ -2223,7 +2229,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, struct scrub_page *spage; u64 l = min_t(u64, len, PAGE_SIZE); - spage = kzalloc(sizeof(*spage), GFP_KERNEL); + spage = kzalloc(sizeof(*spage), GFP_NOFS); if (!spage) { leave_nomem: spin_lock(&sctx->stat_lock); @@ -2250,7 +2256,7 @@ static int scrub_pages(struct scrub_ctx *sctx, u64 logical, u64 len, spage->have_csum = 0; } sblock->page_count++; - spage->page = alloc_page(GFP_KERNEL); + spage->page = alloc_page(GFP_NOFS); if (!spage->page) goto leave_nomem; len -= l;