[PATCH 1/4] nvme-rdma: use reconnect_work for initial connect

[hare@suse.de (Hannes Reinecke)]

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 <hare at suse.com>
>> ---
>> ? 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)