Commit b7644592 authored by Jason Gerecke's avatar Jason Gerecke Committed by Jiri Kosina

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: default avatarJason Gerecke <jason.gerecke@wacom.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent 42d43c92
...@@ -881,8 +881,8 @@ static int wacom_add_shared_data(struct hid_device *hdev) ...@@ -881,8 +881,8 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (!data) { if (!data) {
data = kzalloc(sizeof(struct wacom_hdev_data), GFP_KERNEL); data = kzalloc(sizeof(struct wacom_hdev_data), GFP_KERNEL);
if (!data) { if (!data) {
retval = -ENOMEM; mutex_unlock(&wacom_udev_list_lock);
goto out; return -ENOMEM;
} }
kref_init(&data->kref); kref_init(&data->kref);
...@@ -890,11 +890,12 @@ static int wacom_add_shared_data(struct hid_device *hdev) ...@@ -890,11 +890,12 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list); list_add_tail(&data->list, &wacom_udev_list);
} }
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared; wacom_wac->shared = &data->shared;
retval = devm_add_action(&hdev->dev, wacom_remove_shared_data, wacom); retval = devm_add_action(&hdev->dev, wacom_remove_shared_data, wacom);
if (retval) { if (retval) {
mutex_unlock(&wacom_udev_list_lock);
wacom_remove_shared_data(wacom); wacom_remove_shared_data(wacom);
return retval; return retval;
} }
...@@ -904,8 +905,6 @@ static int wacom_add_shared_data(struct hid_device *hdev) ...@@ -904,8 +905,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN) else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
wacom_wac->shared->pen = hdev; wacom_wac->shared->pen = hdev;
out:
mutex_unlock(&wacom_udev_list_lock);
return retval; return retval;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment