Re: [PATCH v3 2/9] HID: haptic: Get rid of input_ff_destroy() in error paths

[Rong Zhang <i@rong.moe>]

Hi Dmitry,

On Thu, 2026-11-12 at 21:50 -0800, Dmitry Torokhov wrote:
> Hi Rong,
> 
> On Fri, Nov 13, 2026 at 01:53:55AM +0800, Rong Zhang wrote:
> > It's painful to have error paths after calling input_ff_create(),
> > because input_ff_destroy() frees too much things but does not restore
> > some other things.
> > 
> > On get_device failure, the module's bail-out path calls module_put()
> > right before hid_haptic_destroy() calling it again, causing refcount
> > underflow.
> > 
> > Besides, every error path calling input_ff_destroy() frees ff->private,
> > while the others don't. This divergence has caused quite a lot of
> > trouble.
> > 
> > Get the module and the device beforehand, so that there is no error path
> > after input_ff_create(), eliminating the call to input_ff_destroy(). In
> > this manner, we don't need to restore dev->flush and dev->event either.
> 
> The only reason hid_haptic_init() had try_module_get(THIS_MODULE) and
> get_device(&hdev->dev) (and their counterparts in hid_haptic_destroy())
> was to work around input_ff_destroy() previously being called only from
> input_dev_release(), which could be deferred until after hdev was
> unbound or the driver module was unloaded if userspace kept the evdev
> node open.

Thanks for the clarification. Yeah, it was a painful design...

> 
> I am queuing an input core change ("Input: destroy force-feedback device
> on input unregistration") [1] that calls input_ff_destroy() directly in
> __input_unregister_device() once all handlers are disconnected and the
> device is removed from sysfs.
> 
> With that change in place, hid_haptic_destroy() is guaranteed to run
> synchronously during input_unregister_device() while hdev is still bound
> and alive (and dev->dev.parent already holds a reference to &hdev->dev
> anyway). That means get_device(&hdev->dev) / put_device(&hdev->dev) and
> try_module_get(THIS_MODULE) / module_put(THIS_MODULE) are not needed at
> all here (until patch 8/9 where hid-haptic becomes a separate plugin
> module and hid_plugin_get() is taken before calling self_plugin->init()),
> and simply dropping them eliminates the error path after
> input_ff_create(). It also avoids the devres lifetime issue with
> put_device(&hdev->dev) mentioned in patch 1/9.

Thanks for fixing the ff core and reviewing this series! The lifecycle of
ff does look much better with your patch.

I will follow your suggestion to simplify the fixes in the series and
resubmit it when your patch lands.

Thanks,
Rong

> 
> [1] https://lore.kernel.org/linux-input/2f722967b6db0692d4dd4164aff39b62@gmail.com/
> 
> Thanks.