[PATCH V7 20/24] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

[PATCH V7 20/24] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

[Ming Lei]

bch_bio_alloc_pages() is always called on one new bio, so it is safe
to access the bvec table directly. Given it is the only kind of this
case, open code the bvec table access since bio_for_each_segment_all()
will be changed to support for iterating over multipage bvec.

Cc: Coly Li <colyli@suse.de>
Cc: linux-bcache@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/md/bcache/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
index fc479b026d6d..9f2a6fd5dfc9 100644
--- a/drivers/md/bcache/util.c
+++ b/drivers/md/bcache/util.c
@@ -268,7 +268,7 @@ int bch_bio_alloc_pages(struct bio *bio, gfp_t gfp_mask)
 	int i;
 	struct bio_vec *bv;
 
-	bio_for_each_segment_all(bv, bio, i) {
+	for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) {
 		bv->bv_page = alloc_page(gfp_mask);
 		if (!bv->bv_page) {
 			while (--bv >= bio->bi_io_vec)
-- 
2.9.5

Re: [PATCH V7 20/24] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

[Coly Li]

On 2018/6/27 8:45 PM, Ming Lei wrote:
> bch_bio_alloc_pages() is always called on one new bio, so it is safe
> to access the bvec table directly. Given it is the only kind of this
> case, open code the bvec table access since bio_for_each_segment_all()
> will be changed to support for iterating over multipage bvec.
> 
> Cc: Coly Li <colyli@suse.de>
> Cc: linux-bcache@vger.kernel.org
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/md/bcache/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
> index fc479b026d6d..9f2a6fd5dfc9 100644
> --- a/drivers/md/bcache/util.c
> +++ b/drivers/md/bcache/util.c
> @@ -268,7 +268,7 @@ int bch_bio_alloc_pages(struct bio *bio, gfp_t gfp_mask)
>  	int i;
>  	struct bio_vec *bv;
> 

Hi Ming,

> -	bio_for_each_segment_all(bv, bio, i) {
> +	for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) {


Is it possible to treat this as a special condition of
bio_for_each_segement_all() ? I mean only iterate one time in
bvec_for_each_segment(). I hope the above change is not our last choice
before I reply an Acked-by :-)

Thanks.

Coly Li

>  		bv->bv_page = alloc_page(gfp_mask);
>  		if (!bv->bv_page) {
>  			while (--bv >= bio->bi_io_vec)
> 

Re: [PATCH V7 20/24] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

[Ming Lei]

On Wed, Jun 27, 2018 at 11:55:33PM +0800, Coly Li wrote:
> On 2018/6/27 8:45 PM, Ming Lei wrote:
> > bch_bio_alloc_pages() is always called on one new bio, so it is safe
> > to access the bvec table directly. Given it is the only kind of this
> > case, open code the bvec table access since bio_for_each_segment_all()
> > will be changed to support for iterating over multipage bvec.
> > 
> > Cc: Coly Li <colyli@suse.de>
> > Cc: linux-bcache@vger.kernel.org
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/md/bcache/util.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
> > index fc479b026d6d..9f2a6fd5dfc9 100644
> > --- a/drivers/md/bcache/util.c
> > +++ b/drivers/md/bcache/util.c
> > @@ -268,7 +268,7 @@ int bch_bio_alloc_pages(struct bio *bio, gfp_t gfp_mask)
> >  	int i;
> >  	struct bio_vec *bv;
> > 
> 
> Hi Ming,
> 
> > -	bio_for_each_segment_all(bv, bio, i) {
> > +	for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) {
> 
> 
> Is it possible to treat this as a special condition of
> bio_for_each_segement_all() ? I mean only iterate one time in
> bvec_for_each_segment(). I hope the above change is not our last choice
> before I reply an Acked-by :-)

Now the bvec from bio_for_each_segement_all() can't be changed any more
since the referenced 'bvec' is generated in-flight given we store
real multipage bvec.

BTW, this way is actually suggested by Christoph for saving one new
helper of bio_for_each_bvec_all() as done in V6, and per previous discussion,
seems both Kent and Christoph agrees to convert bcache into bio_add_page()
finally.

So I guess this open code style should be fine.

Thanks,
Ming

Re: [PATCH V7 20/24] bcache: avoid to use bio_for_each_segment_all() in bch_bio_alloc_pages()

[Coly Li]

On 2018/6/28 9:28 AM, Ming Lei wrote:
> On Wed, Jun 27, 2018 at 11:55:33PM +0800, Coly Li wrote:
>> On 2018/6/27 8:45 PM, Ming Lei wrote:
>>> bch_bio_alloc_pages() is always called on one new bio, so it is safe
>>> to access the bvec table directly. Given it is the only kind of this
>>> case, open code the bvec table access since bio_for_each_segment_all()
>>> will be changed to support for iterating over multipage bvec.
>>>
>>> Cc: Coly Li <colyli@suse.de>
>>> Cc: linux-bcache@vger.kernel.org
>>> Signed-off-by: Ming Lei <ming.lei@redhat.com>
>>> ---
>>>  drivers/md/bcache/util.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
>>> index fc479b026d6d..9f2a6fd5dfc9 100644
>>> --- a/drivers/md/bcache/util.c
>>> +++ b/drivers/md/bcache/util.c
>>> @@ -268,7 +268,7 @@ int bch_bio_alloc_pages(struct bio *bio, gfp_t gfp_mask)
>>>  	int i;
>>>  	struct bio_vec *bv;
>>>
>>
>> Hi Ming,
>>
>>> -	bio_for_each_segment_all(bv, bio, i) {
>>> +	for (i = 0, bv = bio->bi_io_vec; i < bio->bi_vcnt; bv++) {
>>
>>
>> Is it possible to treat this as a special condition of
>> bio_for_each_segement_all() ? I mean only iterate one time in
>> bvec_for_each_segment(). I hope the above change is not our last choice
>> before I reply an Acked-by :-)
> 
> Now the bvec from bio_for_each_segement_all() can't be changed any more
> since the referenced 'bvec' is generated in-flight given we store
> real multipage bvec.
> 
> BTW, this way is actually suggested by Christoph for saving one new
> helper of bio_for_each_bvec_all() as done in V6, and per previous discussion,
> seems both Kent and Christoph agrees to convert bcache into bio_add_page()
> finally.
> 
> So I guess this open code style should be fine.

Hi Ming,

I see, thanks for the hint.

Acked-by: Coly Li <colyli@suse.de>

Coly Li