lkml.org 
[lkml]   [2010]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 29/49] vcs: add poll/fasync support
On Wed, 10 Nov 2010, Kay Sievers wrote:

> On Wed, Nov 10, 2010 at 02:26, Nicolas Pitre
> <nicolas.pitre@canonical.com> wrote:
> > On Wed, 10 Nov 2010, Kay Sievers wrote:
> >
> >> On Fri, Oct 22, 2010 at 20:21, Greg Kroah-Hartman <gregkh@suse.de> wrote:
> >> > From: Nicolas Pitre <nico@fluxnic.net>
> >> >
> >> > 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.
> >>
> >> Shouldn't it raise POLLPRI/POLLERR then, when it's not about new data
> >> to read? We do this for several files in the kernel where we just want
> >> to wakup someone, but the pretty well-defined semantics of poll()
> >> don't apply.
> >
> > Hmmm...  Maybe POLLPRI, but POLLERR makes no sense.
>
> I have no opinion about that, but it's what others do:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=31b07093c44a7a442394d44423e21d783f5523b8
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4508a7a734b111b8b7e39986237d84acb1168dd0
>
> The POLLIN should definitely go, it's POSIX defined. What we do here
> is more like an error that wakes up than a stream data to read. :)

OK. What about this then?
----- >8
Subject: [PATCH] vcs: make proper usage of the poll flags

Kay Sievers pointed out that usage of POLLIN is well defined by POSIX,
and the current usage here doesn't follow that definition. So let's
duplicate the same semantics as implemented by sysfs_poll() instead.

Signed-off-by: Nicolas Pitre <nicolas.pitre@canonical.com>
---
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 273ab44..a93856e 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -553,12 +553,12 @@ static unsigned int
vcs_poll(struct file *file, poll_table *wait)
{
struct vcs_poll_data *poll = vcs_poll_data_get(file);
- int ret = 0;
+ int ret = DEFAULT_POLLMASK|POLLERR|POLLPRI;

if (poll) {
poll_wait(file, &poll->waitq, wait);
- if (!poll->seen_last_update)
- ret = POLLIN | POLLRDNORM;
+ if (poll->seen_last_update)
+ ret = DEFAULT_POLLMASK;
}
return ret;
}
\
 
 \ /
  Last update: 2010-11-10 07:35    [W:0.104 / U:0.760 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site