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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 May 2023 22:24:12 +0900
From:   Jinyoung CHOI <j-young.choi@...sung.com>
To:     "hch@....de" <hch@....de>
CC:     "axboe@...nel.dk" <axboe@...nel.dk>,
        "kbusch@...nel.org" <kbusch@...nel.org>,
        "sagi@...mberg.me" <sagi@...mberg.me>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "johannes.thumshirn@....com" <johannes.thumshirn@....com>,
        "kch@...dia.com" <kch@...dia.com>,
        "willy@...radead.org" <willy@...radead.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE:(2) [PATCH v2 05/14] block: blk-merge: fix to add the number of
 integrity segments to the request twice

>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.