Re: [dm-devel] [PATCH 01/19] fs: remove mpage_alloc
On Wed, Mar 23, 2022 at 07:42:48AM +0100, Christoph Hellwig wrote:
> On Wed, Mar 23, 2022 at 06:38:22AM +0900, Ryusuke Konishi wrote:
> > This looks because the mask of GFP_KERNEL is removed along with
> > the removal of mpage_alloc().
> >
>
> > The default value of the gfp flag is set to GFP_HIGHUSER_MOVABLE by
> > inode_init_always().
> > So, __GFP_HIGHMEM hits the gfp warning at bio_alloc() that
> > do_mpage_readpage() calls.
>
> Yeah. Let's try this to match the iomap code:
>
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 9ed1e58e8d70b..d465883edf719 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -148,13 +148,11 @@ static struct bio *do_mpage_readpage(struct
> mpage_readpage_args *args)
> int op = REQ_OP_READ;
> unsigned nblocks;
> unsigned relative_block;
> - gfp_t gfp;
> + gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
>
> if (args->is_readahead) {
> op |= REQ_RAHEAD;
> - gfp = readahead_gfp_mask(page->mapping);
> - } else {
> - gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
> + gfp |= __GFP_NORETRY | __GFP_NOWARN;
> }
>
> if (page_has_buffers(page))
That fixes the problem for me.
Tested-by: Guenter Roeck
Guenter
Re: [dm-devel] [PATCH 01/19] fs: remove mpage_alloc
On Wed, Mar 23, 2022 at 3:42 PM Christoph Hellwig wrote:
>
> On Wed, Mar 23, 2022 at 06:38:22AM +0900, Ryusuke Konishi wrote:
> > This looks because the mask of GFP_KERNEL is removed along with
> > the removal of mpage_alloc().
> >
>
> > The default value of the gfp flag is set to GFP_HIGHUSER_MOVABLE by
> > inode_init_always().
> > So, __GFP_HIGHMEM hits the gfp warning at bio_alloc() that
> > do_mpage_readpage() calls.
>
> Yeah. Let's try this to match the iomap code:
>
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 9ed1e58e8d70b..d465883edf719 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -148,13 +148,11 @@ static struct bio *do_mpage_readpage(struct
> mpage_readpage_args *args)
> int op = REQ_OP_READ;
> unsigned nblocks;
> unsigned relative_block;
> - gfp_t gfp;
> + gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
>
> if (args->is_readahead) {
> op |= REQ_RAHEAD;
> - gfp = readahead_gfp_mask(page->mapping);
> - } else {
> - gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
> + gfp |= __GFP_NORETRY | __GFP_NOWARN;
> }
>
> if (page_has_buffers(page))
I did not test for iomap, but this patch has fixed the same regression on the
latest mainline at least for ext2, exfat, vfat and nilfs2. Thanks!
Ryusuke Konishi
Re: [dm-devel] [PATCH 01/19] fs: remove mpage_alloc
On Wed, Mar 23, 2022 at 06:38:22AM +0900, Ryusuke Konishi wrote:
> This looks because the mask of GFP_KERNEL is removed along with
> the removal of mpage_alloc().
>
> The default value of the gfp flag is set to GFP_HIGHUSER_MOVABLE by
> inode_init_always().
> So, __GFP_HIGHMEM hits the gfp warning at bio_alloc() that
> do_mpage_readpage() calls.
Yeah. Let's try this to match the iomap code:
diff --git a/fs/mpage.c b/fs/mpage.c
index 9ed1e58e8d70b..d465883edf719 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -148,13 +148,11 @@ static struct bio *do_mpage_readpage(struct
mpage_readpage_args *args)
int op = REQ_OP_READ;
unsigned nblocks;
unsigned relative_block;
- gfp_t gfp;
+ gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
if (args->is_readahead) {
op |= REQ_RAHEAD;
- gfp = readahead_gfp_mask(page->mapping);
- } else {
- gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
+ gfp |= __GFP_NORETRY | __GFP_NOWARN;
}
if (page_has_buffers(page))
Re: [dm-devel] [PATCH 01/19] fs: remove mpage_alloc
On Wed, Mar 23, 2022 at 6:19 AM Guenter Roeck wrote:
>
> On Mon, Jan 24, 2022 at 10:10:49AM +0100, Christoph Hellwig wrote:
> > open code mpage_alloc in it's two callers and simplify the results
> > because of the context:
> >
> > - __mpage_writepage always passes GFP_NOFS and can thus always sleep and
> > will never get a NULL return from bio_alloc at all.
> > - do_mpage_readpage can only get a non-sleeping context for readahead
> >which never sets PF_MEMALLOC and thus doesn't need the retry loop
> >either.
> >
> > Both cases will never have __GFP_HIGH set.
> >
>
> With this patch in the tree, I get:
>
> [1.198134] Unexpected gfp: 0x2 (__GFP_HIGHMEM). Fixing up to gfp:
> 0x1192888
> (GFP_NOWAIT|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_NOMEMALLOC|__GFP_HARDWALL|__GFP_MOVABLE|__GFP_SKIP_KASAN_POISON).
> Fix your code!
> [1.198783] CPU: 0 PID: 1 Comm: init Not tainted
> 5.17.0-01402-g8565d64430f8 #1
> [1.199165] Stack : 0042 0008
> dae882cc7dea7ec4
> [1.199563] a800014f0c00 a8000146b2c8
> 80d3a920
> [1.199750] a8000146b0e0 0001
>
> [1.199936] 0003087f 806d9f54
>
> [1.200121] a8000146b16f 80da 0001
> 0119288a
> [1.200306] 80da 0119288a
> 0119288a
> [1.200491] a80001416f00 80774d30
> a0042718
> [1.200676] 80ec2158 a80001468000 a8000146b2c0
>
> [1.200861] 80b55730 a8000146b3f8
> 80d3a920
> [1.201046] 0001 0119288a 80108fa0
> dae882cc7dea7ec4
> [1.201236] ...
> [1.201548] Call Trace:
> [1.201622] [] show_stack+0x38/0x118
> [1.201960] [] dump_stack_lvl+0x50/0x6c
> [1.202105] [] kmalloc_fix_flags+0x60/0x88
> [1.202249] [] new_slab+0x2d8/0x320
> [1.202375] [] ___slab_alloc.constprop.0+0x33c/0x5e8
> [1.202528] [] __slab_alloc.constprop.0+0x34/0x50
> [1.202675] [] kmem_cache_alloc+0x320/0x368
> [1.202811] [] bvec_alloc+0x78/0x128
> [1.202936] [] bio_alloc_bioset+0x194/0x340
> [1.203073] [] do_mpage_readpage+0x540/0x6e0
> [1.203213] [] mpage_readahead+0xc0/0x198
> [1.203346] [] read_pages+0xc0/0x2e0
> [1.203472] [] page_cache_ra_unbounded+0x1cc/0x290
> [1.203622] [] filemap_fault+0x4f4/0x7e8
> [1.203753] [] __do_fault+0x44/0x190
> [1.203878] [] __handle_mm_fault+0x7e4/0xcd0
> [1.204015] [] handle_mm_fault+0x110/0x258
> [1.204149] [] do_page_fault+0x110/0x4f0
> [1.204278] [] tlb_do_page_fault_1+0x108/0x110
> [1.204421] [] padzero+0x64/0x98
> [1.204538] [] load_elf_binary+0x1808/0x18d0
> [1.204677] [] bprm_execve+0x240/0x5a8
> [1.204806] [] kernel_execve+0x144/0x200
> [1.204937] [] try_to_run_init_process+0x18/0x58
> [1.205085] [] kernel_init+0xb4/0x10c
> [1.205220] [] ret_from_kernel_thread+0x14/0x1c
>
> with some qemu emulations. Bisect log is attached.
>
> I can not easily revert the patch since an attempt to do so causes
> conflicts, so I can not test upstream without this patch.
>
> Guenter
This looks because the mask of GFP_KERNEL is removed along with
the removal of mpage_alloc().
-static struct bio *
-mpage_alloc(struct block_device *bdev,
- sector_t first_sector, int nr_vecs,
- gfp_t gfp_flags)
-{
- struct bio *bio;
-
- /* Restrict the given (page cache) mask for slab allocations */
- gfp_flags &= GFP_KERNEL;
- bio = bio_alloc(gfp_flags, nr_vecs);
In read ahead mode, do_mpage_readpage() uses the gfp flag of page->mapping.
if (args->is_readahead) {
op |= REQ_RAHEAD;
gfp = readahead_gfp_mask(page->mapping);
} else {
gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
}
The default value of the gfp flag is set to GFP_HIGHUSER_MOVABLE by
inode_init_always().
So, __GFP_HIGHMEM hits the gfp warning at bio_alloc() that
do_mpage_readpage() calls.
Ryusuke Konishi
>
> ---
> # bad: [8565d64430f8278bea38dab0a3ab60b4e11c71e4] Merge tag
> 'bounds-fixes-v5.18-rc1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux
> # good: [f443e374ae131c168a065ea1748feac6b2e76613] Linux 5.17
> git bisect start 'HEAD' 'v5.17'
> # good: [5628b8de1228436d47491c662dc521bc138a3d43] Merge tag
> 'random-5.18-rc1-for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/crng/random
> git bisect good 5628b8de1228436d47491c662dc521bc138a3d43
> # bad: [69d1dea852b54eecd8ad2ec92a7fd371e9aec4bd] Merge tag
> 'for-5.18/drivers-2022-03-18' of git://git.kernel.dk/linux-block
> git bisect bad 69d1dea852b54eecd8ad2ec92a7fd371e9aec4bd
> # good: [b080cee72ef3
Re: [dm-devel] [PATCH 01/19] fs: remove mpage_alloc
On Mon, Jan 24, 2022 at 10:10:49AM +0100, Christoph Hellwig wrote: > open code mpage_alloc in it's two callers and simplify the results > because of the context: > > - __mpage_writepage always passes GFP_NOFS and can thus always sleep and > will never get a NULL return from bio_alloc at all. > - do_mpage_readpage can only get a non-sleeping context for readahead >which never sets PF_MEMALLOC and thus doesn't need the retry loop >either. > > Both cases will never have __GFP_HIGH set. > With this patch in the tree, I get: [1.198134] Unexpected gfp: 0x2 (__GFP_HIGHMEM). Fixing up to gfp: 0x1192888 (GFP_NOWAIT|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_NOMEMALLOC|__GFP_HARDWALL|__GFP_MOVABLE|__GFP_SKIP_KASAN_POISON). Fix your code! [1.198783] CPU: 0 PID: 1 Comm: init Not tainted 5.17.0-01402-g8565d64430f8 #1 [1.199165] Stack : 0042 0008 dae882cc7dea7ec4 [1.199563] a800014f0c00 a8000146b2c8 80d3a920 [1.199750] a8000146b0e0 0001 [1.199936] 0003087f 806d9f54 [1.200121] a8000146b16f 80da 0001 0119288a [1.200306] 80da 0119288a 0119288a [1.200491] a80001416f00 80774d30 a0042718 [1.200676] 80ec2158 a80001468000 a8000146b2c0 [1.200861] 80b55730 a8000146b3f8 80d3a920 [1.201046] 0001 0119288a 80108fa0 dae882cc7dea7ec4 [1.201236] ... [1.201548] Call Trace: [1.201622] [] show_stack+0x38/0x118 [1.201960] [] dump_stack_lvl+0x50/0x6c [1.202105] [] kmalloc_fix_flags+0x60/0x88 [1.202249] [] new_slab+0x2d8/0x320 [1.202375] [] ___slab_alloc.constprop.0+0x33c/0x5e8 [1.202528] [] __slab_alloc.constprop.0+0x34/0x50 [1.202675] [] kmem_cache_alloc+0x320/0x368 [1.202811] [] bvec_alloc+0x78/0x128 [1.202936] [] bio_alloc_bioset+0x194/0x340 [1.203073] [] do_mpage_readpage+0x540/0x6e0 [1.203213] [] mpage_readahead+0xc0/0x198 [1.203346] [] read_pages+0xc0/0x2e0 [1.203472] [] page_cache_ra_unbounded+0x1cc/0x290 [1.203622] [] filemap_fault+0x4f4/0x7e8 [1.203753] [] __do_fault+0x44/0x190 [1.203878] [] __handle_mm_fault+0x7e4/0xcd0 [1.204015] [] handle_mm_fault+0x110/0x258 [1.204149] [] do_page_fault+0x110/0x4f0 [1.204278] [] tlb_do_page_fault_1+0x108/0x110 [1.204421] [] padzero+0x64/0x98 [1.204538] [] load_elf_binary+0x1808/0x18d0 [1.204677] [] bprm_execve+0x240/0x5a8 [1.204806] [] kernel_execve+0x144/0x200 [1.204937] [] try_to_run_init_process+0x18/0x58 [1.205085] [] kernel_init+0xb4/0x10c [1.205220] [] ret_from_kernel_thread+0x14/0x1c with some qemu emulations. Bisect log is attached. I can not easily revert the patch since an attempt to do so causes conflicts, so I can not test upstream without this patch. Guenter --- # bad: [8565d64430f8278bea38dab0a3ab60b4e11c71e4] Merge tag 'bounds-fixes-v5.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux # good: [f443e374ae131c168a065ea1748feac6b2e76613] Linux 5.17 git bisect start 'HEAD' 'v5.17' # good: [5628b8de1228436d47491c662dc521bc138a3d43] Merge tag 'random-5.18-rc1-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/crng/random git bisect good 5628b8de1228436d47491c662dc521bc138a3d43 # bad: [69d1dea852b54eecd8ad2ec92a7fd371e9aec4bd] Merge tag 'for-5.18/drivers-2022-03-18' of git://git.kernel.dk/linux-block git bisect bad 69d1dea852b54eecd8ad2ec92a7fd371e9aec4bd # good: [b080cee72ef355669cbc52ff55dc513d37433600] Merge tag 'for-5.18/io_uring-statx-2022-03-18' of git://git.kernel.dk/linux-block git bisect good b080cee72ef355669cbc52ff55dc513d37433600 # bad: [22027a9811349de28f81e13e20e83299099acd3a] nvmet: replace ida_simple[get|remove] with the simler ida_[alloc|free] git bisect bad 22027a9811349de28f81e13e20e83299099acd3a # bad: [672fdcf0e7de3b1e39416ac85abf178f023271f1] block: partition include/linux/blk-cgroup.h git bisect bad 672fdcf0e7de3b1e39416ac85abf178f023271f1 # bad: [b42c1fc3d55e077d36718ad9800d89100b2aff81] block: fix the kerneldoc for bio_end_io_acct git bisect bad b42c1fc3d55e077d36718ad9800d89100b2aff81 # bad: [4b1dc86d1857f1007865cab759f2285280692eee] drbd: bio_alloc can't fail if it is allow to sleep git bisect bad 4b1dc86d1857f1007865cab759f2285280692eee # bad: [f0d911927b3c7cf5f9edb5941d0287144a602d0d] nilfs2: remove nilfs_alloc_seg_bio git bisect bad f0d911927b3c7cf5f9edb5941d0287144a602d0d # good: [e7243285c0fc87054990fcde630583586ff8ed5f] block: move blk_drop_partitions to blk.h git bisect good e7243285c0fc87054990fcde630583586ff8ed5f # bad: [d5f68a42da7a4516e7503c281a54a58727f07dc3]
