Re: [PATCH v3] HID: multitouch: use GFP_ATOMIC for report/application/usage allocations

@@ -37,6 +37,7 @@
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/input/mt.h>
 #include <linux/jiffies.h>
 #include <linux/string.h>

@@ -186,6 +187,11 @@ struct mt_device {
 	bool serial_maybe;	/* need to check for serial protocol */
 
 	struct list_head applications;
+	spinlock_t application_list_lock;	/* protects applications, incl.
+						 * concurrent add from softirq
+						 * vs. list_for_each_entry from
+						 * timer/sysfs context
+						 */
 	struct list_head reports;
 };
 

@@ -477,6 +483,7 @@ static ssize_t mt_set_quirks(struct device *dev,
 	struct hid_device *hdev = to_hid_device(dev);
 	struct mt_device *td = hid_get_drvdata(hdev);
 	struct mt_application *application;
+	unsigned long flags;
 
 	unsigned long val;
 

@@ -485,11 +492,13 @@ static ssize_t mt_set_quirks(struct device *dev,
 
 	td->mtclass.quirks = val;
 
+	spin_lock_irqsave(&td->application_list_lock, flags);
 	list_for_each_entry(application, &td->applications, list) {
 		application->quirks = val;
 		if (!application->have_contact_count)
 			application->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
 	}
+	spin_unlock_irqrestore(&td->application_list_lock, flags);
 
 	return count;
 }

@@ -604,6 +613,7 @@ static struct mt_usages *mt_allocate_usage(struct hid_device *hdev,
 {
 	struct mt_usages *usage;
 
+	/* only called from mt_touch_input_mapping(), at probe time */
 	usage = devm_kzalloc(&hdev->dev, sizeof(*usage), GFP_KERNEL);
 	if (!usage)
 		return NULL;

@@ -627,14 +637,16 @@ static struct mt_usages *mt_allocate_usage(struct hid_device *hdev,
 	return usage;
 }
 
+/* called with td->application_list_lock held */
 static struct mt_application *mt_allocate_application(struct mt_device *td,
 						      struct hid_report *report)
 {
 	unsigned int application = report->application;
 	struct mt_application *mt_application;
 
+	/* may run from hid_report_raw_event() in (soft)irq context */
 	mt_application = devm_kzalloc(&td->hdev->dev, sizeof(*mt_application),
-				      GFP_KERNEL);
+				      GFP_ATOMIC);
 	if (!mt_application)
 		return NULL;
 

@@ -667,6 +679,15 @@ static struct mt_application *mt_find_application(struct mt_device *td,
 {
 	unsigned int application = report->application;
 	struct mt_application *tmp, *mt_application = NULL;
+	unsigned long flags;
+
+	/*
+	 * Hold the lock across the search and the fallback allocation so
+	 * a concurrent mt_release_contacts()/mt_set_quirks() never walks
+	 * the list mid-insert, and two callers can never both decide to
+	 * allocate the same application.
+	 */
+	spin_lock_irqsave(&td->application_list_lock, flags);
 
 	list_for_each_entry(tmp, &td->applications, list) {
 		if (application == tmp->application) {

@@ -681,6 +702,8 @@ static struct mt_application *mt_find_application(struct mt_device *td,
 	if (!mt_application)
 		mt_application = mt_allocate_application(td, report);
 
+	spin_unlock_irqrestore(&td->application_list_lock, flags);
+
 	return mt_application;
 }
 

@@ -692,7 +715,8 @@ static struct mt_report_data *mt_allocate_report_data(struct mt_device *td,
 	struct hid_field *field;
 	int r, n;
 
-	rdata = devm_kzalloc(&td->hdev->dev, sizeof(*rdata), GFP_KERNEL);
+	/* may run from hid_report_raw_event() in (soft)irq context */
+	rdata = devm_kzalloc(&td->hdev->dev, sizeof(*rdata), GFP_ATOMIC);
 	if (!rdata)
 		return NULL;
 

@@ -2060,6 +2084,7 @@ static void mt_release_contacts(struct hid_device *hid)
 	struct hid_input *hidinput;
 	struct mt_application *application;
 	struct mt_device *td = hid_get_drvdata(hid);
+	unsigned long flags;
 
 	list_for_each_entry(hidinput, &hid->inputs, list) {
 		struct input_dev *input_dev = hidinput->input;

@@ -2077,9 +2102,11 @@ static void mt_release_contacts(struct hid_device *hid)
 		}
 	}
 
+	spin_lock_irqsave(&td->application_list_lock, flags);
 	list_for_each_entry(application, &td->applications, list) {
 		application->num_received = 0;
 	}
+	spin_unlock_irqrestore(&td->application_list_lock, flags);
 }
 
 static void mt_expired_timeout(struct timer_list *t)

@@ -2127,6 +2154,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	hid_set_drvdata(hdev, td);
 
 	INIT_LIST_HEAD(&td->applications);
+	spin_lock_init(&td->application_list_lock);
 	INIT_LIST_HEAD(&td->reports);
 
 	if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)