Hi, On Tue, Jun 27, 2017 at 01:38:35AM +0200, Samuel Thibault wrote: > Okash Khawaja, on dim. 25 juin 2017 10:21:26 +0100, wrote: > > Third one is interesting. I haven't investigated it yet, but it seems to > > throw a kernel oops from tty_ldisc_reinit. > > So it's probably some missing cleanups in the speakup code, which just > needs fixing :) Yes, there are a couple of things. The kernel oops comes from ldisc. Because we call tty_ldisc_release on module exit but don't set tty->ldiscs[N_SPEAKUP] to NULL, the next time that tty is opened, it tries to use N_SPEAKUP ldisc whose memory was released in tty_ldisc_release. Calling tty_ldisc_unregister resolves it by setting tty->ldiscs[N_SPEAKUP] to NULL. Next, I think, we need to restore tty->termios.c_line to the ldisc before speakup was loaded, because that's what is used when opening tty [1]. Locally, I have done that by caching original ldisc and then calling tty_set_termios_ldisc on module exit which resolves that issue. However, in kernel tty_set_termios_ldisc is static function and will need to be exported if we want to call it. Finally, there is tty->count mismatch. On module exit, we don't decrement tty->count which was incremented by tty_open_by_driver. I don't know yet how to fix that. So in summary, we need above changes, and also the flags to prevent contention between user and kernel space. > > > We can avoid 2 and 3 above by calling tty_open_by_driver only when > > speakup is built into kernel and not when loaded as module. > > That looks odd to me :) Yes it does :) Not very familiar with kernel side use cases yet. My thinking was to use different way of acquiring tty_struct when loaded as module, because then we will have the device file. Probably not very elegant. Thanks, Okash [1] http://elixir.free-electrons.com/linux/latest/source/drivers/tty/tty_io.c#L1484 _______________________________________________ Speakup mailing list Speakup@xxxxxxxxxxxxxxxxx http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
