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.