[RFC PATCH] Input: evdev - drop redundant list-locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



evdev->client_list is rcu-protected. There is no need to have a
separate spinlock just for the list. Either one is good enough, so lets
drop the spinlock.

Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
---
Hi

I stumbled across this one when doing some evdev reviews. Maybe I'm missing
something obvious and I should stop coding on Sundays. But the RCU-protection
should be enough here, right?

We _could_ do a synchronize_rcu() during evdev_attach_client() to guarantee that
new events are really delivered once it returns. But that seems rather pedantic
to me.

I'm also not sure why we use RCU here, anyway. I mean, there's no high
contention so a spinlock should be fine and would get rid of the very expensive
synchronize_rcu during close(). But then again, I don't really care enough to
write benchmarks for that..

Thanks
David

 drivers/input/evdev.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 7a25a7a..1f38bd1 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -34,7 +34,6 @@ struct evdev {
 	wait_queue_head_t wait;
 	struct evdev_client __rcu *grab;
 	struct list_head client_list;
-	spinlock_t client_lock; /* protects client_list */
 	struct mutex mutex;
 	struct device dev;
 	struct cdev cdev;
@@ -433,17 +432,13 @@ static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
 static void evdev_attach_client(struct evdev *evdev,
 				struct evdev_client *client)
 {
-	spin_lock(&evdev->client_lock);
 	list_add_tail_rcu(&client->node, &evdev->client_list);
-	spin_unlock(&evdev->client_lock);
 }
 
 static void evdev_detach_client(struct evdev *evdev,
 				struct evdev_client *client)
 {
-	spin_lock(&evdev->client_lock);
 	list_del_rcu(&client->node);
-	spin_unlock(&evdev->client_lock);
 	synchronize_rcu();
 }
 
@@ -485,10 +480,10 @@ static void evdev_hangup(struct evdev *evdev)
 {
 	struct evdev_client *client;
 
-	spin_lock(&evdev->client_lock);
-	list_for_each_entry(client, &evdev->client_list, node)
+	rcu_read_lock();
+	list_for_each_entry_rcu(client, &evdev->client_list, node)
 		kill_fasync(&client->fasync, SIGIO, POLL_HUP);
-	spin_unlock(&evdev->client_lock);
+	rcu_read_unlock();
 
 	wake_up_interruptible(&evdev->wait);
 }
@@ -1438,7 +1433,6 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
 	}
 
 	INIT_LIST_HEAD(&evdev->client_list);
-	spin_lock_init(&evdev->client_lock);
 	mutex_init(&evdev->mutex);
 	init_waitqueue_head(&evdev->wait);
 	evdev->exist = true;
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux