Le Sat, 14 Jun 2025 12:33:11 -0700,
Jakub Kicinski <kuba@xxxxxxxxxx> a écrit :
> On Tue, 10 Jun 2025 10:11:41 +0200 Kory Maincent wrote:
> > +static bool
> > +pse_pi_is_admin_enable_not_applied(struct pse_controller_dev *pcdev,
> > + int id)
>
> the only caller of this function seems to negate the return value:
>
> drivers/net/pse-pd/pse_core.c:369: if
> (!pse_pi_is_admin_enable_not_applied(pcdev, i))
>
> let's avoid the double negation ?
I thought it was better for comprehension.
If we inverse the behavior we would have a function name like that:
pse_pi_is_admin_disable_not_detected_or_applied()
Do you have a better proposition?
> > +static int pse_disable_pi_pol(struct pse_controller_dev *pcdev, int id)
> > +{
> > + unsigned long notifs = ETHTOOL_PSE_EVENT_OVER_BUDGET;
> > + struct pse_ntf ntf = {};
> > + int ret;
> > +
> > + dev_dbg(pcdev->dev, "Disabling PI %d to free power budget\n", id);
> > +
> > + NL_SET_ERR_MSG_FMT(&ntf.extack,
> > + "Disabling PI %d to free power budget", id);
>
> You so dutifully fill in this extack but it doesn't go anywhere.
> Extacks can only be attached to NLMSG_ERROR and NLMSG_DONE
> control messages. You can use the extack infra for the formatting,
> but you need to add a string attribute to the notification message
> to actually expose it to the user.
Indeed it seems there are useless extacks.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
