All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Rong Zhang <i@rong.moe>
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	Angela Czubak <aczubak@google.com>,
	Henrik Rydberg <rydberg@bitmath.org>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/9] HID: haptic: Get rid of input_ff_destroy() in error paths
Date: Thu, 12 Nov 2026 21:50:00 -0800	[thread overview]
Message-ID: <f44447354f28c295e79a025bbcab7a4e@gmail.com> (raw)
In-Reply-To: <20261113-hid-haptic-kmod-v3-2-e7dfa8766c0d@rong.moe>

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.

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.

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

Thanks.

-- 
Dmitry

  reply	other threads:[~2026-11-13  5:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-12 17:53 [PATCH v3 0/9] HID: Add hid-plugin framework & convert haptic into a plugin Rong Zhang
2026-11-12 17:53 ` [PATCH v3 1/9] HID: haptic: Fix use-after-free when draining workqueue Rong Zhang
2026-11-12 17:53 ` [PATCH v3 2/9] HID: haptic: Get rid of input_ff_destroy() in error paths Rong Zhang
2026-11-13  5:50   ` Dmitry Torokhov [this message]
2026-11-12 17:53 ` [PATCH v3 3/9] HID: haptic, multitouch: Check haptic prerequisite properly Rong Zhang
2026-11-12 17:53 ` [PATCH v3 4/9] HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation Rong Zhang
2026-11-12 17:53 ` [PATCH v3 5/9] HID: haptic: Postpone GET_REPORT to init Rong Zhang
2026-11-12 17:53 ` [PATCH v3 6/9] HID: haptic: Convert simple methods into static inline ones Rong Zhang
2026-11-12 17:54 ` [PATCH v3 7/9] HID: Add hid-plugin framework for loadable plugin modules Rong Zhang
2026-11-12 17:54 ` [PATCH v3 8/9] HID: haptic: Convert into a loadable plugin module Rong Zhang
2026-11-12 17:54 ` [PATCH v3 9/9] HID: haptic: Remove stubs in favor of inlined dead code elimination Rong Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f44447354f28c295e79a025bbcab7a4e@gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=aczubak@google.com \
    --cc=bentiss@kernel.org \
    --cc=i@rong.moe \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rydberg@bitmath.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.