Currently, the VT_WAITACTIVE ioctl can be used to wait until a
specific VT becomes _active_. This is used by ConsoleKit to follow
which VT is the active one. This patch extends this logic in a simple
way, so that it can be used to wait until a specific VT becomes
_inactive_.
If the argument passed to VT_WAITACTIVE is positive the current
behaviour (waiting until active) is exposed. However, if it is negative
the 'inverse' behaviour (waiting until inactive) is exposed.
Why all this?
Currently ConsoleKit creates 64 seperate threads, one for each
theoretical VT and calls VT_WAITACTIVE in them, once for each VT. It thus
will get a woken up once for each VT change. Having that many threads
around is certainly ugly and also racy, since multiple quick VT changes
might be processed in the wrong order. With this patch CK can simply call
VT_WAITACTIVE in a loop for whatever VT is currently considered active
and will then get a wakeup when it isn't anymore. Then it can reread the
current VT index and reenter VT_WAITACTIVE again. This allows CK to run
only one thread instead of 64 to watch the VT status and also fixes the
ordering race pointed out above.
How use this?
/* this will wait until VT 5 is activated */
ioctl(0, VT_WAITACTIVE, 5L);
/* this will wait until VT 5 is deactivated again */
ioctl(0, VT_WAITACTIVE, -5L);
ConsoleKit would probably just call it in a loop like this:
for (;;) {
struct vt_stat st;
ioctl(0, VT_GETSTATE, &st);
printf("console %s is active.\n", st.v_active);
ioctl(0, VT_WAITACTIVE, (signed long) - st.v_active);
}
I tested this and it seems to work fine.
Signed-off-by: Lennart Poettering <[email protected]>
---
drivers/char/vt_ioctl.c | 17 +++++++++++++----
include/linux/vt_kern.h | 2 +-
kernel/power/console.c | 4 ++--
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index e6ce632..3fe8988 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -843,14 +843,22 @@ int vt_ioctl(struct tty_struct *tty, struct file * file,
/*
* wait until the specified VT has been activated
*/
- case VT_WAITACTIVE:
+ case VT_WAITACTIVE: {
+ signed long s_arg = arg;
+ bool active;
+
if (!perm)
goto eperm;
+
+ active = s_arg > 0;
+ arg = active ? s_arg : -s_arg;
+
if (arg == 0 || arg > MAX_NR_CONSOLES)
ret = -ENXIO;
else
- ret = vt_waitactive(arg - 1);
+ ret = vt_waitactive(arg - 1, active);
break;
+ }
/*
* If a vt is under process control, the kernel will not switch to it
@@ -1181,7 +1189,7 @@ static DECLARE_WAIT_QUEUE_HEAD(vt_activate_queue);
* Sleeps until a vt is activated, or the task is interrupted. Returns
* 0 if activation, -EINTR if interrupted by a signal handler.
*/
-int vt_waitactive(int vt)
+int vt_waitactive(int vt, bool active)
{
int retval;
DECLARE_WAITQUEUE(wait, current);
@@ -1199,7 +1207,8 @@ int vt_waitactive(int vt)
*/
acquire_console_sem();
set_current_state(TASK_INTERRUPTIBLE);
- if (vt == fg_console) {
+ if ((active && vt == fg_console) ||
+ (!active && vt != fg_console)) {
release_console_sem();
break;
}
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index 2f11134..4d8a9b8 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -91,7 +91,7 @@ int con_copy_unimap(struct vc_data *dst_vc, struct vc_data *src_vc);
#endif
/* vt.c */
-int vt_waitactive(int vt);
+int vt_waitactive(int vt, bool active);
void change_console(struct vc_data *new_vc);
void reset_vc(struct vc_data *vc);
extern int unbind_con_driver(const struct consw *csw, int first, int last,
diff --git a/kernel/power/console.c b/kernel/power/console.c
index a3961b2..146367e 100644
--- a/kernel/power/console.c
+++ b/kernel/power/console.c
@@ -60,7 +60,7 @@ int pm_prepare_console(void)
}
release_console_sem();
- if (vt_waitactive(SUSPEND_CONSOLE)) {
+ if (vt_waitactive(SUSPEND_CONSOLE, true)) {
pr_debug("Suspend: Can't switch VCs.");
return 1;
}
@@ -79,7 +79,7 @@ void pm_restore_console(void)
set_console(orig_fgconsole);
release_console_sem();
- if (vt_waitactive(orig_fgconsole)) {
+ if (vt_waitactive(orig_fgconsole, true)) {
pr_debug("Resume: Can't switch VCs.");
return;
}
--
1.6.3.2
Lennart
--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net ICQ# 11060553
http://0pointer.net/lennart/ GnuPG 0x1A015CC4
On Tue, 30 Jun 2009 02:00:04 +0200
Lennart Poettering <[email protected]> wrote:
> Currently, the VT_WAITACTIVE ioctl can be used to wait until a
> specific VT becomes _active_.
Well sort of - by the time it returns another VT may have switched again
unless you are using all the VT locking mess. Ditto with your changesit
seems.
> This is used by ConsoleKit
Yes someone posted a similar patch over a year ago (but ignored the
request for a signed-off-by) and his consolekit bug which was marked "Not
a bug" and ignored. I'm aware of this issue and the original patch would
have been merged but for the signed-off-by: issue.
> which VT is the active one. This patch extends this logic in a simple
> way, so that it can be used to wait until a specific VT becomes
> _inactive_.
Only it may have become active again by the time the call returns or it
may have become active and inactive again and be missed.
> will get a woken up once for each VT change. Having that many threads
> around is certainly ugly and also racy, since multiple quick VT changes
Good to see this has finally been accepted.
> /* this will wait until VT 5 is activated */
> ioctl(0, VT_WAITACTIVE, 5L);
I'd rather
ioctl(0, VT_WAITINACTIVE, ...)
[and both waits should be bitmasks of consoles but its a bit late for
that]
but wouldn't the following be a more useful looking call for most stuff
struct vtevent vtinfo {
int event;
#define VT_EVENT_SWITCH 1
#define VT_EVENT_BLANK 2
#define VT_EVENT_UNBLANK 3
#define VT_EVENT_RESIZE 4
unsigned int old;
unsigned int new;
unsigned int pad[4];
}
ioctl(0, VT_WAITEVENT, &vtinfo);
either way adding magic "negative number" semantics to VT_WAITACTIVE
isn't very nice.
We have an open bug for VT blank reporting to user space (and the
notifier hooks internally to do it). We also have races with the implicit
races in the WAITfoo APIs unless you are using the VT locking/change
stuff for things like X11
It looks like the job could be done better once even if for the moment it
simply reports VT changes.
On Tue, 30 Jun 2009 02:00:04 +0200, Lennart Poettering said:
> ConsoleKit would probably just call it in a loop like this:
>
> for (;;) {
> struct vt_stat st;
> ioctl(0, VT_GETSTATE, &st);
> printf("console %s is active.\n", st.v_active);
> ioctl(0, VT_WAITACTIVE, (signed long) - st.v_active);
> }
We will include a documented way for ConsoleKit and similar to be able
to determine if the kernel has this feature, right? (If older kernels
return a -EBOGUS for ioctl(0,VT_WAITACTIVE,-1) that's probably good enough)