On Sat, Aug 14, 2021 at 08:55:21AM +0100, Christoph Hellwig wrote:
> On Fri, Aug 13, 2021 at 04:38:59PM +0800, Guoqing Jiang wrote:
> >
> > Ok, thanks.
> >
> > > In general the size of a bio only depends on the number of vectors, not
> > > the total I/O size. But alloc_behind_master_bio allocates new backing
> > > pages using order 0 allocations, so in this exceptional case the total
> > > size oes actually matter.
> > >
> > > While we're at it: this huge memory allocation looks really deadlock
> > > prone.
> >
> > Hmm, let me think more about it, or could you share your thought? ????
>
> Well, you'd need a mempool which can fit the max payload of a bio,
> that is BIO_MAX_VECS pages.
>
> FYI, this is what I'd do instead of this patch for now. We don't really
> need a vetor per sector, just per page. So this limits the I/O
> size a little less.
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 3c44c4bb40fc..5b27d995302e 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1454,6 +1454,15 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> goto retry_write;
> }
>
> + /*
> + * When using a bitmap, we may call alloc_behind_master_bio below.
> + * alloc_behind_master_bio allocates a copy of the data payload a page
> + * at a time and thus needs a new bio that can fit the whole payload
> + * this bio in page sized chunks.
> + */
> + if (bitmap)
> + max_sectors = min_t(int, max_sectors, BIO_MAX_VECS * PAGE_SIZE);
s/PAGE_SIZE/PAGE_SECTORS
> +
> if (max_sectors < bio_sectors(bio)) {
> struct bio *split = bio_split(bio, max_sectors,
> GFP_NOIO, &conf->bio_split);
>
Here the limit is max single-page vectors, and the above way may not work,
such as:
0 ~ 254: each bvec's length is 512
255: bvec's length is 8192
the total length is just 512*255 + 8192 = 138752 bytes = 271 sectors, but it
still may need 257 bvecs, which can't be allocated via bio_alloc_bioset().
One solution is to add queue limit of max_single_page_bvec, and let
blk_queue_split() handle it.
Thanks,
Ming
