RE:(2) [PATCH v2 05/14] block: blk-merge: fix to add the number of integrity segments to the request twice

From: Jinyoung CHOI
Date: Tue May 16 2023 - 09:24:34 EST


>The subject looks a bit odd, I think you're trying to say:
>
>"do not add the number of integrity segments to the request twice"
>
>based on the actual patch, is this correct?
>

Yes. I will fix it.

>> +static inline bool blk_integrity_bypass_check(struct request *req,
>> +                                              struct bio *bio)
>> +{
>> +        return blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL;
>> +}
>
>No need for the explicit comparisms, this could just be:
>
>        return !blk_integrity_rq(req) && !bio_integrity(bio);
>
>and given that it just has two callers I'm not sure the helper is
>all that useful to start with.

There are many conditional sentences like that, so I left them for unity,
If it's okay to change, I'll do so.

>> +static bool __blk_integrity_mergeable(struct request_queue *q,
>> +                                      struct request *req, struct bio *bio)
>> +{
>> +        if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
>> +                return false;
>> +
>> +        if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags)
>> +                return false;
>> +
>> +        return true;
>> +}
>> +
>> +bool blk_integrity_mergeable(struct request_queue *q, struct request *req,
>> +                             struct bio *bio)
>> +{
>> +        if (blk_integrity_bypass_check(req, bio))
>> +                return true;
>> +
>> +        return __blk_integrity_mergeable(q, req, bio);
>> +}
>
>Similarly here, I'm not even sure we need all these helpers.  I supect
>the code would become more readable by dropping these helpers and just
>making the checks explicitlẏ

OK. I will drop this.

Best Regards,
Jinyoung.