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

public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed

* 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@lists.infradead.org, kbusch@kernel.org, hch@lst.de,
	james.smart@broadcom.com

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

* Re: [PATCH V6] nvme-core: use xarray for ctrl ns tracking
  2021-06-09 20:48   ` [PATCH V6] nvme-core: use xarray for ctrl ns tracking Chaitanya Kulkarni
@ 2021-06-10  3:53     ` Chaitanya Kulkarni
  2021-06-15 16:27     ` hch
  1 sibling, 0 replies; 3+ messages in thread
From: Chaitanya Kulkarni @ 2021-06-10  3:53 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: linux-nvme@lists.infradead.org, kbusch@kernel.org, hch@lst.de,
	james.smart@broadcom.com

On 6/9/21 13:48, Chaitanya Kulkarni wrote:
>>>   
>>>   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?
>>

We insert the code ns into xarray with nsid as index so there is 1:1 mapping
between the ns, nsid, and the array index. Given that it is an array in
iteration it always starts with the first element. Hence no sorting is
needed.

To check the correctness of the code (with multipath enabled) I used the
quick
debug.patch [1] and here is the the output [2].

nvme: print all the ns in id-ns from xarray

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 6fdbb0a22f3b..1f33fd46bf34 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1425,6 +1425,8 @@ static int nvme_identify_ns(struct nvme_ctrl
*ctrl, unsigned nsid,
 {
        struct nvme_command c = { };
        int error;
+       struct nvme_ns *ns;
+       unsigned long idx;
 
        /* gcc-4.4.4 (at least) has issues with initializers and anon
unions */
        c.identify.opcode = nvme_admin_identify;
@@ -1452,6 +1454,10 @@ static int nvme_identify_ns(struct nvme_ctrl
*ctrl, unsigned nsid,
            !memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
                memcpy(ids->nguid, (*id)->nguid, sizeof(ids->nguid));
 
+       xa_for_each(&ctrl->namespaces, idx, ns)
+               pr_info("%s %d nsid %d\n", __func__, __LINE__,
ns->head->ns_id);
+
+       pr_info("%s %d------------------------------", __func__, __LINE__);
        return 0;
 
 out_free_id:


Creation of the bdev controller with xarray (/dev/nvme1) and using that
as an
input to the passthru controller (/dev/nvme2), iterating over the xarray and
printing all the elements. All the ns are printed in sorted manner for the
passthru controller :-

[  229.877166] nvmet: creating controller 2 for subsystem pt-nqn for NQN
nqn.2014-08.org.nvmexpress:uuid:e4cfc949-8f19-4db2-a232-ab360b79204a.
[  229.880143] nvme nvme2: creating 64 I/O queues.
[  229.901741] nvme nvme2: new ctrl: "pt-nqn"
[  229.902230] nvme_identify_ns 1460------------------------------
[  229.932624] nvme_identify_ns 1458 nsid 1
[  229.932634] nvme_identify_ns 1460------------------------------
[  229.961368] nvme_identify_ns 1458 nsid 1
[  229.961375] nvme_identify_ns 1458 nsid 2
[  229.961381] nvme_identify_ns 1460------------------------------
[  229.986053] nvme_identify_ns 1458 nsid 1
[  229.986060] nvme_identify_ns 1458 nsid 2
[  229.986063] nvme_identify_ns 1458 nsid 3
[  229.986068] nvme_identify_ns 1460------------------------------
[  230.010679] nvme_identify_ns 1458 nsid 1
[  230.010686] nvme_identify_ns 1458 nsid 2
[  230.010689] nvme_identify_ns 1458 nsid 3
[  230.010691] nvme_identify_ns 1458 nsid 4
[  230.010696] nvme_identify_ns 1460------------------------------
[  230.034748] nvme_identify_ns 1458 nsid 1
[  230.034755] nvme_identify_ns 1458 nsid 2
[  230.034758] nvme_identify_ns 1458 nsid 3
[  230.034761] nvme_identify_ns 1458 nsid 4
[  230.034763] nvme_identify_ns 1458 nsid 5
[  230.034769] nvme_identify_ns 1460------------------------------
[  230.058996] nvme_identify_ns 1458 nsid 1
[  230.059003] nvme_identify_ns 1458 nsid 2
[  230.059006] nvme_identify_ns 1458 nsid 3
[  230.059009] nvme_identify_ns 1458 nsid 4
[  230.059012] nvme_identify_ns 1458 nsid 5
[  230.059014] nvme_identify_ns 1458 nsid 6
[  230.059019] nvme_identify_ns 1460------------------------------
[  230.083433] nvme_identify_ns 1458 nsid 1
[  230.083440] nvme_identify_ns 1458 nsid 2
[  230.083443] nvme_identify_ns 1458 nsid 3
[  230.083446] nvme_identify_ns 1458 nsid 4
[  230.083450] nvme_identify_ns 1458 nsid 5
[  230.083452] nvme_identify_ns 1458 nsid 6
[  230.083455] nvme_identify_ns 1458 nsid 7
[  230.083459] nvme_identify_ns 1460------------------------------
[  230.107702] nvme_identify_ns 1458 nsid 1
[  230.107709] nvme_identify_ns 1458 nsid 2
[  230.107744] nvme_identify_ns 1458 nsid 3
[  230.107748] nvme_identify_ns 1458 nsid 4
[  230.107751] nvme_identify_ns 1458 nsid 5
[  230.107754] nvme_identify_ns 1458 nsid 6
[  230.107756] nvme_identify_ns 1458 nsid 7
[  230.107758] nvme_identify_ns 1458 nsid 8
[  230.107763] nvme_identify_ns 1460------------------------------
[  230.132132] nvme_identify_ns 1458 nsid 1
[  230.132139] nvme_identify_ns 1458 nsid 2
[  230.132141] nvme_identify_ns 1458 nsid 3
[  230.132144] nvme_identify_ns 1458 nsid 4
[  230.132147] nvme_identify_ns 1458 nsid 5
[  230.132149] nvme_identify_ns 1458 nsid 6
[  230.132152] nvme_identify_ns 1458 nsid 7
[  230.132154] nvme_identify_ns 1458 nsid 8
[  230.132156] nvme_identify_ns 1458 nsid 9



Now disable the 5th and 8th namespace from target that should leave nid
5 & 8
but rest of the namespaces should be printed in the ascending order
in the ns rescan sequence :-


# echo 0 > /sys/kernel/config/nvmet/subsystems/x/namespaces/5/enable
# echo 0 > /sys/kernel/config/nvmet/subsystems/x/namespaces/8/enable

[  620.836412] nvme nvme1: rescanning namespaces.
[  620.836985] nvme_identify_ns 1458 nsid 1
[  620.836993] nvme_identify_ns 1458 nsid 2
[  620.836998] nvme_identify_ns 1458 nsid 3
[  620.837003] nvme_identify_ns 1458 nsid 4
[  620.837007] nvme_identify_ns 1458 nsid 6
[  620.837012] nvme_identify_ns 1458 nsid 7
[  620.837017] nvme_identify_ns 1458 nsid 8
[  620.837021] nvme_identify_ns 1458 nsid 9
[  620.837026] nvme_identify_ns 1458 nsid 10
[  620.837034] nvme_identify_ns 1460------------------------------
[  620.857901] nvme_identify_ns 1458 nsid 1
[  620.857909] nvme_identify_ns 1458 nsid 2
[  620.857913] nvme_identify_ns 1458 nsid 3
[  620.857929] nvme_identify_ns 1458 nsid 4
[  620.857933] nvme_identify_ns 1458 nsid 6
[  620.857937] nvme_identify_ns 1458 nsid 7
[  620.857940] nvme_identify_ns 1458 nsid 8
[  620.857944] nvme_identify_ns 1458 nsid 9
[  620.857948] nvme_identify_ns 1458 nsid 10
[  620.857956] nvme_identify_ns 1460------------------------------
[  620.873913] nvme_identify_ns 1458 nsid 1
[  620.873921] nvme_identify_ns 1458 nsid 2
[  620.873925] nvme_identify_ns 1458 nsid 3
[  620.873939] nvme_identify_ns 1458 nsid 4
[  620.873943] nvme_identify_ns 1458 nsid 6
[  620.873947] nvme_identify_ns 1458 nsid 7
[  620.873951] nvme_identify_ns 1458 nsid 8
[  620.873955] nvme_identify_ns 1458 nsid 9
[  620.873960] nvme_identify_ns 1458 nsid 10
[  620.873969] nvme_identify_ns 1460------------------------------
[  620.887976] nvme_identify_ns 1458 nsid 1
[  620.887983] nvme_identify_ns 1458 nsid 2
[  620.887988] nvme_identify_ns 1458 nsid 3
[  620.887991] nvme_identify_ns 1458 nsid 4
[  620.887995] nvme_identify_ns 1458 nsid 6
[  620.887999] nvme_identify_ns 1458 nsid 7
[  620.888002] nvme_identify_ns 1458 nsid 8
[  620.888006] nvme_identify_ns 1458 nsid 9
[  620.888010] nvme_identify_ns 1458 nsid 10
[  620.888018] nvme_identify_ns 1460------------------------------
[  620.901911] nvme_identify_ns 1458 nsid 1
[  620.901919] nvme_identify_ns 1458 nsid 2
[  620.901924] nvme_identify_ns 1458 nsid 3
[  620.901928] nvme_identify_ns 1458 nsid 4
[  620.901932] nvme_identify_ns 1458 nsid 6
[  620.901936] nvme_identify_ns 1458 nsid 7
[  620.901941] nvme_identify_ns 1458 nsid 8
[  620.901945] nvme_identify_ns 1458 nsid 9
[  620.901948] nvme_identify_ns 1458 nsid 10
[  620.901957] nvme_identify_ns 1460------------------------------
[  620.919913] nvme_identify_ns 1458 nsid 1
[  620.919921] nvme_identify_ns 1458 nsid 2
[  620.919926] nvme_identify_ns 1458 nsid 3
[  620.919930] nvme_identify_ns 1458 nsid 4
[  620.919934] nvme_identify_ns 1458 nsid 6
[  620.919938] nvme_identify_ns 1458 nsid 7
[  620.919942] nvme_identify_ns 1458 nsid 8
[  620.919946] nvme_identify_ns 1458 nsid 9
[  620.919950] nvme_identify_ns 1458 nsid 10
[  620.919959] nvme_identify_ns 1460------------------------------
[  620.933910] nvme_identify_ns 1458 nsid 1
[  620.933918] nvme_identify_ns 1458 nsid 2
[  620.933923] nvme_identify_ns 1458 nsid 3
[  620.933927] nvme_identify_ns 1458 nsid 4
[  620.933931] nvme_identify_ns 1458 nsid 6
[  620.933935] nvme_identify_ns 1458 nsid 7
[  620.933939] nvme_identify_ns 1458 nsid 8
[  620.933943] nvme_identify_ns 1458 nsid 9
[  620.933947] nvme_identify_ns 1458 nsid 10
[  620.933956] nvme_identify_ns 1460------------------------------
[  621.022320] nvme_identify_ns 1458 nsid 1
[  621.022330] nvme_identify_ns 1458 nsid 2
[  621.022335] nvme_identify_ns 1458 nsid 3
[  621.022340] nvme_identify_ns 1458 nsid 4
[  621.022344] nvme_identify_ns 1458 nsid 6
[  621.022348] nvme_identify_ns 1458 nsid 7
[  621.022352] nvme_identify_ns 1458 nsid 9
[  621.022355] nvme_identify_ns 1458 nsid 10

Hope this answers your questions.



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

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

* Re: [PATCH V6] nvme-core: use xarray for ctrl ns tracking
  2021-06-09 20:48   ` [PATCH V6] nvme-core: use xarray for ctrl ns tracking Chaitanya Kulkarni
  2021-06-10  3:53     ` Chaitanya Kulkarni
@ 2021-06-15 16:27     ` hch
  1 sibling, 0 replies; 3+ messages in thread
From: hch @ 2021-06-15 16:27 UTC (permalink / raw)
  To: Chaitanya Kulkarni
  Cc: Sagi Grimberg, linux-nvme@lists.infradead.org, kbusch@kernel.org,
	hch@lst.de, james.smart@broadcom.com

On Wed, Jun 09, 2021 at 08:48:10PM +0000, Chaitanya Kulkarni wrote:
> 
> 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.

Or document why this is changed in the commit log, that is what it is
for.

_______________________________________________
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

end of thread, other threads:[~2021-06-15 20:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20210608024247.35286-1-chaitanya.kulkarni@wdc.com>
     [not found] ` <dbab5916-a6de-5cd0-4bb5-b06d0349df0b@grimberg.me>
2021-06-09 20:48   ` [PATCH V6] nvme-core: use xarray for ctrl ns tracking Chaitanya Kulkarni
2021-06-10  3:53     ` Chaitanya Kulkarni
2021-06-15 16:27     ` hch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox