Re: [PATCH] hwmon: corsair-psu: fix and readd locking of command buffer

[Wilken Gottwalt <wilken.gottwalt@posteo.net>]

On Wed, 13 May 2026 09:42:08 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Wed, May 13, 2026 at 03:53:51PM +0000, Wilken Gottwalt wrote:
> > On Wed, 13 May 2026 07:58:14 -0700
> > Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> ...
> > Okay, that will get a bit complex now, because I added my hack and I see
> > exactly what I assumed is happening.
> > 
> ...
> > 
> > If this does not explain the obvious issue, I have not idea how explain
> > it further. My English is limited. This is a HID driver with data gathering
> > functions running in the context of the USB-HID context. Callbacks from the
> > hwmon and the debugfs subsystem call these data gathering functions, and the
> > first function in that context, corsairpsu_request(), which can run several
> > instances in paralellel, needs the mutex.
> > 
> 
> You don't explain why the patches below are insufficient.
> 
> I used guard() to keep the changes simple, but hwmon_lock() / hwmon_unlock()
> would be similar. Please provide evidence that this does not work.
> 
> Thanks,
> Guenter
> --
> From aa3ec1484bdd619e8fa2ce569ec653d35fbf3615 Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Wed, 13 May 2026 07:14:33 -0700
> Subject: [PATCH 1/4] hwmon: Support guard() and scoped_guard for subsystem
>  locks
> 
> Add support for guard() and scoped_guard() for the hwmon subsystem lock
> to simplify its use.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  Documentation/hwmon/hwmon-kernel-api.rst | 7 ++++---
>  include/linux/hwmon.h                    | 2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
> index 1d7f1397a827..9fcde32a140d 100644
> --- a/Documentation/hwmon/hwmon-kernel-api.rst
> +++ b/Documentation/hwmon/hwmon-kernel-api.rst
> @@ -85,9 +85,10 @@ removal.
>  When using ``[devm_]hwmon_device_register_with_info()`` to register the
>  hardware monitoring device, accesses using the associated access functions
>  are serialised by the hardware monitoring core. If a driver needs locking
> -for other functions such as interrupt handlers or for attributes which are
> -fully implemented in the driver, hwmon_lock() and hwmon_unlock() can be used
> -to ensure that calls to those functions are serialized.
> +for other functions such as interrupt handlers, attributes which are fully
> +implemented in the driver, or debugfs functions, hwmon_lock() and hwmon_unlock()
> +can be used to ensure that calls to those functions are serialized. Those
> +functions also support guard() and scoped_guard() variants.
>  
>  Using devm_hwmon_device_register_with_info()
>  --------------------------------------------
> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> index 301a83afbd66..04959e044fd0 100644
> --- a/include/linux/hwmon.h
> +++ b/include/linux/hwmon.h
> @@ -495,6 +495,8 @@ char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
>  void hwmon_lock(struct device *dev);
>  void hwmon_unlock(struct device *dev);
>  
> +DEFINE_GUARD(hwmon_lock, struct device *, hwmon_lock(_T), hwmon_unlock(_T))
> +
>  /**
>   * hwmon_is_bad_char - Is the char invalid in a hwmon name
>   * @ch: the char to be considered

Now I am completely confused. What is that guard() and scoped_guard() patch?
It is not part of Torvalds repo or your
git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git repo. Not
in branch hwmon, hwmon-next, hwmon-playground, hwmon-staging, testing, fixes
or master.
I mean, it is a bit hard to check against this, if it is not upstream. Did
you maybe forgot to push it? I really fetched everything to be on the
latest commits and grepped for "DEFINE_GUARD". The only one I see is the
one in drivers/hwmon/pmbus/pmbus.h.

greetings,
Wilken