mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ross Cawston <ross@r-sc.ca>
To: kory.maincent@bootlin.com
Cc: netdev@vger.kernel.org, kyle.swenson@est.tech,
	github@szelinsky.de, o.rempel@pengutronix.de,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@kernel.org,
	kuba@kernel.org, pabeni@redhat.com, linux-kernel@vger.kernel.org,
	Ross Cawston <ross@r-sc.ca>
Subject: Re: [PATCH net v2] net: pse-pd: don't enable a PI again while disabling it
Date: Thu, 22 Oct 2026 19:03:14 -0700	[thread overview]
Message-ID: <20261023020315.1159-1-ross@r-sc.ca> (raw)
In-Reply-To: <3a9f47f0-884a-49c0-a0a3-d68ea8bdf015@bootlin.com>

On Thu, 22 Oct 2026 14:21:12 +0200, Kory Maincent wrote:
> On 10/22/26 12:08, netdev-bot+sashiko@kernel.org wrote:
>> - [High] The fix is incomplete: the PI being disabled is still enabled
>>   again, and the power budget still leaks, on the priority-eviction path.
>>
>> Pre-existing issues:
>> - [Medium] The priority cut-off in pse_pw_d_retry_power_delivery()
>>   (drivers/net/pse-pd/pse_core.c) never takes effect.
>
> Can you take a look at the above issues?
> The other ones are currently in discussion with Kyle due to similar
> report he faced on his series.

Yes, both are right.

The eviction path: v2 leaves it as it was, and its changelog undersold
what is wrong there. Besides powering the PI just turned off again, the
retry in pse_disable_pi_pol() can enable the PI that is making room,
from inside its own allocation, which then requests its budget a second
time; or it turns the other PI off again and recurses. That was already
so before v2, when _pse_pi_disable() retried the domain itself.

I have two patches for it on top of v2, build-tested only so far:

- "net: pse-pd: don't allocate the budget of a powered PI again": power
  delivery skips a PI that holds budget and is enabled, and a PI that
  holds budget while it is off has it freed and allocated again.
- "net: pse-pd: make room for a PI without recursing": it selects the
  PIs of lower priority to turn off before turning any off, turns them
  off without a retry, and requests the budget once. If that fails, they
  get their budget back and are enabled again. Only the PIs that stay
  off are notified as over budget, and the domain is retried once
  afterwards.

The prio_max cut-off: the second patch replaces it. The retry goes from
the highest priority (lowest value) to the lowest, the power-up order
that ethtool-netlink.rst documents, so a PI of lower priority no longer
takes the budget that one of higher priority waits for. It drops the
cut-off: with the PIs to turn off selected first, a PI of higher
priority only fails with -ERANGE when turning off all the PIs of lower
priority that hold budget wouldn't make room either. The budget left
over is then of no use to it, and once there is room, it can still turn
off a PI of lower priority that took some. If you'd rather keep the
cut-off as well, with prio_max declared outside the loop, I'll add it.

I'll send v3 as a series of the v2 fix and these two, once I've tested
it on the RTL8239 switch with a budget tight enough for the priorities
to matter. A test on a TPS23881 would be welcome too.

I'll leave the other three to the discussion with Kyle, Cc'd. One note
on the PSE_BUDGET_EVAL_STRAT_DYNAMIC test in pse_set_config_isr(): once
it no longer drops a classification reported with a detection, that
classification can reach the recursion above, so its fix should go in
after the eviction fix, or together with it.

Carlo's net v2 series changes the same functions; whichever goes in
first, the other rebases on it.

pw-bot: cr

Ross

      reply	other threads:[~2026-10-23  2:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19 22:08 Ross Cawston
2026-10-22 10:08 ` netdev-bot+sashiko
2026-10-22 12:21   ` Kory Maincent
2026-10-23  2:03     ` Ross Cawston [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=20261023020315.1159-1-ross@r-sc.ca \
    --to=ross@r-sc.ca \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=github@szelinsky.de \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=kyle.swenson@est.tech \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /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

all inboxes | Powered by JetHome®