[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87f86921-373d-436e-93d8-4616a57e0697@gmail.com>
Date: Wed, 24 Jan 2024 13:32:47 -0600
From: stuart hayes <stuart.w.hayes@...il.com>
To: Chaitanya Kulkarni <chaitanyak@...dia.com>,
Keith Busch <kbusch@...nel.org>, Sagi Grimberg <sagi@...mberg.me>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Jens Axboe <axboe@...nel.dk>, Christoph Hellwig <hch@....de>,
"linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>
Subject: Re: [PATCH v2] nvme_core: scan namespaces asynchronously
On 1/23/2024 2:21 PM, Chaitanya Kulkarni wrote:
> On 1/23/2024 8:37 AM, Keith Busch wrote:
>> On Mon, Jan 22, 2024 at 11:13:15AM +0200, Sagi Grimberg wrote:
>>> On 1/18/24 23:03, Stuart Hayes wrote:
>>>> @@ -3901,19 +3932,25 @@ static int nvme_scan_ns_list(struct nvme_ctrl *ctrl)
>>>> goto free;
>>>> }
>>>> + /*
>>>> + * scan list starting at list offset 0
>>>> + */
>>>> + atomic_set(&scan_state.count, 0);
>>>> for (i = 0; i < nr_entries; i++) {
>>>> u32 nsid = le32_to_cpu(ns_list[i]);
>>>> if (!nsid) /* end of the list? */
>>>> goto out;
>>>> - nvme_scan_ns(ctrl, nsid);
>>>> + async_schedule_domain(nvme_scan_ns, &scan_state, &domain);
>>>> while (++prev < nsid)
>>>> nvme_ns_remove_by_nsid(ctrl, prev);
>>>> }
>>>> + async_synchronize_full_domain(&domain);
>>
>> You mentioned async scanning was an improvement if you have 1000
>> namespaces, but wouldn't this be worse if you have very few namespaces?
>> IOW, the decision to use the async schedule should be based on
>> nr_entries, right?
>>
>
> Perhaps it's also helpful to documents the data for small number of
> namespaces, we can think of collecting data something like this:-
>
> NR Namespaces Seq Scan Async Scan
> 2
> 4
> 8
> 16
> 32
> 64
> 128
> 256
> 512
> 1024
>
> If we find that difference is not that much then we can go ahead with
> this patch, if it the difference is not acceptable to the point that it
> will regress for common setups then we can make it configurable ?
>
> -ck
>
>
I believe the only reason the async scanning should take any longer than
sync scanning is that nvme_scan_ns() has to wait on the workqueue until it
is scheduled.
Testing on my system (with pcie nvme devices with a single namespace), it
looks like it only takes a fraction of a ms (100us or less typically) for
that to happen. Then it takes 6-10ms or more for the actual namesapce scan.
So scanning asynchronously, even using a local pcie device with a single
namespace, doesn't take significantly longer. Of course I guess it might
take a bit longer on a busy system, but I wouldn't think that scanning
namespaces is a critical path where a few milliseconds would make much
difference (?). It wouldn't be too hard to make it scan synchronously if
there aren't more than, say, a couple namespaces, but my opinion is that
the minimal benefit wouldn't be worth the extra code.
Powered by blists - more mailing lists