linux-kernel - Re: [PATCH] nvme-tcp: wait socket wmem to drain in queue stop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250417051241.b3cnkp5svew3zhud@xldev1604-tmpl.dev.purestorage.com>
Date: Wed, 16 Apr 2025 23:12:41 -0600
From: Michael Liang <mliang@...estorage.com>
To: Chaitanya Kulkarni <chaitanyak@...dia.com>
Cc: Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>,
	Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
	Mohamed Khalfella <mkhalfella@...estorage.com>,
	Randy Jennings <randyj@...estorage.com>,
	"linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] nvme-tcp: wait socket wmem to drain in queue stop

On Tue, Apr 08, 2025 at 09:00:00PM +0000, Chaitanya Kulkarni wrote:
> On 4/4/25 22:48, Michael Liang wrote:
> > +static void nvme_tcp_stop_queue_wait(struct nvme_tcp_queue *queue)
> > +{
> > +	int timeout = 100;
> > +
> 
> is there a guarantee that above will work for all the setups?
> using configurable timeout values helps creating more generic
> fix, do we need to consider that here ?
The value here primarily reflects the latency between __tcp_transmit_skb()
and the freeing of the skb in the TX completion path. For most scenarios,
100ms should be sufficient. While it's theoretically possible to see higher
latencies, such cases might not be typical or practical for NVMe-TCP (please
correct me if I’m wrong).

That said, I'm open to making this timeout configurable if needed—perhaps
via a module parameter?

> > +	while (timeout > 0) {
> > +		if (!sk_wmem_alloc_get(queue->sock->sk))
> > +			return;
> > +		msleep(2);
> > +		timeout -= 2;
> > +	}
> > +	dev_warn(queue->ctrl->ctrl.device,
> > +		 "qid %d: wait draining sock wmem allocation timeout\n",
> > +		 nvme_tcp_queue_id(queue));
> > +}
> > +
> 
> -ck
> 
> 

Thanks,
Michael