Re: [PATCH 2/2] hwmon: (dell-smm) Unify i8k_ioctl() and i8k_ioctl_unlocked()

From: Guenter Roeck
Date: Mon Nov 22 2021 - 14:28:39 EST

On 11/22/21 11:10 AM, Pali Rohár wrote:
On Monday 22 November 2021 19:50:14 Armin Wolf wrote:
Am 22.11.21 um 18:55 schrieb Guenter Roeck:
On 11/22/21 8:01 AM, Pali Rohár wrote:
On Saturday 20 November 2021 18:03:19 Armin Wolf wrote:
The only purpose of i8k_ioctl() is to call i8k_ioctl_unlocked()
with i8k_mutex held. Judging from the hwmon code, this mutex
only needs to be held when setting the fan speed/mode.

Really? I think that there is no difference between setting and getting
fan speed/mode. At least I do not see why 'set' needs mutex and 'get' do
not need it. Some more explanation is needed...

I8K_SET_FAN sets the fan speed and returns the current status. Without
locking, the returned status may not match or be associated with the
set operation.

Maybe that doesn't matter, and the synchronization is not needed. If so,
you can probably remove the locking entirely.


That is the reason i kept the locking code. Since i do not want to break
the ioctl interfacein any way, removing the locking code seems too risky
to me.

I see. That is a good point.

But there is same race condition also when at the same time going to
change speed via ioctl and also via hwmon sysfs.

I thought the sysfs code does not change the fan speed and report the
fan status in the same request. Did I miss something ?