On Thu, May 23, 2024 at 8:41 PM Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
>
>
> 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;
> }
This can be reduced to
return (struct bio_vec){ };
>
> > > @@ -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
> >
>
>
--
Anuj Gupta
