greybus: svc: fix racy hotplug handling

Fix racy hotplug handling by serialising all processing of hot-plug and
unplug requests using a single-threaded dedicated workqueue.

This fixes a reported crash during enumeration when processing multiple
events.

The current svc implementation does not handle concurrency at all (e.g.
no interface list lock or refcounting) so we need to use the big hammer
for now.

Note that we will eventually want to process events for different
interfaces in parallel, but that we'd still need a workqueue in order
not to starve other svc requests (e.g. for timesync).

Reported-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>

diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c
index 34713d6b007465b3220a4e2337f3a97f3c8c480f..af41e8e49c3101e8804a9798053edaa4e424b57d 100644 (file)
--- a/drivers/staging/greybus/svc.c
+++ b/drivers/staging/greybus/svc.c
@@ -511,6 +511,7 @@ static void gb_svc_process_deferred_request(struct work_struct *work)
 
 static int gb_svc_queue_deferred_request(struct gb_operation *operation)
 {
+       struct gb_svc *svc = operation->connection->private;
        struct gb_svc_deferred_request *dr;
 
        dr = kmalloc(sizeof(*dr), GFP_KERNEL);
@@ -522,7 +523,7 @@ static int gb_svc_queue_deferred_request(struct gb_operation *operation)
        dr->operation = operation;
        INIT_WORK(&dr->work, gb_svc_process_deferred_request);
 
-       queue_work(system_unbound_wq, &dr->work);
+       queue_work(svc->wq, &dr->work);
 
        return 0;
 }
@@ -532,7 +533,7 @@ static int gb_svc_queue_deferred_request(struct gb_operation *operation)
  * initialization on the module side. Over that, we may also need to download
  * the firmware first and flash that on the module.
  *
- * In order to make other hotplug events to not wait for all this to finish,
+ * In order not to make other svc events wait for all this to finish,
  * handle most of module hotplug stuff outside of the hotplug callback, with
  * help of a workqueue.
  */
@@ -658,6 +659,7 @@ static void gb_svc_release(struct device *dev)
        struct gb_svc *svc = to_gb_svc(dev);
 
        ida_destroy(&svc->device_id_map);
+       destroy_workqueue(svc->wq);
        kfree(svc);
 }
 
@@ -675,6 +677,12 @@ static int gb_svc_connection_init(struct gb_connection *connection)
        if (!svc)
                return -ENOMEM;
 
+       svc->wq = alloc_workqueue("%s:svc", WQ_UNBOUND, 1, dev_name(&hd->dev));
+       if (!svc->wq) {
+               kfree(svc);
+               return -ENOMEM;
+       }
+
        svc->dev.parent = &hd->dev;
        svc->dev.bus = &greybus_bus_type;
        svc->dev.type = &greybus_svc_type;

diff --git a/drivers/staging/greybus/svc.h b/drivers/staging/greybus/svc.h
index 99be0411d000162388231be7ad6263fb03b77b13..3acfa07cc73bab8abcdc017beb3772cd3a86a787 100644 (file)
--- a/drivers/staging/greybus/svc.h
+++ b/drivers/staging/greybus/svc.h
@@ -22,6 +22,7 @@ struct gb_svc {
        struct gb_connection    *connection;
        enum gb_svc_state       state;
        struct ida              device_id_map;
+       struct workqueue_struct *wq;
 
        u16 endo_id;
        u8 ap_intf_id;