Netdev List
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@purestorage.com>
To: Ariel Elior <aelior@marvell.com>
Cc: Caleb Sander <csander@purestorage.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"GR-everest-linux-l2@marvell.com"
	<GR-everest-linux-l2@marvell.com>
Subject: Re: [EXT] Re: [PATCH] qed: avoid spin loops in _qed_mcp_cmd_and_union()
Date: Thu, 28 Oct 2021 07:59:01 -0700	[thread overview]
Message-ID: <YXq6tTWTdiSPM/wr@cork> (raw)
In-Reply-To: <PH0PR18MB465585F216AEC6E441B7E0E4C4869@PH0PR18MB4655.namprd18.prod.outlook.com>

On Thu, Oct 28, 2021 at 05:47:10AM +0000, Ariel Elior wrote:
>
> Indeed this function sends messages to the management FW, and may
> be invoked both from atomic contexts and from non atomic ones.
> CAN_SLEEP indicated whether it is permissible in the context from which
> it was invoked to sleep.

That is a rather unfortunate pattern.  I understand the desire for code
reuse, but the result is often to use udelay-loops that can take
seconds.  In case of unresponsive firmware you tend to always hit the
timeouts and incur maximum latency.

Since the scheduler is blocked on the local CPU for the time of the spin
loop and won't even bother migrating high-priority threads away - the
assumption is that the current thread will not loop for a long time -
the result can be pretty bad for latency-sensitive code.  You cannot
guarantee any latencies below the timeout of those loops, essentially.

Having a flag or some other means to switch between sleeping and
spinning would help to reduce the odds.  Avoiding calls from atomic
contexts would help even more.  Ideally I would like to remove all
such calls.  The only legitimate exceptions should be those handling
with high-volume packet RX/TX and never involve long-running loops.
Anything else can be handled from a kworker or similar.  If a 1s loop is
acceptable, waiting a few ms for the scheduler must also be acceptable.

Jörn

--
If a problem has a hardware solution, and a software solution,
do it in software.
-- Arnd Bergmann

      reply	other threads:[~2021-10-28 14:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-27 21:45 [PATCH] qed: avoid spin loops in _qed_mcp_cmd_and_union() Caleb Sander
2021-10-27 22:25 ` Eric Dumazet
2021-10-28  0:20   ` Caleb Sander
2021-10-28  5:47     ` [EXT] " Ariel Elior
2021-10-28 14:59       ` Jörn Engel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YXq6tTWTdiSPM/wr@cork \
    --to=joern@purestorage.com \
    --cc=GR-everest-linux-l2@marvell.com \
    --cc=aelior@marvell.com \
    --cc=csander@purestorage.com \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox