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: <4002ce40-56bc-4a89-a4bf-7da28c94f7db@163.com>
Date: Fri, 22 Aug 2025 23:53:20 +0800
From: Hans Zhang <18255117159@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
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

Dear Bjorn,

Thank you very much for your reply. I'll try to understand your meaning 
and then submit the next version.

If I haven't fully understood your meaning yet, please continue to help
correct the mistakes.

Best regards,
Hans

On 2025/8/21 02:06, Bjorn Helgaas wrote:
> 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/