Re: svn commit: r190857 - head/sys/dev/kbdmux

Maksim Yevmenkin <emax_at_freebsd.org> wrote:

> On Sun, Apr 12, 2009 at 8:03 AM, Fabian Keil
> <freebsd-listen_at_fabiankeil.de> wrote:
> > Maksim Yevmenkin <emax_at_FreeBSD.org> wrote:
> >
> >> Author: emax
> >> Date: Wed Apr  8 20:52:30 2009
> >> New Revision: 190857
> >> URL: http://svn.freebsd.org/changeset/base/190857
> >>
> >> Log:
> >>   Undo SVN rev 183283
> >>
> >>   Do not use Giant for kbdmux(4) locking. This is wrong and apparently
> >>   causing more problems than it solves. This will re-open the issue
> >>   where interrupt handlers may race with kbdmux(4) in polling mode.
> >>   Typical symptoms include (but not limited to) duplicated and/or
> >>   missing characters when low level console functions (such as gets)
> >>   are used while interrupts are enabled (for example geli password
> >>   prompt, mountroot prompt etc.)
> >>
> >>   MFC after:  3 days
> >>
> >> Modified:
> >>   head/sys/dev/kbdmux/kbdmux.c

[...]

> > Not even enabling the "visible characters" option helps
> > because obviously backspace is broken too.
> 
> if you do not need kbdmix(4) you might just want to disable it on your
> system. i think it should help with your particular problem.

Removing kbdmux from the kernel does indeed work around the problem.

> > Before theses locks were introduces I worked around the problem
> > with this gets() hack (which forced me to reduce the key entropy):
> > http://www.fabiankeil.de/sourcecode/freebsd/gets-no-duplicates.diff
> > and now I will simply revert your commit locally, but I assume I'm
> > not the only geli user who prefers to be able to boot the system
> > without local patches.
> 
> if your primary keyboard is atkbd(4), you might want to try the
> following patch. it is completely untested (i did not even compile
> it), so be warned ...
> 
> Index: atkbd.c
> ===================================================================
> --- atkbd.c     (revision 190905)
> +++ atkbd.c     (working copy)
> _at__at_ -476,7 +476,7 _at__at_
>  static int
>  atkbd_intr(keyboard_t *kbd, void *arg)
>  {
> -       atkbd_state_t *state;
> +       atkbd_state_t *state = (atkbd_state_t *)kbd->kb_data;
>         int delay[2];
>         int c;
> 
> _at__at_ -485,7 +485,6 _at__at_
>                  * The keyboard was not detected before;
>                  * it must have been reconnected!
>                  */
> -               state = (atkbd_state_t *)kbd->kb_data;
>                 init_keyboard(state->kbdc, &kbd->kb_type,
>                               kbd->kb_config);
>                 KBD_FOUND_DEVICE(kbd);
> _at__at_ -497,6 +496,9 _at__at_
>         }
> 
>         if (KBD_IS_ACTIVE(kbd) && KBD_IS_BUSY(kbd)) {
> +               if (state->ks_polling)
> +                       return 0;
> +
>                 /* let the callback function to process the input */
>                 (*kbd->kb_callback.kc_func)(kbd, KBDIO_KEYINPUT,
>                                             kbd->kb_callback.kc_arg);
> 

It compiles alright but once the system is running the keyboard
no longer works at all. I tested the patch with kbdmux already
disabled, but I assume it doesn't make a difference.

Anyway, I don't need kbdmux, so having to remove it is no problem.
Thanks a lot.

Fabian