Messages in this thread | | | Subject | Re: [PATCH] bcache: remove the redundant judgment on bi_size | From | Coly Li <> | Date | Tue, 31 Aug 2021 09:35:09 +0800 |
| |
On 8/30/21 2:29 PM, Xifengfei wrote: > (Sorry, there was an obvious typo in the last email) > Thanks a lot. I understand the purpose. > So is the original judgment process too complicated? Can we judge bi_size directly? This will be more concise > > @@ -423,7 +423,7 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio) > add_sequential(task); > i->sequential = 0; > found: > - if (i->sequential + bio->bi_iter.bi_size > i->sequential) > + if (bio->bi_iter.bi_size) > i->sequential += bio->bi_iter.bi_size; > > i->last = bio_end_sector(bio);
The above change works, but the code readability decreased because how/why i->sequential is maintained is not that directly visible.
This is a difference of coding styles. IMHO for this particular case, the readability is more important than less CPU instructions.
Thanks.
Coly Li
> Thanks > Fengfei > > -----邮件原件----- > 发件人: Coly Li [mailto:colyli@suse.de] > 发送时间: 2021年8月29日 15:50 > 收件人: xifengfei (RD) <xi.fengfei@h3c.com> > 抄送: linux-bcache@vger.kernel.org; linux-kernel@vger.kernel.org; kent.overstreet@gmail.com > 主题: Re: [PATCH] bcache: remove the redundant judgment on bi_size > > On 8/29/21 12:49 PM, Fengfei Xi wrote: >> The bi_size is unsigned int type data not less than 0, so we can >> directly add bi_size without extra judgment >> >> Signed-off-by: Fengfei Xi <xi.fengfei@h3c.com> > NACK. The check is necessary to avoid redundant and unnecessary memory write. > > Coly Li > >> --- >> drivers/md/bcache/request.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c >> index 6d1de889b..2788eec3a 100644 >> --- a/drivers/md/bcache/request.c >> +++ b/drivers/md/bcache/request.c >> @@ -423,9 +423,7 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio) >> add_sequential(task); >> i->sequential = 0; >> found: >> - if (i->sequential + bio->bi_iter.bi_size > i->sequential) >> - i->sequential += bio->bi_iter.bi_size; >> - >> + i->sequential += bio->bi_iter.bi_size; >> i->last = bio_end_sector(bio); >> i->jiffies = jiffies + msecs_to_jiffies(5000); >> task->sequential_io = i->sequential;
| |