Re: [PATCH 29/49] vcs: add poll/fasync support

[Nicolas Pitre]

On Wed, 10 Nov 2010, Kay Sievers wrote:

> On Wed, Nov 10, 2010 at 02:26, Nicolas Pitre
> <nicolas.pitre@xxxxxxxxxxxxx> wrote:
> > On Wed, 10 Nov 2010, Kay Sievers wrote:
> >
> >> On Fri, Oct 22, 2010 at 20:21, Greg Kroah-Hartman <gregkh@xxxxxxx> wrote:
> >> > From: Nicolas Pitre <nico@xxxxxxxxxxx>
> >> >
> >> > 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@xxxxxxxxxxxxx>
---
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;
 }