Re: Issues about the merge_bvec_fn callback in 3.10 series

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 21 2019, Jianchao Wang wrote:

> Hi dear all
>
> This is a question in older kernel versions.
>
> We are using 3.10 series kernel in our production. And we encountered issue as below,
>
> When add a page into a bio, .merge_bvec_fn will be invoked down to the bottom,
> and the bio->bi_rw would be saved into bvec_merge_data.bi_rw as the following code,
>
> __bio_add_page
> ---
> 	if (q->merge_bvec_fn) {
> 		struct  bvm = {
> 			.bi_bdev = bio->bi_bdev,
> 			.bi_sector = bio->bi_iter.bi_sector,
> 			.bi_size = bio->bi_iter.bi_size,
> 			.bi_rw = bio->bi_rw,
> 		};
>
> 		/*
> 		 * merge_bvec_fn() returns number of bytes it can accept
> 		 * at this offset
> 		 */
> 		if (q->merge_bvec_fn(q, &bvm, bvec) < bvec->bv_len) {
> 			bvec->bv_page = NULL;
> 			bvec->bv_len = 0;
> 			bvec->bv_offset = 0;
> 			return 0;
> 		}
> 	}
> ---
>
> However, it seems that the bio->bi_rw has not been set at the moment (set by submit_bio), 
> so it is always zero.

Yeah, that's a problem.

>
> We have a raid5 and the raid5_mergeable_bvec would always handle the write as read and then
> we always get a write bio with a stripe chunk size which is not expected and would degrade the
> performance. This is code,
>
> raid5_mergeable_bvec
> ---
> 	if ((bvm->bi_rw & 1) == WRITE)
> 		return biovec->bv_len; /* always allow writes to be mergeable */
>
> 	if (mddev->new_chunk_sectors < mddev->chunk_sectors)
> 		chunk_sectors = mddev->new_chunk_sectors;
> 	max =  (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;
> 	if (max < 0) max = 0;
> 	if (max <= biovec->bv_len && bio_sectors == 0)
> 		return biovec->bv_len;
> 	else
> 		return max;
>
> ---
>
> I have checked   
> v3.10.108
> v3.18.140
> v4.1.49
> but there seems not fix for it.
>
> And maybe it would be fixed until 
> 8ae126660fddbeebb9251a174e6fa45b6ad8f932
> block: kill merge_bvec_fn() completely
>
> Would anyone please give some suggestion on this ?

One option would be to make sure that ->bi_rw is set before
bio_add_page is called.
There are about 80 calls, so that isn't trivial, but you might not care
about several of them.

You could backport the 'kill merge_bvec_fn' patch if you like, but I
wouldn't.  The change of introducing a bug is much higher.

NeilBrown


> Any comment will be welcomed.
>
> Thanks in advance
> Jianchao

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux