From mboxrd@z Thu Jan 1 00:00:00 1970 From: hare@suse.de (Hannes Reinecke) Date: Tue, 21 Aug 2018 16:18:21 +0200 Subject: [PATCH 1/4] nvme-rdma: use reconnect_work for initial connect In-Reply-To: References: <20180821134329.69577-1-hare@suse.de> <20180821134329.69577-2-hare@suse.de> Message-ID: <89755a0c-0fb2-b232-ef33-a0e7f65416aa@suse.de> On 08/21/2018 04:10 PM, Max Gurtovoy wrote: > > > On 8/21/2018 4:43 PM, Hannes Reinecke wrote: >> The 'reconnect_work' function does exactly the same than we do >> for the initial connect, so we can as well schedule the workqueue >> item here and wait for it to complete. > > If we do this we should rename it to connect_work. > Sure, can do. >> >> Signed-off-by: Hannes Reinecke >> --- >> ? drivers/nvme/host/rdma.c | 20 +++++++++++++------- >> ? 1 file changed, 13 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c >> index dc042017c293..0906acbb800e 100644 >> --- a/drivers/nvme/host/rdma.c >> +++ b/drivers/nvme/host/rdma.c >> @@ -1990,19 +1990,25 @@ static struct nvme_ctrl >> *nvme_rdma_create_ctrl(struct device *dev, >> ????? changed = nvme_change_ctrl_state(&ctrl->ctrl, >> NVME_CTRL_CONNECTING); >> ????? WARN_ON_ONCE(!changed); >> ? -??? ret = nvme_rdma_setup_ctrl(ctrl, true); > > we call nvme_rdma_setup_ctrl with "new == true" here and with "new == > false" in the reconnect work. > so how does this work ? > Ah. Overlooked this. Will be fixing it up. >> -??? if (ret) >> -??????? goto out_uninit_ctrl; >> - >> -??? dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n", >> -??????? ctrl->ctrl.opts->subsysnqn, &ctrl->addr); >> - >> ????? nvme_get_ctrl(&ctrl->ctrl); >> ? ????? mutex_lock(&nvme_rdma_ctrl_mutex); >> ????? list_add_tail(&ctrl->list, &nvme_rdma_ctrl_list); >> ????? mutex_unlock(&nvme_rdma_ctrl_mutex); >> ? +??? if (!queue_delayed_work(nvme_wq, &ctrl->reconnect_work, 0)) { >> +??????? nvme_put_ctrl(&ctrl->ctrl); >> +??????? dev_err(ctrl->ctrl.device, >> +??????????? "failed to schedule initial connect\n"); >> +??????? goto out_uninit_ctrl; >> +??? } >> + >> +??? flush_delayed_work(&ctrl->reconnect_work); >> + >> +??? dev_info(ctrl->ctrl.device, "new ctrl: NQN \"%s\", addr %pISpcs\n", >> +??????? ctrl->ctrl.opts->subsysnqn, &ctrl->addr); >> + >> + >> ????? return &ctrl->ctrl; >> ? ? out_uninit_ctrl: >> > > what is the benefit here ? > We have more lines of code now :) The benefit comes with the next patches, as moving things out to a workqueue allows us to implement the 'async_connect' option. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare at suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N?rnberg)