Re: [PATCH] raid1: ensure bio doesn't have more than BIO_MAX_VECS sectors

[Christoph Hellwig <hch@infradead.org>]

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);
+
 	if (max_sectors < bio_sectors(bio)) {
 		struct bio *split = bio_split(bio, max_sectors,
 					      GFP_NOIO, &conf->bio_split);