Skip to content

Commit 924c5ee

Browse files
committed
Input: serio - use guard notation when acquiring mutexes and spinlocks
Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section. Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Dmitry Torokhov <[email protected]>
1 parent f7d15dc commit 924c5ee

File tree

1 file changed

+66
-99
lines changed

1 file changed

+66
-99
lines changed

drivers/input/serio/serio.c

Lines changed: 66 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -38,33 +38,27 @@ static void serio_attach_driver(struct serio_driver *drv);
3838

3939
static int serio_connect_driver(struct serio *serio, struct serio_driver *drv)
4040
{
41-
int retval;
42-
43-
mutex_lock(&serio->drv_mutex);
44-
retval = drv->connect(serio, drv);
45-
mutex_unlock(&serio->drv_mutex);
41+
guard(mutex)(&serio->drv_mutex);
4642

47-
return retval;
43+
return drv->connect(serio, drv);
4844
}
4945

5046
static int serio_reconnect_driver(struct serio *serio)
5147
{
52-
int retval = -1;
48+
guard(mutex)(&serio->drv_mutex);
5349

54-
mutex_lock(&serio->drv_mutex);
5550
if (serio->drv && serio->drv->reconnect)
56-
retval = serio->drv->reconnect(serio);
57-
mutex_unlock(&serio->drv_mutex);
51+
return serio->drv->reconnect(serio);
5852

59-
return retval;
53+
return -1;
6054
}
6155

6256
static void serio_disconnect_driver(struct serio *serio)
6357
{
64-
mutex_lock(&serio->drv_mutex);
58+
guard(mutex)(&serio->drv_mutex);
59+
6560
if (serio->drv)
6661
serio->drv->disconnect(serio);
67-
mutex_unlock(&serio->drv_mutex);
6862
}
6963

7064
static int serio_match_port(const struct serio_device_id *ids, struct serio *serio)
@@ -147,17 +141,15 @@ static LIST_HEAD(serio_event_list);
147141
static struct serio_event *serio_get_event(void)
148142
{
149143
struct serio_event *event = NULL;
150-
unsigned long flags;
151144

152-
spin_lock_irqsave(&serio_event_lock, flags);
145+
guard(spinlock_irqsave)(&serio_event_lock);
153146

154147
if (!list_empty(&serio_event_list)) {
155148
event = list_first_entry(&serio_event_list,
156149
struct serio_event, node);
157150
list_del_init(&event->node);
158151
}
159152

160-
spin_unlock_irqrestore(&serio_event_lock, flags);
161153
return event;
162154
}
163155

