Re: [STABLE v5.15-v5.10 2/6] HID: hid-steam: Make client_opened a counter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hid: hid-steam: Redundant manual refcounting in open/close callbacks
--

commit 606f575d1b062004ef69d4378f3403ab1764d5a8
Author: Vicki Pfau <vi@endrift.com>

HID: hid-steam: Make client_opened a counter

This patch changes the client_opened flag from a boolean to a counter. The
intent is to track how many clients have opened the hidraw node so that
concurrent opens and closes do not prematurely re-enable lizard mode.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 379e7765a93e9..78104e1246e82 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c
[ ... ]
> @@ -624,7 +624,7 @@ static int steam_client_ll_open(struct hid_device *hdev)
>  	struct steam_device *steam = hdev->driver_data;
>  
>  	mutex_lock(&steam->mutex);
> -	steam->client_opened = true;
> +	steam->client_opened++;

[Severity: Low]
This isn't a bug, but is this manual refcounting redundant?

The commit message indicates this change is to handle multiple clients opening
the device at the same time. However, the HID core already natively refcounts
and serializes the low-level open and close callbacks.

Looking at hid_hw_open() in drivers/hid/hid-core.c:

	if (!hdev->ll_open_count++) {
		ret = hdev->ll_driver->open(hdev);
		...
	}

Since the core only calls the low-level open callback on the 0 to 1 transition
and the close callback on the 1 to 0 transition, concurrent opens are handled
before reaching steam_client_ll_open(). Does this mean the manual counter
behaves identically to the original boolean flag and won't actually track
multiple clients at this level?

>  	mutex_unlock(&steam->mutex);
>  
>  	steam_input_unregister(steam);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013091215.529531-1-lee@kernel.org?part=2