[PATCH] nvmet: Avoid writing fabric_ops, queue pointers on every request.

[Parav Pandit]

Hi James,

Does this change look fine for FC?
I do not have infrastructure to test the FC side.

Parav

> -----Original Message-----
> From: Parav Pandit
> Sent: Wednesday, February 8, 2017 2:03 PM
> To: 'Sagi Grimberg' <sagi at grimberg.me>; hch at lst.de;
> james.smart at broadcom.com; linux-nvme at lists.infradead.org
> Subject: RE: [PATCH] nvmet: Avoid writing fabric_ops, queue pointers on
> every request.
> 
> Hi Sagi,
> 
> > -----Original Message-----
> > From: Sagi Grimberg [mailto:sagi at grimberg.me]
> > Sent: Wednesday, February 8, 2017 12:18 PM
> > To: Parav Pandit <parav at mellanox.com>; hch at lst.de;
> > james.smart at broadcom.com; linux-nvme at lists.infradead.org
> > Subject: Re: [PATCH] nvmet: Avoid writing fabric_ops, queue pointers
> > on every request.
> >
> >
> > >>> Additionally this patch further avoid nvme cq and sq pointer
> > >>> initialization for every request during every request processing
> > >>> for rdma because nvme queue linking occurs during queue allocation
> > >>> time for AQ and IOQ.
> > >>
> > >> This breaks SRQ mode where every nvmet_rdma_cmd serves different
> > >> queues in it's lifetime..
> > >
> > > I fail to understand that.
> > > nvmet_rdma_create_queue_ib() is call for as many QPs as we create;
> > > not
> > based on number of SRQs we create.
> >
> > Correct.
> >
> > > nvmet_rdma_queue stores cq and sq.
> >
> > Correct.
> >
> > > So there are as many cq and sq on the fabric side as QPs for
> > > fabric_connect
> > command is called.
> > > queue is pulled out of cq context on which we received the command.
> > > SRQ is just a place shared among this nvme queues to share the RQ
> > > buffer,
> > right?
> >
> > Correct too, but we then assign the queue to the command, which is the
> > context of the received SQE (maybe with in-capsule data). For the SRQ
> > case we allocate the commands and pre-post them (before we have any
> > queues), they are absolutely not bound to a given queue, they can't
> actually.
> >
> > So for each new recv completion, the command context is now bound to
> > the queue that it completed on, so it can be bound to different queues
> > in its life- time.
> 
> Sorry, I am still not getting it.
> nvmet_rdma_rsp structure contains nvmet_req.
> nvmet_rdma_rsp (and so nvmet_req) are per QP allocations.
> cq and sq pointers are initialized inside nvmet_req.
> 
> nvmet_rdma_cmd is per RQ/SRQ allocation.
> When we do recv_done(), we bind rsp structure to cmd (cmd can be from RQ
> or SRQ).
> So I believe this is still good.
> If we have cq and sq pointer inside the nvmet_rdma_cmd than I can
> understand that it can break.
> 
> On a side note: I tested the patch with use_srq flag (but didn't publish its
> performance numbers as they awaiting your per core SRQ fixes to match
> regular RQ numbers :-) ).
> 
>