netdev - Re: [PATCH net-next 1/4] i40e: avoid spin loop in i40e_asq_send

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: <20211028072607.4db76c84@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 28 Oct 2021 07:26:07 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jörn Engel <joern@...estorage.com>
Cc:     Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
        Caleb Sander <csander@...estorage.com>, netdev@...r.kernel.org,
        sassmann@...hat.com, Tony Brelinski <tony.brelinski@...el.com>
Subject: Re: [PATCH net-next 1/4] i40e: avoid spin loop in
 i40e_asq_send_command()

On Thu, 28 Oct 2021 04:49:03 -0700 Jörn Engel wrote:
> On Wed, Oct 27, 2021 at 09:01:03AM -0700, Jakub Kicinski wrote:
> > On Mon, 25 Oct 2021 10:55:05 -0700 Tony Nguyen wrote:  
> > > +			cond_resched();
> > >  			udelay(50);  
> > 
> > Why not switch to usleep_range() if we can sleep here?  
> 
> Looking at usleep_range() vs. udelay(), I wonder if there is still a
> hidden reason to prefer udelay().  Basically, if you typically want
> short delays like the 50µs above, going to sleep will often result in
> much longer delays, 1ms or higher.

Right, if you call sleep you always yield so another process can kick
in and consume its scheduler slice before it lets you back in.

How much does the FW typically take to respond? If we really care about
latency 50us already sounds like a pretty coarse granularity.

Also if cond_resched() fired doing the delay is probably a waste of
time:

	if (!cond_resched())
		usleep/udelay

> I can easily see situations where multiple calls to udelay(50) are
> fine while multiple calls to usleep_range() will cause timeouts.

The status of the command is re-checked after the loop, sleeping too
long should not cause timeouts here.

> Is that a known problem and do we have good heuristics when to prefer
> one over the other?

I'm not aware of any.