On Fri, Jan 27, 2012 at 3:02 PM, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 27 Jan 2012 13:04:37 +0100
> richard -rw- weinberger <richard.weinberger@xxxxxxxxx> wrote:
>
>> On Fri, Jan 27, 2012 at 12:51 PM, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:
>> >> UML's console driver (arch/um/drivers/line.c) implements tty_operations.
>> >> The crash happens because the tty subsystem calls the driver's close()
>> >> function and later
>> >> write_room() or chars_in_buffer().
>> >>
>> >> write_room() and chars_in_buffer() fail badly because close() already
>> >> cleaned up the driver's private data...
>> >
>> > You don't want to do that.
>>
>> That's what i thought.
>>
>> >> Greg, is UML's assumption wrong that after closing the tty no call to
>> >> write_room() or chars_in_buffer() can happen?
>> >> I have no idea why systemd is able to trigger this, UML's console
>> >> driver is old and has always worked quite well.
>> >
>> > It's always been untrue but it's even more untrue nowdays. The tty layer
>> > objects are refcounted, and the code has had significant rewrites. line.c
>> > hidden away in uml hasn't been updated.
>> >
>> > I added a comment about 3 years ago pointing out another older change
>> > that was needed and that wasn't acted on either..
>> >
>> > Take a look at how all the other tty drivers use tty_port, how the ioctls
>> > have been supposed to work for the past few years and the callback
>> > changes, then use them.
>>
>> Can you recommend a well-written driver?
>
> drivers/mmc/card/sdio_uart.c
>
> uses just about all the features including handling hotplug and stuff you
> don't need.
>
> drivers/usb/serial/usb-serial.c
>
> may also be handy as it provides the interface but then calls into other
> driver code to do the work.
>
> Basically though you want a struct tty_port in your private data, either
> created at open, or usually more cleanly for the physical port lifetime
>
> tty_port_init()
>
> Sets it up, then set the port ops
>
> tty_port_open()
> tty_port_close()
> tty_port_hangup()
>
> do almost all of the rest of the work for you. They call back to your
> activate and shutdown port methods, they serialize them, they call them
> on first open/last close in matching pairs.
>
> For the tty itself
>
> tty_port_tty_get()
>
> gets you a reference to the tty from the port (or NULL) - so handles a
> close/hangup racing with data arrival
>
> tty_kref_put()
>
> releases a reference
>
> and
> tty->ops->cleanup()
>
> is called on the final destruction of the tty object (ie its where you
> can free tty lifetime data in tty->private_data)
>
> So for a simple non pluggable tty it tends to look like
>
> int my_tty_open(struct tty_struct *tty, struct file *filp)
> {
> tty->driver_data = &my_port;
> return tty_port_open(&my_port, tty, filp);
> }
>
> void my_tty_close(struct tty_struct *tty, struct file *filp)
> {
> struct my_tty *m = tty->driver_data;
> if (m == NULL)
> return;
> tty_port_close(&m->port, tty, filp);
> }
>
> void my_tty_hangup(struct tty_struct *tty)
> {
> struct my_tty *m = tty->driver_data;
> tty_port_hangup(&m->port);
> }
>
> provide the needed callbacks and it'll do the locking and the like for
> you.
>
> On the ioctl side as far as I can see you should simply get rid of the
> method entirely.
>
> For buffer_data you might want to allocate the buffer sanely at open time
> (tty_port has a function for this too) so it can't fail weirdly
>
> And your termios method is a bit odd but makes sense if you are just
> pretending anything works and is supported.
Thanks for your help!
tty_port makes sense and I think I understood it.
But after modifying the driver to use tty_port the kernel crashes in
tty_io.c:__tty_fasync() because filp->f_path.dentry is NULL.
Any idea which kind of error can cause this?
--
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
