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.
Alan
--
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
