[RFC] nvme-tcp: Handle number of queue changes

Daniel Wagner dwagner at suse.de
Thu Aug 18 00:06:06 PDT 2022


On Wed, Aug 17, 2022 at 02:28:12PM +0300, Sagi Grimberg wrote:
> 
> > On reconnect, the number of queues might have changed. In the case
> > where the old number of queues is lower than the new one, we try to
> > restart the new number of queues. At this point though, the tagset
> > is has not yet been updated and the operations fails.
> > 
> > Thus, on reconnect only start only the old number of queues, then update
> > the tag set and then start the rest of the newly added queues.
> 
> I agree this change is needed. The same issue exists in rdma and
> possibly others...

Ok, I'll take a look.

> > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> > index e82dcfcda29b..177d2f5d9047 100644
> > --- a/drivers/nvme/host/tcp.c
> > +++ b/drivers/nvme/host/tcp.c
> > @@ -1671,6 +1671,9 @@ static int nvme_tcp_start_queue(struct nvme_ctrl *nctrl, int idx)
> >   	struct nvme_tcp_ctrl *ctrl = to_tcp_ctrl(nctrl);
> >   	int ret;
> > +	if (test_bit(NVME_TCP_Q_LIVE, &ctrl->queues[idx].flags))
> > +		return 0;
> > +
> 
> How about we make concise calls to queue starts instead of
> making this re-entrant safe... Although this does follow
> the same model as pci...

I am also not too fond about this version here but I was at least sure
it works correctly. I see what I can do.

> > +	dev_info(nctrl->device, "nr_hw_queues %d queue_count %d qcnt %d\n",
> > +	         ctrl->tag_set.nr_hw_queues, nctrl->queue_count, qcnt);
> 
> No need for this print as info, its just spamming...

Sure, was just for the RFC here. Will drop it.

> Shouldn't we do the same for the create itself? Is there any chance that
> we will leave a tcp connection idle in any situation?

No idea, though would this not change how we handle the writes to
/dev/fabrics? Currently, we block and return success or fail.




More information about the Linux-nvme mailing list