linux-kernel - Re: [PATCH] driver core: fw_devlink: Don't warn in fw_devlink_dev_sync

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPDyKFp0uV2WaQGjwsO8JG1zF_1oXj7-GVnaejih+dN9RAdykg@mail.gmail.com>
Date: Fri, 26 Sep 2025 14:05:05 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Saravana Kannan <saravanak@...gle.com>
Cc: Diederik de Haas <didi.debian@...ow.org>, "Rafael J . Wysocki" <rafael@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-pm@...r.kernel.org, 
	Geert Uytterhoeven <geert@...ux-m68k.org>, 
	Nicolas Frattaroli <nicolas.frattaroli@...labora.com>, Heiko Stuebner <heiko@...ech.de>, 
	Sebastian Reichel <sebastian.reichel@...labora.com>, Sebin Francis <sebin.francis@...com>, 
	Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Jon Hunter <jonathanh@...dia.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] driver core: fw_devlink: Don't warn in fw_devlink_dev_sync_state()

On Thu, 25 Sept 2025 at 23:49, Saravana Kannan <saravanak@...gle.com> wrote:
>
> On Thu, Sep 25, 2025 at 10:52 AM Diederik de Haas <didi.debian@...ow.org> wrote:
> >
> > On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:
> > > On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@...ow.org> wrote:
> > >> On Thu Sep 25, 2025 at 1:59 PM CEST, Ulf Hansson wrote:
> > >> > Due to the wider deployment of the ->sync_state() support, for PM domains
> > >> > for example, we are receiving reports about the messages that are being
> > >> > logged in fw_devlink_dev_sync_state(). In particular as they are at the
> > >> > warning level, which doesn't seem correct.
> > >> >
> > >> > Even if it certainly is useful to know that the ->sync_state() condition
> > >> > could not be met, there may be nothing wrong with it. For example, a driver
> > >> > may be built as module and are still waiting to be initialized/probed.
> > >>
> > >> "there may be nothing wrong with it" doesn't sound very convincing.
> > >> So there *can* be something wrong with it, so warning sounds
> > >> appropriate? If there is (certainly) something wrong with it, I expect
> > >> an error.
> > >
> > > Sorry if I was too vague. See more below.
> > >
> > >> FWIW: most of my drivers/modules are built as modules.
> > >> I do seem to run into 'problems' more then average because of that, but
> > >> to me it just signals there is something wrong ... which should be
> > >> fixed. Not silenced.
> > >
> > > Well, why is it wrong to have drivers being built as modules? They
>
> IIUC/Remember Kconfig correctly, FW_DEVLINK_SYNC_STATE_TIMEOUT should
> be off by default? (if I don't give any default, what ends up
> happening?)

As "FW_DEVLINK_SYNC_STATE_STRICT" is default, the "sync_state()
pending due..." print gets printed for a lot of consumer devices that
have not been probed yet.

>
> If we can assume, timeout won't happen by default, then the only
> people seeing this warning are people setting the flag or setting the
> command line param. So I'm okay with making it an info.

I think you got this wrong. It's the default behaviour that triggers
lots of prints.

>
> But the "sync_state() pending due to" is for the default behavior. I'm
> assuming without the sync_state() supported added to power domains,
> the ones getting this warning would have been powered off? But after
> the sync_state() support, it's not powered off anymore. That can cause
> increased power usage. Seems like something worth warning about.

Yes, I certainly agree, but...

To me, it kind of sounds like we should change to use
FW_DEVLINK_SYNC_STATE_TIMEOUT as the default behaviour. In that case,
I don't think there is a need to change the log-level for
"sync_state() pending due to..".

>
> Also, another thing I want to understand is, for all the reports you
> are getting for "sync_state() pending due to", is it actually correct
> to have them pending? Or can fw_devlink do better in those cases and
> turn them off? It could be a driver fix/fw_devlink fix depending on
> the issue.

There are some cases that we potentially could fix, in particular for
when a genpd-provider, provides multiple power-domains per device-node
(#power-domain-cells = <1>;). But let's discuss that separately.

>
> TL;DR:
> 1. For timeout messages, since timeout isn't default behavior, I'm
> okay making it an info. But
> 2. For pending sync state messages, I think it should remain as a warning.

See above.

This means drivers being built as modules will trigger a lot of
warnings, in the default behavior. Is this really what we want?

[...]

Kind regards
Uffe