[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120214142540.04de41df@notabene.brown>
Date: Tue, 14 Feb 2012 14:25:40 +1100
From: NeilBrown <neilb@...e.de>
To: Arve Hjønnevåg <arve@...roid.com>
Cc: linux-input@...r.kernel.org, linux-pm@...r.kernel.org,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Matthew Garrett <mjg@...hat.com>,
Chase Douglas <chase.douglas@...onical.com>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Input: Add ioctl to block suspend while event queue is
not empty.
On Fri, 20 Jan 2012 18:24:16 -0800 Arve Hjønnevåg <arve@...roid.com> wrote:
> Add an ioctl, EVIOCSSUSPENDBLOCK, to block suspend while the event
> queue is not empty. This allows userspace code to process input
> events while the device appears to be asleep.
>
> Signed-off-by: Arve Hjønnevåg <arve@...roid.com>
This is exactly the sort of "feature creep" that I worried about in my reply
to Rafael's recent "autosleep" patches.
A particular issue here: This patch allows any process that can open an
input device to keep the device awake - by not reading an event that has
arrived (whether due to incompetence or malice).
So either we would need strict controls on who can open /dev/input/eventX,
or be happy that any process can disable suspend.
Or add some extra feature-creep to provide access control.
(or just keep this stuff out of the kernel and let a user-space daemon make
those decisions).
NeilBrown
> ---
> drivers/input/evdev.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/input.h | 3 ++
> 2 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 76457d5..e212757 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -43,6 +43,7 @@ struct evdev_client {
> unsigned int tail;
> unsigned int packet_head; /* [future] position of the first element of next packet */
> spinlock_t buffer_lock; /* protects access to buffer, head and tail */
> + struct wakeup_source *wakeup_source;
> struct fasync_struct *fasync;
> struct evdev *evdev;
> struct list_head node;
> @@ -75,10 +76,14 @@ static void evdev_pass_event(struct evdev_client *client,
> client->buffer[client->tail].value = 0;
>
> client->packet_head = client->tail;
> + if (client->wakeup_source)
> + __pm_relax(client->wakeup_source);
> }
>
> if (event->type == EV_SYN && event->code == SYN_REPORT) {
> client->packet_head = client->head;
> + if (client->wakeup_source)
> + __pm_stay_awake(client->wakeup_source);
> kill_fasync(&client->fasync, SIGIO, POLL_IN);
> }
>
> @@ -255,6 +260,10 @@ static int evdev_release(struct inode *inode, struct file *file)
> mutex_unlock(&evdev->mutex);
>
> evdev_detach_client(evdev, client);
> + if (client->wakeup_source) {
> + __pm_relax(client->wakeup_source);
> + wakeup_source_unregister(client->wakeup_source);
> + }
> kfree(client);
>
> evdev_close_device(evdev);
> @@ -373,6 +382,9 @@ static int evdev_fetch_next_event(struct evdev_client *client,
> if (have_event) {
> *event = client->buffer[client->tail++];
> client->tail &= client->bufsize - 1;
> + if (client->wakeup_source &&
> + client->packet_head == client->tail)
> + __pm_relax(client->wakeup_source);
> }
>
> spin_unlock_irq(&client->buffer_lock);
> @@ -623,6 +635,48 @@ static int evdev_handle_set_keycode_v2(struct input_dev *dev, void __user *p)
> return input_set_keycode(dev, &ke);
> }
>
> +static int evdev_enable_suspend_block(struct evdev *evdev,
> + struct evdev_client *client)
> +{
> + struct wakeup_source *ws;
> + char name[28];
> +
> + if (client->wakeup_source)
> + return 0;
> +
> + snprintf(name, sizeof(name), "%s-%d",
> + dev_name(&evdev->dev), task_tgid_vnr(current));
> +
> + ws = wakeup_source_register(name);
> + if (!ws)
> + return -ENOMEM;
> +
> + spin_lock_irq(&client->buffer_lock);
> + client->wakeup_source = ws;
> + if (client->packet_head != client->tail)
> + __pm_stay_awake(client->wakeup_source);
> + spin_unlock_irq(&client->buffer_lock);
> + return 0;
> +}
> +
> +static int evdev_disable_suspend_block(struct evdev *evdev,
> + struct evdev_client *client)
> +{
> + struct wakeup_source *ws;
> +
> + spin_lock_irq(&client->buffer_lock);
> + ws = client->wakeup_source;
> + client->wakeup_source = NULL;
> + spin_unlock_irq(&client->buffer_lock);
> +
> + if (ws) {
> + __pm_relax(ws);
> + wakeup_source_unregister(ws);
> + }
> +
> + return 0;
> +}
> +
> static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> void __user *p, int compat_mode)
> {
> @@ -696,6 +750,15 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>
> case EVIOCSKEYCODE_V2:
> return evdev_handle_set_keycode_v2(dev, p);
> +
> + case EVIOCGSUSPENDBLOCK:
> + return put_user(!!client->wakeup_source, ip);
> +
> + case EVIOCSSUSPENDBLOCK:
> + if (p)
> + return evdev_enable_suspend_block(evdev, client);
> + else
> + return evdev_disable_suspend_block(evdev, client);
> }
>
> size = _IOC_SIZE(cmd);
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 3862e32..daf0177 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -129,6 +129,9 @@ struct input_keymap_entry {
>
> #define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
>
> +#define EVIOCGSUSPENDBLOCK _IOR('E', 0x91, int) /* get suspend block enable */
> +#define EVIOCSSUSPENDBLOCK _IOW('E', 0x91, int) /* set suspend block enable */
> +
> /*
> * Device properties and quirks
> */
Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)
Powered by blists - more mailing lists