@@ -171,9 +163,8 @@ static void serio_remove_duplicate_events(void *object,
171163
enum serio_event_type type)
172164
{
173165
struct serio_event *e, *next;
174-
unsigned long flags;
175166

176-
spin_lock_irqsave(&serio_event_lock, flags);
167+
guard(spinlock_irqsave)(&serio_event_lock);
177168

178169
list_for_each_entry_safe(e, next, &serio_event_list, node) {
179170
if (object == e->object) {
@@ -189,15 +180,13 @@ static void serio_remove_duplicate_events(void *object,
189180
serio_free_event(e);
190181
}
191182
}
192-
193-
spin_unlock_irqrestore(&serio_event_lock, flags);
194183
}
195184

196185
static void serio_handle_event(struct work_struct *work)
197186
{
198187
struct serio_event *event;
199188

200-
mutex_lock(&serio_mutex);
189+
guard(mutex)(&serio_mutex);
201190

202191
while ((event = serio_get_event())) {
203192

@@ -228,20 +217,16 @@ static void serio_handle_event(struct work_struct *work)
228217
serio_remove_duplicate_events(event->object, event->type);
229218
serio_free_event(event);
230219
}
231-
232-
mutex_unlock(&serio_mutex);
233220
}
234221

235222
static DECLARE_WORK(serio_event_work, serio_handle_event);
236223

237224
static int serio_queue_event(void *object, struct module *owner,
238225
enum serio_event_type event_type)
239226
{
240-
unsigned long flags;
241227
struct serio_event *event;
242-
int retval = 0;
243228

244-
spin_lock_irqsave(&serio_event_lock, flags);
229+
guard(spinlock_irqsave)(&serio_event_lock);
245230

246231
/*
247232
* Scan event list for the other events for the same serio port,
@@ -253,24 +238,22 @@ static int serio_queue_event(void *object, struct module *owner,
253238
list_for_each_entry_reverse(event, &serio_event_list, node) {
254239
if (event->object == object) {
255240
if (event->type == event_type)
256-
goto out;
241+
return 0;
257242
break;
258243
}
259244
}
260245

261246
event = kmalloc(sizeof(*event), GFP_ATOMIC);
262247
if (!event) {
263248
pr_err("Not enough memory to queue event %d\n", event_type);
264-
retval = -ENOMEM;
265-
goto out;
249+
return -ENOMEM;
266250
}
267251

268252
if (!try_module_get(owner)) {
269253
pr_warn("Can't get module reference, dropping event %d\n",
270254
event_type);
271255
kfree(event);
272-
retval = -EINVAL;
273-
goto out;
256+
return -EINVAL;
274257
}
275258

276259
event->type = event_type;
@@ -280,9 +263,7 @@ static int serio_queue_event(void *object, struct module *owner,
280263
list_add_tail(&event->node, &serio_event_list);
281264
queue_work(system_long_wq, &serio_event_work);
282265

283-
out:
284-
spin_unlock_irqrestore(&serio_event_lock, flags);
285-
return retval;
266+
return 0;
286267
}
287268

288269
/*
@@ -292,18 +273,15 @@ static int serio_queue_event(void *object, struct module *owner,
292273
static void serio_remove_pending_events(void *object)
293274
{
294275
struct serio_event *event, *next;
295-
unsigned long flags;
296276

297-
spin_lock_irqsave(&serio_event_lock, flags);
277+
guard(spinlock_irqsave)(&serio_event_lock);
298278

299279
list_for_each_entry_safe(event, next, &serio_event_list, node) {
300280
if (event->object == object) {
301281
list_del_init(&event->node);
302282
serio_free_event(event);
303283
}
304284
}
305-
306-
spin_unlock_irqrestore(&serio_event_lock, flags);
307285
}
308286

309287
/*
@@ -315,23 +293,19 @@ static void serio_remove_pending_events(void *object)
315293
static struct serio *serio_get_pending_child(struct serio *parent)
316294
{
317295
struct serio_event *event;
318-
struct serio *serio, *child = NULL;
319-
unsigned long flags;
296+
struct serio *serio;
320297

321-
spin_lock_irqsave(&serio_event_lock, flags);
298+
guard(spinlock_irqsave)(&serio_event_lock);
322299

323300
list_for_each_entry(event, &serio_event_list, node) {
324301
if (event->type == SERIO_REGISTER_PORT) {
325302
serio = event->object;
326-
if (serio->parent == parent) {
327-
child = serio;
328-
break;
329-
}
303+
if (serio->parent == parent)
304+
return serio;
330305
}
331306
}
332307

333-
spin_unlock_irqrestore(&serio_event_lock, flags);
334-
return child;
308+
return NULL;
335309
}
336310

337311
/*
@@ -382,29 +356,27 @@ static ssize_t drvctl_store(struct device *dev, struct device_attribute *attr, c
382356
struct device_driver *drv;
383357
int error;
384358

385-
error = mutex_lock_interruptible(&serio_mutex);
386-
if (error)
387-
return error;
388-
389-
if (!strncmp(buf, "none", count)) {
390-
serio_disconnect_port(serio);
391-
} else if (!strncmp(buf, "reconnect", count)) {
392-
serio_reconnect_subtree(serio);
393-
} else if (!strncmp(buf, "rescan", count)) {
394-
serio_disconnect_port(serio);
395-
serio_find_driver(serio);
396-
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
397-
} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
398-
serio_disconnect_port(serio);
399-
error = serio_bind_driver(serio, to_serio_driver(drv));
400-
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
401-
} else {
402-
error = -EINVAL;
359+
scoped_cond_guard(mutex_intr, return -EINTR, &serio_mutex) {
360+
if (!strncmp(buf, "none", count)) {
361+
serio_disconnect_port(serio);
362+
} else if (!strncmp(buf, "reconnect", count)) {
363+
serio_reconnect_subtree(serio);
364+
} else if (!strncmp(buf, "rescan", count)) {
365+
serio_disconnect_port(serio);
366+
serio_find_driver(serio);
367+
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
368+
} else if ((drv = driver_find(buf, &serio_bus)) != NULL) {
369+
serio_disconnect_port(serio);
370+
error = serio_bind_driver(serio, to_serio_driver(drv));
371+
serio_remove_duplicate_events(serio, SERIO_RESCAN_PORT);
372+
if (error)
373+
return error;
374+
} else {
375+
return -EINVAL;
376+
}
403377
}
404378

405-
mutex_unlock(&serio_mutex);
406-
407-
return error ? error : count;
379+
return count;
408380
}
409381

410382
static ssize_t serio_show_bind_mode(struct device *dev, struct device_attribute *attr, char *buf)
@@ -526,9 +498,9 @@ static void serio_add_port(struct serio *serio)
526498
int error;
527499

528500
if (parent) {
529-
serio_pause_rx(parent);
501+
guard(serio_pause_rx)(parent);
502+
530503
list_add_tail(&serio->child_node, &parent->children);
531-
serio_continue_rx(parent);
532504
}
533505

534506
list_add_tail(&serio->node, &serio_list);
@@ -560,9 +532,9 @@ static void serio_destroy_port(struct serio *serio)
560532
serio->stop(serio);
561533

562534
if (serio->parent) {
563-
serio_pause_rx(serio->parent);
535+
guard(serio_pause_rx)(serio->parent);
536+
564537
list_del_init(&serio->child_node);
565-
serio_continue_rx(serio->parent);
566538
serio->parent = NULL;
567539
}
568540

@@ -701,10 +673,10 @@ EXPORT_SYMBOL(__serio_register_port);
701673
*/
702674
void serio_unregister_port(struct serio *serio)
703675
{
704-
mutex_lock(&serio_mutex);
676+
guard(mutex)(&serio_mutex);
677+
705678
serio_disconnect_port(serio);
706679
serio_destroy_port(serio);
707-
mutex_unlock(&serio_mutex);
708680
}
709681
EXPORT_SYMBOL(serio_unregister_port);
710682

@@ -715,12 +687,12 @@ void serio_unregister_child_port(struct serio *serio)
715687
{
716688
struct serio *s, *next;
717689

718-
mutex_lock(&serio_mutex);
690+
guard(mutex)(&serio_mutex);
691+
719692
list_for_each_entry_safe(s, next, &serio->children, child_node) {
720693
serio_disconnect_port(s);
721694
serio_destroy_port(s);
722695
}
723-
mutex_unlock(&serio_mutex);
724696
}
725697
EXPORT_SYMBOL(serio_unregister_child_port);
726698

@@ -784,10 +756,10 @@ static void serio_driver_remove(struct device *dev)
784756

785757
static void serio_cleanup(struct serio *serio)
786758
{
787-
mutex_lock(&serio->drv_mutex);
759+
guard(mutex)(&serio->drv_mutex);
760+
788761
if (serio->drv && serio->drv->cleanup)
789762
serio->drv->cleanup(serio);
790-
mutex_unlock(&serio->drv_mutex);
791763
}
792764

793765
static void serio_shutdown(struct device *dev)
@@ -850,7 +822,7 @@ void serio_unregister_driver(struct serio_driver *drv)
850822
{
851823
struct serio *serio;
852824

853-
mutex_lock(&serio_mutex);
825+
guard(mutex)(&serio_mutex);
854826

855827
drv->manual_bind = true; /* so serio_find_driver ignores it */
856828
serio_remove_pending_events(drv);
@@ -866,15 +838,14 @@ void serio_unregister_driver(struct serio_driver *drv)
866838
}
867839

868840
driver_unregister(&drv->driver);
869-
mutex_unlock(&serio_mutex);
870841
}
871842
EXPORT_SYMBOL(serio_unregister_driver);
872843

873844
static void serio_set_drv(struct serio *serio, struct serio_driver *drv)
874845
{
875-
serio_pause_rx(serio);
846+
guard(serio_pause_rx)(serio);
847+
876848
serio->drv = drv;
877-
serio_continue_rx(serio);
878849
}
879850

880851
static int serio_bus_match(struct device *dev, const struct device_driver *drv)
@@ -935,14 +906,14 @@ static int serio_resume(struct device *dev)
935906
struct serio *serio = to_serio_port(dev);
936907
int error = -ENOENT;
937908

938-
mutex_lock(&serio->drv_mutex);
939-
if (serio->drv && serio->drv->fast_reconnect) {
940-
error = serio->drv->fast_reconnect(serio);
941-
if (error && error != -ENOENT)
942-
dev_warn(dev, "fast reconnect failed with error %d\n",
943-
error);
909+
scoped_guard(mutex, &serio->drv_mutex) {
910+
if (serio->drv && serio->drv->fast_reconnect) {
911+
error = serio->drv->fast_reconnect(serio);
912+
if (error && error != -ENOENT)
913+
dev_warn(dev, "fast reconnect failed with error %d\n",
914+
error);
915+
}
944916
}
945-
mutex_unlock(&serio->drv_mutex);
946917

947918
if (error) {
948919
/*
@@ -989,21 +960,17 @@ EXPORT_SYMBOL(serio_close);
989960
irqreturn_t serio_interrupt(struct serio *serio,
990961
unsigned char data, unsigned int dfl)
991962
{
992-
unsigned long flags;
993-
irqreturn_t ret = IRQ_NONE;
963+
guard(spinlock_irqsave)(&serio->lock);
994964

995-
spin_lock_irqsave(&serio->lock, flags);
965+
if (likely(serio->drv))
966+
return serio->drv->interrupt(serio, data, dfl);
996967

997-
if (likely(serio->drv)) {
998-
ret = serio->drv->interrupt(serio, data, dfl);
999-
} else if (!dfl && device_is_registered(&serio->dev)) {
968+
if (!dfl && device_is_registered(&serio->dev)) {
1000969
serio_rescan(serio);
1001-
ret = IRQ_HANDLED;
970+
return IRQ_HANDLED;
1002971
}
1003972

1004-
spin_unlock_irqrestore(&serio->lock, flags);
1005-
1006-
return ret;
973+
return IRQ_NONE;
1007974
}
1008975
EXPORT_SYMBOL(serio_interrupt);
1009976

0 commit comments

Comments
 (0)