Re: [PATCH net] eth: fbnic: fix race between concurrent hwmon sensor reads

[Alexander Duyck]

On Wed, Jun 24, 2026 at 2:56 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Wed, Jun 24, 2026 at 11:51:29PM +0200, Andrew Lunn wrote:
> > On Wed, Jun 24, 2026 at 01:05:14PM -0700, Zinc Lim wrote:
> > > From: Zinc Lim <limzhineng2@xxxxxxxxx>
> > >
> > > Reading an hwmon sensor
> >
> > Since this is a hwmon related patch, it is a good idea to Cc: the
> > hwmon Maintainer.
> >
> > I don't know hwmon too well, i've never had to look at locking, but...
> >
> > static int hwmon_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
> > {
> >       struct hwmon_thermal_data *tdata = thermal_zone_device_priv(tz);
> >       struct hwmon_device *hwdev = to_hwmon_device(tdata->dev);
> >       int ret;
> >       long t;
> >
> >       guard(mutex)(&hwdev->lock);
> >
> >       ret = hwdev->chip->ops->read(tdata->dev, hwmon_temp, hwmon_temp_input,
> >                                    tdata->index, &t);
> >
> > Could you explain why this lock in the hwmon core is not sufficient?
> > It could be i'm reading it wrong, but it looks like the lock is shared
> > by all hwmon instanced registered by a
> > hwmon_device_register_with_info() call.
>
> Documentation/hwmon/hwmon-kernel-api.rst
>
> 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.

I did some digging and it looks like 3ad2a7b9b15d ("hwmon: Serialize
accesses in hwmon core") landed into 6.18. This was a patch put
together based on issues reported in an earlier kernel version and
explains how this got missed. So what we have is a redundant fix. We
can drop this for upstream and probably look at pulling the upstream
fix into our kernels that were having the issues.

Thanks for the review feedback.

- Alex