linux-kernel - Re: [PATCH] PCI: Replace msleep(2) with usleep

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] [day] [month] [year] [list]

Message-ID: <20250820180651.GA631082@bhelgaas>
Date: Wed, 20 Aug 2025 13:06:51 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Hans Zhang <18255117159@....com>
Cc: bhelgaas@...gle.com, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: Replace msleep(2) with usleep_range() for precise
 delay

On Thu, Aug 21, 2025 at 12:09:44AM +0800, Hans Zhang wrote:
> The msleep(2) may sleep up to 20ms due to timer granularity, which can
> cause unnecessary delays. According to PCI spec v3.0 7.6.4.2, the minimum
> Trst is 1ms and we doubled that to 2ms to meet the requirement. Using
> usleep_range(2000, 2001) provides a more precise delay of exactly 2ms.
> ...

Please cite a recent spec version, i.e., r7.0.  I see this probably
came from the comment at the change; I wouldn't object to updating
the comment, too.

> WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
> #4630: FILE: drivers/pci/pci.c:4630:
> +		msleep(1);
> ...
> WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see function description of msleep().
> #3970: FILE: drivers/pci/quirks.c:3970:
> +		msleep(10);

> +++ b/drivers/pci/pci.c
> @@ -4967,7 +4967,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
>  	 * PCI spec v3.0 7.6.4.2 requires minimum Trst of 1ms.  Double
>  	 * this to 2ms to ensure that we meet the minimum requirement.
>  	 */
> -	msleep(2);
> +	usleep_range(2000, 2001);

IMO the most valuable thing here would be to replace the hard-coded
"2" with some kind of #define explicitly tied to the spec.  Similarly
for the other cases.

There is some concern [1] about places that say "msleep(1)" but
actually rely on the current behavior of a longer sleep.

Apart from that concern, I think fsleep() would be my first choice.
usleep_range(x, x+1) defeats the purpose of the range, which is to
reduce interrupts; see 5e7f5a178bba ("timer: Added usleep_range
timer").

Bjorn

[1] https://lore.kernel.org/all/20070809001640.ec2f3bfb.akpm@linux-foundation.org/