[5/6] vcs: poll(): cope with a deallocated vt

Message ID 20190109035504.8413-6-nicolas.pitre@linaro.org
State Accepted
Commit 1bf931ab94a963851aa1dfba5d9f03f9f1ad8637
Headers show
Series
  • assorted console/vt/vcs fixes
Related show

Commit Message

Nicolas Pitre Jan. 9, 2019, 3:55 a.m.
When VT_DISALLOCATE is used on a vt, user space waiting with poll() on
the corresponding /dev/vcs device is not awakened. This is now fixed by
returning POLLHUP|POLLERR to user space.

Also, in the normal screen update case, we don't set POLLERR anymore as
POLLPRI alone is a much more logical response in a non-error situation,
saving some confusion on the user space side. The only known user app
making use of poll() on /dev/vcs* is BRLTTY which is known to cope with
that change already, so the risk of breakage is pretty much nonexistent.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

---
 drivers/tty/vt/vc_screen.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

-- 
2.20.1

Comments

Nicolas Pitre Jan. 10, 2019, 12:13 a.m. | #1
On Tue, 8 Jan 2019, Nicolas Pitre wrote:

> When VT_DISALLOCATE is used on a vt, user space waiting with poll() on

> the corresponding /dev/vcs device is not awakened. This is now fixed by

> returning POLLHUP|POLLERR to user space.

> 

> Also, in the normal screen update case, we don't set POLLERR anymore as

> POLLPRI alone is a much more logical response in a non-error situation,

> saving some confusion on the user space side. The only known user app

> making use of poll() on /dev/vcs* is BRLTTY which is known to cope with

> that change already, so the risk of breakage is pretty much nonexistent.

> 

> Signed-off-by: Nicolas Pitre <nico@linaro.org>


That patch introduced a small unwanted behavior change that I intend to 
fix in a follow-up patch (it will be tagged [PATCH 7/6]). I prefer to go 
with a separate patch rather than respinning this one as this gives me 
the opportunity to separately document said behavior.

Nicolas

Patch

diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 3dba60825c..1bbe2a30cd 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -80,7 +80,7 @@ 
 struct vcs_poll_data {
 	struct notifier_block notifier;
 	unsigned int cons_num;
-	bool seen_last_update;
+	int event;
 	wait_queue_head_t waitq;
 	struct fasync_struct *fasync;
 };
@@ -94,7 +94,7 @@  vcs_notifier(struct notifier_block *nb, unsigned long code, void *_param)
 		container_of(nb, struct vcs_poll_data, notifier);
 	int currcons = poll->cons_num;
 
-	if (code != VT_UPDATE)
+	if (code != VT_UPDATE && code != VT_DEALLOCATE)
 		return NOTIFY_DONE;
 
 	if (currcons == 0)
@@ -104,7 +104,7 @@  vcs_notifier(struct notifier_block *nb, unsigned long code, void *_param)
 	if (currcons != vc->vc_num)
 		return NOTIFY_DONE;
 
-	poll->seen_last_update = false;
+	poll->event = code;
 	wake_up_interruptible(&poll->waitq);
 	kill_fasync(&poll->fasync, SIGIO, POLL_IN);
 	return NOTIFY_OK;
@@ -261,7 +261,7 @@  vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
 	poll = file->private_data;
 	if (count && poll)
-		poll->seen_last_update = true;
+		poll->event = 0;
 	read = 0;
 	ret = 0;
 	while (count) {
@@ -616,12 +616,21 @@  static __poll_t
 vcs_poll(struct file *file, poll_table *wait)
 {
 	struct vcs_poll_data *poll = vcs_poll_data_get(file);
-	__poll_t ret = DEFAULT_POLLMASK|EPOLLERR|EPOLLPRI;
+	__poll_t ret = DEFAULT_POLLMASK|EPOLLERR;
 
 	if (poll) {
 		poll_wait(file, &poll->waitq, wait);
-		if (poll->seen_last_update)
+		switch (poll->event) {
+		case VT_UPDATE:
+			ret = DEFAULT_POLLMASK|EPOLLPRI;
+			break;
+		case VT_DEALLOCATE:
+			ret = DEFAULT_POLLMASK|EPOLLHUP|EPOLLERR;
+			break;
+		case 0:
 			ret = DEFAULT_POLLMASK;
+			break;
+		}
 	}
 	return ret;
 }