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

[Guenter Roeck]

On 5/13/26 07:21, Wilken Gottwalt wrote:
> On Wed, 13 May 2026 07:05:41 -0700
> Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> > On 5/13/26 06:43, Guenter Roeck wrote:
> > > On 5/13/26 06:32, Wilken Gottwalt wrote:
> > > > Fix removed locking mechanism. The locking mechanism does protect
> > > > chained commands (set rail + get value), which are two separate calls
> > > > to the low level access function. The hwmon (temps for example) and
> > > > debugfs (uptimes for example) subsystem can trigger that chain of
> > > > commands in parallel. The serialization in the hw monioring core alone
> > > > is not enough.
> > > >
> > > > Fixes: 4207069edbf0 ("hwmon: (corsair-psu) Rely on subsystem locking")
> > > > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@xxxxxxxxxx>
> > >
> > > You'll need to explain why using the subsystem lock for debugfs
> > > accesses does not work.
> >
> > Clarifying: "Why using hwmon_lock() / hwmon_unlock() in the debugfs functions
> > would be insufficient".
>
> Yes, I understand that. You gave me an idea for a nice "hack" that would
> demonstrate the problem. I will try it and look if it really happens.
> Though, my thought process is, that debugfs and hwmon are two subsystems
> which do not run in the same thread context. Each one of them would
> trigger a call to corsairpsu_request(), one comming from a *_show callback
> of the debufs and one comming from a hwmon_ops.read callback.
> corsairpsu_request() often calls corsairpsu_usb_cmd() twice, one for
> setting the rail, the second for reading a value of the rail. The mutex
> protects that chain of calls. I really don't think that the debugfs
> callbacks are serailized against the hwmon callbacks. I could create a
> nice demonstration if I passthrough a little hint via function parameters
> from where a callback is triggered. Do you understand, what I mean?

No. That does not explain why calling hwmon_lock() / hwmon_unlock()
would not work when called from the debugfs functions.

Guenter