On Tue, Sep 03, 2019 at 01:21:59PM -0600, Jens Axboe wrote: > On 9/3/19 1:11 PM, Sagi Grimberg wrote: > > > >> + if (blk_integrity_rq(req) && req_op(req) == REQ_OP_READ && > >> + error == BLK_STS_OK) > >> + t10_pi_complete(req, > >> + nr_bytes / queue_logical_block_size(req->q)); > >> + > > > > div in this path? better to use >> ilog2(block_size). > > > > Also, would be better to have a wrapper in place like: > > > > static inline unsigned short blk_integrity_interval(struct request *rq) > > { > > return queue_logical_block_size(rq->q); > > } > > If it's a hot path thing that matters, I'd strongly suggest to add > a queue block size shift instead. Make that a protection_interval_shift, please. While that currently is the same as the logical block size the concepts are a little different, and that makes it clear. Except for that this patch looks very nice to me, it is great to avoid having drivers to deal with the PI remapping.