Re: blktests nvme/062 lockdep — defer TLS inlin e send to io_work?
Hannes Reinecke <[email protected]>
| Newsgroups | org.infradead.lists.linux-nvme |
|---|---|
| Message-ID | <[email protected]> |
On 8/18/26 7:38 AM, Shin'ichiro Kawasaki wrote:
> On Jul 01, 2026 / 09:50, Xixin Liu wrote:
>> Hi Hannes,
>>
>> On Mon, 29 Jun 2026, Hannes Reinecke wrote:
>>> The most likely testcase showing regressions would be having as many
>>> jobs as your machine has CPUs and use qd=1.
>>> Just a single job will not be enough to show a noticeable difference.
>>
>> I re-ran the TLS perf comparison with one job per CPU and queue depth 1
>> on a new test host. Please treat the results below as the authoritative
>> perf data for this workload.
>>
>> Test setup
>> ----------
>> aarch64 server, 128 CPUs, linux-next 7.2.0-rc1-*-perf-dirty
>>
>> Same-machine nvmet-tcp with TLS; target backing store is a 32G file on a
>> local NVMe SSD. fio runs against the connected nvme-tcp namespace.
>>
>> Compared inline send_all (baseline) vs deferring TLS inline send (io_work).
>> fio randwrite 4k, direct I/O, 30s per run, 15s warmup discarded before
>> each mode (10 runs total):
>>
>> - iodepth=1, numjobs=128:
>> baseline 54539.4 IOPS, mean 2.34ms, p99 6.02ms
>> patched 54529.0 IOPS, mean 2.34ms, p99 5.99ms
>>
>> - iodepth=128, numjobs=8:
>> baseline 54632.6 IOPS, mean 18.74ms, p99 47.08ms
>> patched 54633.6 IOPS, mean 18.74ms, p99 46.92ms
>>
>> So on a 128-CPU box with the workload you called out, deferring TLS inline
>> send still looks neutral across 10 consecutive runs after warmup.
>
> Xixin, thank you for the effort. The nvme/062 lockdep WARN still exists with
> v7.2 kernel. Per your suggestion, I applied the patch below [*] on top of v7.2
> kernel, and confirmed the nvme/062 goes away. This is great from blktests user
> point of view.
>
> Hannes, Xixin provided the latency numbers above. Are they enough to show no
> impact of the fix patch? Or do you think more workload variations should be
> covered?
>
>
> [*] trial patch
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index ba5c7b3e2a7c..122d815270b9 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -411,6 +411,7 @@ static inline void nvme_tcp_queue_request(struct nvme_tcp_request *req,
> * are on the same cpu, so we don't introduce contention.
> */
> if (queue->io_cpu == raw_smp_processor_id() &&
> + !nvme_tcp_queue_tls(queue) &&
> empty && mutex_trylock(&queue->send_mutex)) {
> nvme_tcp_send_all(queue);
> mutex_unlock(&queue->send_mutex);
I actually thought we could do away with this 'if' condition completely.
But that I guess requires more analysis. So for now:
Reviewed-by: Hannes Reinecke <[email protected]>
Can you send a formal patch?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich