Re: [PATCH V6] nvme-core: use xarray for ctrl ns tracking

* Re: [PATCH V6] nvme-core: use xarray for ctrl ns tracking
       [not found] ` <dbab5916-a6de-5cd0-4bb5-b06d0349df0b@grimberg.me>
@ 2021-06-09 20:48   ` Chaitanya Kulkarni
  2021-06-10  3:53     ` Chaitanya Kulkarni
  2021-06-15 16:27     ` hch
  0 siblings, 2 replies; 3+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-09 20:48 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: linux-nvme, kbusch, hch, james.smart

Sagi,

On 6/8/21 16:25, Sagi Grimberg wrote:
>
> On 6/7/21 7:42 PM, Chaitanya Kulkarni wrote:
>> This patch replaces the ctrl->namespaces tracking from linked list to
>> xarray for better ns-mgmt on the host side. For host side
>> nvme_find_get_ns() falls into the fast path for NVMeOF passthru target.
>> This allows us to have better performance for NVMeOF passthru backend
>> since XArray has shows better performance numbers over having
>> a combination of read-write semapore read + linked list in the
>> nvme_find_get_ns() to find namespace in I/O patch from nsid specified
>> in the nvme_rw_cmd.
>>
[...]
>> @@ -3839,6 +3829,7 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>>   	list_del_rcu(&ns->siblings);
>>   	mutex_unlock(&ns->ctrl->subsys->lock);
>>   
>> +	xa_erase(&ns->ctrl->namespaces, ns->head->ns_id);
>>   	synchronize_rcu(); /* guarantee not available in head->list */
>>   	nvme_mpath_clear_current_path(ns);
>>   	synchronize_srcu(&ns->head->srcu); /* wait for concurrent submissions */
>> @@ -3852,10 +3843,6 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>>   			blk_integrity_unregister(ns->disk);
>>   	}
>>   
>> -	down_write(&ns->ctrl->namespaces_rwsem);
>> -	list_del_init(&ns->list);
>> -	up_write(&ns->ctrl->namespaces_rwsem);
>> -
> Why did this change position? this sort of magic makes me scared
> of this patch...

This is to void the another call to the synchronize_rcu(),

I'll add a second call to synchronize_rcu() and keep the original
position.

>
>>   	nvme_mpath_check_last_path(ns);
>>   	nvme_put_ns(ns);
>>   }
[...]
>>   
>>   static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
>> @@ -4068,10 +4051,6 @@ static void nvme_scan_work(struct work_struct *work)
>>   	if (nvme_scan_ns_list(ctrl) != 0)
>>   		nvme_scan_ns_sequential(ctrl);
>>   	mutex_unlock(&ctrl->scan_lock);
>> -
>> -	down_write(&ctrl->namespaces_rwsem);
>> -	list_sort(NULL, &ctrl->namespaces, ns_cmp);
>> -	up_write(&ctrl->namespaces_rwsem);
> Is xarray sorted by nsid? ana updates rely on ctrl->namespaces to be
> sorted. Where is nvme_update_ana_state change anyway? you tested this
> without CONFIG_NVME_MULTIPATH?
>

Looks like CONFIG_NVME_MULTIPATH disabled in my tree currently.

I've missed the multipath.c in this version that was there in
previous version [1].

Let send out a new version.

[1] https://lists.infradead.org/pipermail/linux-nvme/2020-July/018242.html

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 3+ messages in thread