From: David Herrmann <dh.herrmann@gmail.com>
To: linux-input@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Henrik Rydberg <rydberg@euromail.se>,
David Herrmann <dh.herrmann@gmail.com>
Subject: [RFC PATCH] Input: evdev - drop redundant list-locking
Date: Sun, 20 Jul 2014 20:48:12 +0200 [thread overview]
Message-ID: <1405882092-7005-1-git-send-email-dh.herrmann@gmail.com> (raw)
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@gmail.com>
---
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
next reply other threads:[~2014-07-20 18:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-20 18:48 David Herrmann [this message]
2014-07-20 18:54 ` [RFC PATCH] Input: evdev - drop redundant list-locking Dmitry Torokhov
2014-07-20 19:00 ` David Herrmann
2014-07-20 19:53 ` Dmitry Torokhov
2014-07-20 20:03 ` [PATCH v2] " David Herrmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1405882092-7005-1-git-send-email-dh.herrmann@gmail.com \
--to=dh.herrmann@gmail.com \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=rydberg@euromail.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.