From mboxrd@z Thu Jan  1 00:00:00 1970
From: parav@mellanox.com (Parav Pandit)
Date: Mon, 13 Feb 2017 16:12:49 +0000
Subject: [PATCH] nvmet: Avoid writing fabric_ops, queue pointers on every
 request.
References: <1486507066-23168-1-git-send-email-parav@mellanox.com>
 <5bcb5982-36a6-67f5-8416-45e321235fa9@grimberg.me>
 <VI1PR0502MB3008C2598C5492B2EB4D1E55D1420@VI1PR0502MB3008.eurprd05.prod.outlook.com>
 <700fde5d-3b84-2279-4176-c871364d4b97@grimberg.me> 
Message-ID: <VI1PR0502MB3008119A87F7058CAA4B22C0D1590@VI1PR0502MB3008.eurprd05.prod.outlook.com>

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 :-) ).
> 
>