* [PATCH 1/2] input: synaptics_usb: fix deadlock in autosuspend
@ 2018-01-20 21:47 Marcus Folkesson
2018-01-20 21:47 ` [PATCH 2/2] input: synaptics_usb: do not rely on input_dev->users Marcus Folkesson
0 siblings, 1 reply; 2+ messages in thread
From: Marcus Folkesson @ 2018-01-20 21:47 UTC (permalink / raw)
To: Dmitry Torokhov, Marcus Folkesson, Arvind Yadav; +Cc: linux-input, linux-kernel
usb_autopm_get_interface() that is called in synusb_open() does an
autoresume if the device is suspended.
input_dev->mutex used in synusb_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/mouse/synaptics_usb.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index cb7d15d826d0..2c66913cf5a2 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -82,6 +82,9 @@ struct synusb {
struct urb *urb;
unsigned char *data;
+ /* serialize access to open/suspend */
+ struct mutex pm_mutex;
+
/* input device related data structures */
struct input_dev *input;
char name[128];
@@ -252,6 +255,7 @@ static int synusb_open(struct input_dev *dev)
return retval;
}
+ mutex_lock(&synusb->pm_mutex);
retval = usb_submit_urb(synusb->urb, GFP_KERNEL);
if (retval) {
dev_err(&synusb->intf->dev,
@@ -264,6 +268,7 @@ static int synusb_open(struct input_dev *dev)
synusb->intf->needs_remote_wakeup = 1;
out:
+ mutex_unlock(&synusb->pm_mutex);
usb_autopm_put_interface(synusb->intf);
return retval;
}
@@ -275,8 +280,10 @@ static void synusb_close(struct input_dev *dev)
autopm_error = usb_autopm_get_interface(synusb->intf);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
synusb->intf->needs_remote_wakeup = 0;
+ mutex_unlock(&synusb->pm_mutex);
if (!autopm_error)
usb_autopm_put_interface(synusb->intf);
@@ -315,6 +322,7 @@ static int synusb_probe(struct usb_interface *intf,
synusb->udev = udev;
synusb->intf = intf;
synusb->input = input_dev;
+ mutex_init(&synusb->pm_mutex);
synusb->flags = id->driver_info;
if (synusb->flags & SYNUSB_COMBO) {
@@ -466,11 +474,10 @@ static void synusb_disconnect(struct usb_interface *intf)
static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return 0;
}
@@ -481,14 +488,14 @@ static int synusb_resume(struct usb_interface *intf)
struct input_dev *input_dev = synusb->input;
int retval = 0;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return retval;
}
@@ -496,9 +503,8 @@ static int synusb_resume(struct usb_interface *intf)
static int synusb_pre_reset(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
return 0;
@@ -515,7 +521,7 @@ static int synusb_post_reset(struct usb_interface *intf)
retval = -EIO;
}
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return retval;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH 2/2] input: synaptics_usb: do not rely on input_dev->users
2018-01-20 21:47 [PATCH 1/2] input: synaptics_usb: fix deadlock in autosuspend Marcus Folkesson
@ 2018-01-20 21:47 ` Marcus Folkesson
0 siblings, 0 replies; 2+ messages in thread
From: Marcus Folkesson @ 2018-01-20 21:47 UTC (permalink / raw)
To: Dmitry Torokhov, Marcus Folkesson, Arvind Yadav; +Cc: linux-input, linux-kernel
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
The same URB will then be fail when resubmitted in synusb_open().
Introduce synusb->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/mouse/synaptics_usb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index 2c66913cf5a2..e2b726751220 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -84,6 +84,7 @@ struct synusb {
/* serialize access to open/suspend */
struct mutex pm_mutex;
+ bool is_open;
/* input device related data structures */
struct input_dev *input;
@@ -266,6 +267,7 @@ static int synusb_open(struct input_dev *dev)
}
synusb->intf->needs_remote_wakeup = 1;
+ synusb->is_open = 1;
out:
mutex_unlock(&synusb->pm_mutex);
@@ -283,6 +285,7 @@ static void synusb_close(struct input_dev *dev)
mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
synusb->intf->needs_remote_wakeup = 0;
+ synusb->is_open = 0;
mutex_unlock(&synusb->pm_mutex);
if (!autopm_error)
@@ -485,12 +488,11 @@ static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
static int synusb_resume(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
int retval = 0;
mutex_lock(&synusb->pm_mutex);
- if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+ if ((synusb->is_open || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
@@ -513,10 +515,9 @@ static int synusb_pre_reset(struct usb_interface *intf)
static int synusb_post_reset(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
int retval = 0;
- if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+ if ((synusb->is_open || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-01-20 21:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-20 21:47 [PATCH 1/2] input: synaptics_usb: fix deadlock in autosuspend Marcus Folkesson
2018-01-20 21:47 ` [PATCH 2/2] input: synaptics_usb: do not rely on input_dev->users Marcus Folkesson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).