[systemd-devel] [PATCH] login: share VT-signal handler between sessions

Olivier Brunel jjk at jjacky.com
Mon Aug 11 10:57:15 PDT 2014


On 08/11/14 18:21, David Herrmann wrote:
> sd-event does not allow multiple handlers for a single signal. However,
> logind sets up signal handlers for each session with VT_PROCESS set (that
> is, it has an active controller). Therefore, registering multiple such
> controllers will fail.
> 
> Lets make the VT-handler global, as it's mostly trivial, anyway. This way,
> the sessions don't have to take care of that and we can simply acknowledge
> all VT-switch requests as we always did.
> ---
> Hi Olivier
> 
> Can you give this a try? It should solve your issues.

Yes, just tried it -- it does solve the issue.

Thanks guys,
-j

BTW there's a bug report opened about this issue:
https://bugs.freedesktop.org/show_bug.cgi?id=81932

> 
> Thanks
> David
> 
>  src/login/logind-session.c | 26 ++-----------------
>  src/login/logind-session.h |  1 -
>  src/login/logind.c         | 62 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+), 25 deletions(-)
> 
> diff --git a/src/login/logind-session.c b/src/login/logind-session.c
> index fdeacb1..26b6a90 100644
> --- a/src/login/logind-session.c
> +++ b/src/login/logind-session.c
> @@ -153,8 +153,6 @@ void session_free(Session *s) {
>  
>          hashmap_remove(s->manager->sessions, s->id);
>  
> -        s->vt_source = sd_event_source_unref(s->vt_source);
> -
>          free(s->state_file);
>          free(s);
>  }
> @@ -994,19 +992,9 @@ static int session_open_vt(Session *s) {
>          return s->vtfd;
>  }
>  
> -static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo *si, void *data) {
> -        Session *s = data;
> -
> -        if (s->vtfd >= 0)
> -                ioctl(s->vtfd, VT_RELDISP, 1);
> -
> -        return 0;
> -}
> -
>  void session_prepare_vt(Session *s) {
>          int vt, r;
>          struct vt_mode mode = { 0 };
> -        sigset_t mask;
>  
>          vt = session_open_vt(s);
>          if (vt < 0)
> @@ -1024,20 +1012,12 @@ void session_prepare_vt(Session *s) {
>          if (r < 0)
>                  goto error;
>  
> -        sigemptyset(&mask);
> -        sigaddset(&mask, SIGUSR1);
> -        sigprocmask(SIG_BLOCK, &mask, NULL);
> -
> -        r = sd_event_add_signal(s->manager->event, &s->vt_source, SIGUSR1, session_vt_fn, s);
> -        if (r < 0)
> -                goto error;
> -
>          /* Oh, thanks to the VT layer, VT_AUTO does not work with KD_GRAPHICS.
>           * So we need a dummy handler here which just acknowledges *all* VT
>           * switch requests. */
>          mode.mode = VT_PROCESS;
> -        mode.relsig = SIGUSR1;
> -        mode.acqsig = SIGUSR1;
> +        mode.relsig = SIGRTMIN;
> +        mode.acqsig = SIGRTMIN + 1;
>          r = ioctl(vt, VT_SETMODE, &mode);
>          if (r < 0)
>                  goto error;
> @@ -1058,8 +1038,6 @@ void session_restore_vt(Session *s) {
>          if (vt < 0)
>                  return;
>  
> -        s->vt_source = sd_event_source_unref(s->vt_source);
> -
>          ioctl(vt, KDSETMODE, KD_TEXT);
>  
>          if (read_one_line_file("/sys/module/vt/parameters/default_utf8", &utf8) >= 0 && *utf8 == '1')
> diff --git a/src/login/logind-session.h b/src/login/logind-session.h
> index e62b76d..562332c 100644
> --- a/src/login/logind-session.h
> +++ b/src/login/logind-session.h
> @@ -98,7 +98,6 @@ struct Session {
>          Seat *seat;
>          unsigned int vtnr;
>          int vtfd;
> -        sd_event_source *vt_source;
>  
>          pid_t leader;
>          uint32_t audit_id;
> diff --git a/src/login/logind.c b/src/login/logind.c
> index ec5529d..b470916 100644
> --- a/src/login/logind.c
> +++ b/src/login/logind.c
> @@ -720,6 +720,46 @@ static int manager_connect_bus(Manager *m) {
>          return 0;
>  }
>  
> +static int manager_vt_switch(sd_event_source *src, const struct signalfd_siginfo *si, void *data) {
> +        Manager *m = data;
> +        Session *active, *iter;
> +
> +        /*
> +         * We got a VT-switch signal and we have to acknowledge it immediately.
> +         * Preferably, we'd just use m->seat0->active->vtfd, but unfortunately,
> +         * old user-space might run multiple sessions on a single VT, *sigh*.
> +         * Therefore, we have to iterate all sessions and find one with a vtfd
> +         * on the requested VT.
> +         * As only VTs with active controllers have VT_PROCESS set, our current
> +         * notion of the active VT might be wrong (for instance if the switch
> +         * happens while we setup VT_PROCESS). Therefore, read the current VT
> +         * first and then use s->active->vtnr as reference. Note that this is
> +         * not racy, as no further VT-switch can happen as long as we're in
> +         * synchronous VT_PROCESS mode.
> +         */
> +
> +        seat_read_active_vt(m->seat0);
> +
> +        active = m->seat0->active;
> +        if (!active || active->vtnr < 1) {
> +                log_warning("Received VT_PROCESS signal without a registered session on that VT.");
> +                return 0;
> +        }
> +
> +        if (active->vtfd >= 0) {
> +                ioctl(active->vtfd, VT_RELDISP, 1);
> +        } else {
> +                LIST_FOREACH(sessions_by_seat, iter, m->seat0->sessions) {
> +                        if (iter->vtnr == active->vtnr && iter->vtfd >= 0) {
> +                                ioctl(iter->vtfd, VT_RELDISP, 1);
> +                                break;
> +                        }
> +                }
> +        }
> +
> +        return 0;
> +}
> +
>  static int manager_connect_console(Manager *m) {
>          int r;
>  
> @@ -750,6 +790,28 @@ static int manager_connect_console(Manager *m) {
>                  return r;
>          }
>  
> +        /*
> +         * SIGRTMIN is used as global VT-release signal, SIGRTMIN + 1 is used
> +         * as VT-acquire signal. We ignore any acquire-events (yes, we still
> +         * have to provide a valid signal-number for it!) and acknowledge all
> +         * release events immediately.
> +         */
> +
> +        if (SIGRTMIN + 1 > SIGRTMAX) {
> +                log_error("Not enough real-time signals available: %u-%u", SIGRTMIN, SIGRTMAX);
> +                return -EINVAL;
> +        }
> +
> +        r = sigprocmask_many(SIG_BLOCK, SIGRTMIN, SIGRTMIN + 1, -1);
> +        if (r < 0) {
> +                log_error("Cannot block SIGRTMIN and SIGRTMIN + 1: %s", strerror(-r));
> +                return r;
> +        }
> +
> +        r = sd_event_add_signal(m->event, NULL, SIGRTMIN, manager_vt_switch, m);
> +        if (r < 0)
> +                return r;
> +
>          return 0;
>  }
>  
> 



More information about the systemd-devel mailing list