Re: [PATCH 05/10] HID: steam: Coalesce rumble packets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Self-deadlock in workqueue callbacks due to double locking of `steam->report_mutex`.
--

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

HID: steam: Coalesce rumble packets

This commit introduces a delayed workqueue to rate-limit and coalesce rumble
packets to 20Hz on Steam Deck devices. It also modifies the existing rumble
work callback to add a mutex guard.

> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> index 34653ad383ac0..01b64194ec97b 100644
> --- a/drivers/hid/hid-steam.c
> +++ b/drivers/hid/hid-steam.c

[ ... ]

> @@ -603,10 +604,26 @@ static void steam_haptic_rumble_cb(struct work_struct *work)
>  {
>  	struct steam_device *steam = container_of(work, struct steam_device,
>  							rumble_work);
> +
> +	guard(mutex)(&steam->report_mutex);
>  	steam_haptic_rumble(steam, 0, steam->rumble_left,
>  		steam->rumble_right, 2, 0);

[Severity: High]
Will this cause a self-deadlock when playing force-feedback rumble effects?

This callback acquires report_mutex using guard(), but then immediately calls
steam_haptic_rumble(), which acquires the same mutex again:

steam_haptic_rumble() {
        ...
        mutex_lock(&steam->report_mutex);
        ret = steam_send_report(steam, report, sizeof(report));
        mutex_unlock(&steam->report_mutex);
        return ret;
}

Since Linux mutexes are not recursive, won't this hang the worker thread
and block any subsequent operations needing the report_mutex?

>  }
>  
> +static void steam_coalesce_rumble_cb(struct work_struct *work)
> +{
> +	struct steam_device *steam = container_of(to_delayed_work(work),
> +							struct steam_device,
> +							coalesce_rumble_work);
> +
> +	guard(mutex)(&steam->report_mutex);
> +	steam_haptic_rumble(steam, 0, steam->rumble_left,
> +		steam->rumble_right, 2, 0);

[Severity: High]
Does this new callback have the same self-deadlock issue as described above,
since it also calls steam_haptic_rumble() while holding report_mutex?

> +
> +	if (steam->rumble_left || steam->rumble_right)
> +		schedule_delayed_work(&steam->coalesce_rumble_work, HZ / 20);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702222145.1863104-1-vi@endrift.com?part=5