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
prev parent 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®