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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <DD1XOHE6P5OC.2JUQRAGAE1KTU@cknow.org>
Date: Thu, 25 Sep 2025 15:59:44 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Ulf Hansson" <ulf.hansson@...aro.org>, "Saravana Kannan"
 <saravanak@...gle.com>, "Rafael J . Wysocki" <rafael@...nel.org>, "Greg
 Kroah-Hartman" <gregkh@...uxfoundation.org>, <linux-pm@...r.kernel.org>
Cc: "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()

Hi,

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.
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.

You're the expert, but I'm not so sure this is an improvement.
I do regularly check dmesg level 0, 1, 2, 3 and 4, hence it landed on my
radar. I do not regularly check all the dmesg msgs, so this change would
result it dropping off my (immediate) radar.

On Fri Sep 12, 2025 at 8:32 PM CEST, Saravana Kannan wrote:
> Please don't just disable fw_devlink using fw_devlink=off. We want to
> fix any issues you are hitting with it. I might even delete this "off"
> option sometime. It was meant as an early debug option.

That response made a lot of sense to me. I haven't gotten around to it
yet, but I want to test all my (Rockchip) devices to see if they 'need'
that parameter to 'silence' the warning. I have a (vague) recollection
that some don't need it (for that).

My 0.02

Cheers,
  Diederik

> Ideally these messages should be at the debug level, but since the
> ->sync_state() feature is under an ongoing deployment and the prints
> provides valuable information, let's move to the info level for now.
>
> Cc: Saravana Kannan <saravanak@...gle.com>
> Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> Reported-by: Sebin Francis <sebin.francis@...com>
> Reported-by: Diederik de Haas <didi.debian@...ow.org>
> Reported-by: Jon Hunter <jonathanh@...dia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
>  drivers/base/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d22d6b23e758..97eab79c2f3b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1784,7 +1784,7 @@ static int fw_devlink_dev_sync_state(struct device *dev, void *data)
>  		return 0;
>  
>  	if (fw_devlink_sync_state == FW_DEVLINK_SYNC_STATE_STRICT) {
> -		dev_warn(sup, "sync_state() pending due to %s\n",
> +		dev_info(sup, "sync_state() pending due to %s\n",
>  			 dev_name(link->consumer));
>  		return 0;
>  	}
> @@ -1792,7 +1792,7 @@ static int fw_devlink_dev_sync_state(struct device *dev, void *data)
>  	if (!list_empty(&sup->links.defer_sync))
>  		return 0;
>  
> -	dev_warn(sup, "Timed out. Forcing sync_state()\n");
> +	dev_info(sup, "Timed out. Forcing sync_state()\n");
>  	sup->state_synced = true;
>  	get_device(sup);
>  	list_add_tail(&sup->links.defer_sync, data);

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)