All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cai Huoqing <caihuoqing@baidu.com>
To: <killertofu@gmail.com>, <Ping.Cheng@wacom.com>
Cc: <jikos@kernel.org>, <caihuoqing@baidu.com>,
	<benjamin.tissoires@redhat.com>, <linux-input@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <Jason.Gerecke@wacom.com>,
	<skomra@gmail.com>, <joshua.dickens@wacom.com>,
	Jason Gerecke <jason.gerecke@wacom.com>
Subject: [PATCH v2 1/2] RFC: HID: wacom: Shrink critical section in `wacom_add_shared_data`
Date: Fri, 15 Oct 2021 10:28:02 +0800	[thread overview]
Message-ID: <20211015022803.3827-1-caihuoqing@baidu.com> (raw)

From: Jason Gerecke <jason.gerecke@wacom.com>

The size of the critical section in this function appears to be larger
than necessary. The `wacom_udev_list_lock` exists to ensure that one
interface cannot begin checking if a shared object exists while a second
interface is doing the same (otherwise both could determine that no
object exists yet and create their own independent objects rather than
sharing just one). It should be safe for the critical section to end
once a fresly-allocated shared object would be found by other threads
(i.e., once it has been added to `wacom_udev_list`, which is looped
over by `wacom_get_hdev_data`).

This commit is a necessary pre-requisite for a later change to swap the
use of `devm_add_action` with `devm_add_action_or_reset`, which would
otherwise deadlock in its error case.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
v1->v2:
	*Fix the repeated word 'that' in changelog
	*Sort to patch series with [PATCH v2 2/2] from Cai

 drivers/hid/wacom_sys.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 93f49b766376..62f50e4b837d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -881,8 +881,8 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 	if (!data) {
 		data = kzalloc(sizeof(struct wacom_hdev_data), GFP_KERNEL);
 		if (!data) {
-			retval = -ENOMEM;
-			goto out;
+			mutex_unlock(&wacom_udev_list_lock);
+			return -ENOMEM;
 		}
 
 		kref_init(&data->kref);
@@ -890,11 +890,12 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 		list_add_tail(&data->list, &wacom_udev_list);
 	}
 
+	mutex_unlock(&wacom_udev_list_lock);
+
 	wacom_wac->shared = &data->shared;
 
 	retval = devm_add_action(&hdev->dev, wacom_remove_shared_data, wacom);
 	if (retval) {
-		mutex_unlock(&wacom_udev_list_lock);
 		wacom_remove_shared_data(wacom);
 		return retval;
 	}
@@ -904,8 +905,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
 		wacom_wac->shared->pen = hdev;
 
-out:
-	mutex_unlock(&wacom_udev_list_lock);
 	return retval;
 }
 
-- 
2.25.1


             reply	other threads:[~2021-10-15  2:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15  2:28 Cai Huoqing [this message]
2021-10-15  2:28 ` [PATCH v2 2/2] HID: wacom: Make use of the helper function devm_add_action_or_reset() Cai Huoqing

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=20211015022803.3827-1-caihuoqing@baidu.com \
    --to=caihuoqing@baidu.com \
    --cc=Jason.Gerecke@wacom.com \
    --cc=Ping.Cheng@wacom.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=joshua.dickens@wacom.com \
    --cc=killertofu@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skomra@gmail.com \
    /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.