From b7644592bd0d78cf7aba377124c2d3082607685b Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Fri, 15 Oct 2021 10:28:02 +0800 Subject: HID: wacom: Shrink critical section in `wacom_add_shared_data` 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 Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'drivers/hid') 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; } -- cgit From 3d422a4668ef0c5e30f616e9a9b4c7ac3adaade1 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Fri, 15 Oct 2021 10:28:03 +0800 Subject: HID: wacom: Make use of the helper function devm_add_action_or_reset() The helper function devm_add_action_or_reset() will internally call devm_add_action(), and if devm_add_action() fails then it will execute the action mentioned and return the error code. So use devm_add_action_or_reset() instead of devm_add_action() to simplify the error handling, reduce the code. Signed-off-by: Cai Huoqing Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 62f50e4b837d..2717d39600b4 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -894,11 +894,9 @@ static int wacom_add_shared_data(struct hid_device *hdev) wacom_wac->shared = &data->shared; - retval = devm_add_action(&hdev->dev, wacom_remove_shared_data, wacom); - if (retval) { - wacom_remove_shared_data(wacom); + retval = devm_add_action_or_reset(&hdev->dev, wacom_remove_shared_data, wacom); + if (retval) return retval; - } if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) wacom_wac->shared->touch = hdev; -- cgit