public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Nicolas Pitre <nico@fluxnic.net>
Cc: lkml <linux-kernel@vger.kernel.org>, Greg Kroah-Hartman <gregkh@suse.de>
Subject: Re: [PATCH 1/2] vcs: add poll/fasync support
Date: Mon, 4 Oct 2010 15:54:54 -0700	[thread overview]
Message-ID: <20101004155454.771f7b70.akpm@linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.2.00.1009302320360.1146@xanadu.home>

On Fri, 01 Oct 2010 00:10:23 -0400 (EDT)
Nicolas Pitre <nico@fluxnic.net> wrote:

> The /dev/vcs* devices are used, amongst other things, by accessibility
> applications such as BRLTTY to display the screen content onto refreshable
> braille displays.  Currently this is performed by constantly reading from
> /dev/vcsa0 whether or not the screen content has changed.  Given the
> default braille refresh rate of 25 times per second, this easily qualifies
> as the biggest source of wake-up events preventing laptops from entering
> deeper power saving states.
> 
> To avoid this periodic polling, let's add support for select()/poll() and
> SIGIO with the /dev/vcs* devices.  The implemented semantic is to report
> data availability whenever the corresponding vt has seen some update after
> the last read() operation.  The application still has to lseek() back
> as usual in order to read() the new data.
> 
> Not to create unwanted overhead, the needed data structure is allocated
> and the vt notification callback is registered only when the poll or
> fasync method is invoked for the first time per file instance.
> 
> ...
> 
> diff --git a/drivers/char/vc_screen.c b/drivers/char/vc_screen.c
> index bcce46c..9013573 100644
> --- a/drivers/char/vc_screen.c
> +++ b/drivers/char/vc_screen.c
> @@ -35,6 +35,10 @@
>  #include <linux/console.h>
>  #include <linux/device.h>
>  #include <linux/smp_lock.h>
> +#include <linux/sched.h>
> +#include <linux/poll.h>
> +#include <linux/signal.h>
> +#include <linux/slab.h>

Formally, we need fs.h and notifier.h (at lesat).  I'll fix that up.

>  #include <asm/uaccess.h>
>  #include <asm/byteorder.h>
> @@ -45,6 +49,78 @@
>  #undef addr
>  #define HEADER_SIZE	4
>  
> +struct vcs_poll_data {
> +	struct notifier_block notifier;
> +	unsigned int cons_num;
> +	int has_read;

It would be nice to document the meaning of has_read.  And consider
using the more appropriate `bool' type?

> +	wait_queue_head_t waitq;
> +	struct fasync_struct *fasync;
> +};
> +
> +static int
> +vcs_notifier(struct notifier_block *nb, unsigned long code, void *_param)
> +{
> +	struct vt_notifier_param *param = _param;
> +	struct vc_data *vc = param->vc;
> +	struct vcs_poll_data *poll =
> +		container_of(nb, struct vcs_poll_data, notifier);
> +	int currcons = poll->cons_num;
> +
> +	if (code != VT_UPDATE)
> +		return NOTIFY_DONE;
> +
> +	if (currcons == 0)
> +		currcons = fg_console;
> +	else
> +		currcons--;
> +	if (currcons != vc->vc_num)
> +		return NOTIFY_DONE;
> +
> +	poll->has_read = 0;
> +	wake_up_interruptible(&poll->waitq);
> +	kill_fasync(&poll->fasync, SIGIO, POLL_IN);
> +	return NOTIFY_OK;
> +}
> +
> +static void
> +vcs_poll_data_free(struct vcs_poll_data *poll)
> +{
> +	unregister_vt_notifier(&poll->notifier);
> +	kfree(poll);
> +}
> +
> +static struct vcs_poll_data *
> +vcs_poll_data_get(struct file *file)
> +{
> +	struct vcs_poll_data *poll = file->private_data;
> +
> +	if (poll)
> +		return poll;
> +
> +	poll = kzalloc(sizeof(*poll), GFP_KERNEL);
> +	if (!poll)
> +		return NULL;
> +	poll->cons_num = iminor(file->f_path.dentry->d_inode) & 127;
> +	init_waitqueue_head(&poll->waitq);
> +	poll->notifier.notifier_call = vcs_notifier;
> +	if (register_vt_notifier(&poll->notifier) != 0) {
> +		kfree(poll);
> +		return NULL;
> +	}
> +
> +	spin_lock(&file->f_lock);
> +	if (!file->private_data) {
> +		file->private_data = poll;
> +	} else {
> +		/* someone else raced ahead of us */
> +		vcs_poll_data_free(poll);
> +		poll = file->private_data;
> +	}
> +	spin_unlock(&file->f_lock);

What's the race-handling code here all about?

> +	return poll;
> +}
> +
>
> ...
>

  parent reply	other threads:[~2010-10-04 22:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-01  4:10 [PATCH 1/2] vcs: add poll/fasync support Nicolas Pitre
2010-10-01  9:31 ` Alan Cox
2010-10-04 21:01   ` Nicolas Pitre
2010-10-04 22:54 ` Andrew Morton [this message]
2010-10-05  3:51   ` Nicolas Pitre
2010-10-05  4:20     ` Andrew Morton
2010-10-05 18:22       ` Nicolas Pitre

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=20101004155454.771f7b70.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nico@fluxnic.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox