On Thu, 23 May 2024, Jens Axboe wrote:
> On 5/23/24 8:58 AM, Mikulas Patocka wrote:
> > Here I'm resending the patch with the function rq_integrity_vec removed if
> > CONFIG_BLK_DEV_INTEGRITY is not defined.
>
> That looks better - but can you please just post a full new series,
> that's a lot easier to deal with and look at than adding a v2 of one
> patch in the thread.
OK, I'll post both patches.
> > @@ -853,16 +855,20 @@ static blk_status_t nvme_prep_rq(struct
> > goto out_free_cmd;
> > }
> >
> > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > if (blk_integrity_rq(req)) {
> > ret = nvme_map_metadata(dev, req, &iod->cmd);
> > if (ret)
> > goto out_unmap_data;
> > }
> > +#endif
>
> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) && blk_integrity_rq(req)) {
>
> ?
That wouldn't work, because the calls to rq_integrity_vec need to be
eliminated by the preprocessor.
Should I change rq_integrity_vec to this? Then, we could get rid of the
ifdefs and let the optimizer remove all calls to rq_integrity_vec.
static inline struct bio_vec rq_integrity_vec(struct request *rq)
{
struct bio_vec bv = { };
return bv;
}
> > @@ -962,12 +968,14 @@ static __always_inline void nvme_pci_unm
> > struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
> > struct nvme_dev *dev = nvmeq->dev;
> >
> > +#ifdef CONFIG_BLK_DEV_INTEGRITY
> > if (blk_integrity_rq(req)) {
> > struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
>
> Ditto
>
> > Index: linux-2.6/include/linux/blk-integrity.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/blk-integrity.h
> > +++ linux-2.6/include/linux/blk-integrity.h
> > @@ -109,11 +109,11 @@ static inline bool blk_integrity_rq(stru
> > * Return the first bvec that contains integrity data. Only drivers that are
> > * limited to a single integrity segment should use this helper.
> > */
> > -static inline struct bio_vec *rq_integrity_vec(struct request *rq)
> > +static inline struct bio_vec rq_integrity_vec(struct request *rq)
> > {
> > - if (WARN_ON_ONCE(queue_max_integrity_segments(rq->q) > 1))
> > - return NULL;
> > - return rq->bio->bi_integrity->bip_vec;
> > + WARN_ON_ONCE(queue_max_integrity_segments(rq->q) > 1);
> > + return mp_bvec_iter_bvec(rq->bio->bi_integrity->bip_vec,
> > + rq->bio->bi_integrity->bip_iter);
> > }
>
> Not clear why the return on integrity segments > 1 is removed?
Because we can't return NULL. But I can leave it there, print a warning
and return the first vector.
Mikulas
> --
> Jens Axboe
>
