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
signature.asc
Description: PGP signature
