All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean-Christian de Rivaz <jc@eclis.ch>
To: Thomas Osterried <thomas@osterried.de>, David Ranch <dranch@trinnet.net>
Cc: Ralf Baechle DL5RB <ralf@linux-mips.org>,
	linux-hams@trinnet.net, linux-hams@vger.kernel.org
Subject: Re: AX25 mkiss interface not deleted when the serial port is removed
Date: Thu, 01 Oct 2015 01:04:25 +0200	[thread overview]
Message-ID: <560C6A79.4070400@eclis.ch> (raw)
In-Reply-To: <2A123A0E-5F98-44BE-AC11-23B96B7F8925@osterried.de>

Le 30. 09. 15 22:22, Thomas Osterried a écrit :
> Hello,
>
>> Unfortunately the kernel 4.3.0-rc3+ crash the same way. I suspect that the real question is: what in ax25/mkiss make tty_ldisc_hangup() calling tty_ldisc_open() on a serial port that no longer exists, and how to avoid this ?

First, many many thanks for your time investigating this bug.

> that’s a good question ;)
>
> We had a long debug sessions today.
>
> tty_ldisc_hangup() always calls tty_ldisc_reinit(tty, N_TTY)) and tty_ldisc_open(tty, tty->ldisc)).

Conceptually, is there any good reason to call tty_ldisc_reinit() on a 
deleted device ? I would like to propose a check in tty_ldisc_hangup() 
to test if the underlying device still exists. If not the tty can be 
deleted. That said, I don't know enough this kernel part to assert that 
it's a good proposal or not.

In case the call tty_ldisc_reinit() is really needed, it seem to be 
acceptable to use the N_TTY line discipline instead of N_AX25 one. My 
understanding is that this will prevent calling mkiss_open() right after 
the mkiss_close().

> mkiss_close() is supposed to be called, because the old interface disappeared (we have the same number of ax25-interfaces bevor and after the plug).

I remember having traced this, and yes mkiss_close() is called before. I 
don't think there is any issue regarding this.

> After the plug, mkiss_open() is called again, because the ldisc defines:
> static struct tty_ldisc_ops ax_ldisc = { …         .open           = mkiss_open, …}

This is certainly the strangest part. How is that possible since the 
tty_ldisc_reinit() is explicitly called with the N_TTY line discipline 
in argument ? I would expect that tty_ldisc_reinit() will call this instead:

struct tty_ldisc_ops tty_ldisc_N_TTY = { ...    .open            = 
n_tty_open, ...}

I was unable to catch a problem in tty_ldisc_reinit(tty, N_TTY) that 
correctly set tty->ldisc so that the tty_ldisc_open(tty, tty->ldisc) 
must call n_tty_open(). I now suspect that in tty_ldisc_hangup() the 
reset variable is zero so that tty_ldisc_reinit(tty, 
tty->termios.c_line) and tty_ldisc_open(tty, tty->ldisc) are called. At 
this point I think that the bug is that some code is missing to set 
tty->driver->flags |= TTY_DRIVER_RESET_TERMIOS before tty_ldisc_hangup() 
is called.

I fully agree with all the rest of your message, but I believe it 
describes the logical consequences of an earlier problem.

Regards,
Jean-Christian

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-09-30 23:04 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-25 17:21 AX25 mkiss interface not deleted when the serial port is removed Jean-Christian de Rivaz
2015-09-28 20:06 ` Jean-Christian de Rivaz
2015-09-28 23:13   ` Jean-Christian de Rivaz
2015-09-29  2:13     ` David Ranch
2015-09-29 15:31 ` Ralf Baechle DL5RB
2015-09-29 15:46   ` Jean-Christian de Rivaz
2015-09-29 16:49   ` David Ranch
2015-09-29 17:14     ` Ralf Baechle
2015-09-30  0:29   ` Jean-Christian de Rivaz
2015-09-30 20:22     ` Thomas Osterried
2015-09-30 23:04       ` Jean-Christian de Rivaz [this message]
2015-10-01  2:56         ` [PATCH 1/1] Force mkiss to reset the line discipline when serial device " jc
2015-10-01  7:31           ` Ralf Baechle
2015-10-01  9:18             ` [PATCH v2 " Jean-Christian de Rivaz
2015-10-01 16:56               ` Jean-Christian de Rivaz
2015-10-01 22:57                 ` Peter Hurley
2015-10-02  8:30                   ` Jean-Christian de Rivaz
2015-10-02  8:30                     ` Jean-Christian de Rivaz
2015-10-02 10:35                     ` Thomas Osterried
2015-10-02 10:35                       ` Thomas Osterried
2015-10-02 13:48                       ` Jean-Christian de Rivaz
2015-10-02 17:25                         ` Jean-Christian de Rivaz
2015-10-02 17:25                           ` Jean-Christian de Rivaz
2015-10-02 21:46                           ` [PATCH 1/1] Add poll method to mkiss let notify hangup to the user process Jean-Christian de Rivaz
2015-10-03  0:29                             ` David Ranch
2015-10-03  1:02                               ` Jean-Christian de Rivaz
2016-02-11 19:14                               ` Thomas Osterried
2015-10-02 21:40                         ` [PATCH 1/1] Close the file descriptor and exit when the kernel notice hangup Jean-Christian de Rivaz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560C6A79.4070400@eclis.ch \
    --to=jc@eclis.ch \
    --cc=dranch@trinnet.net \
    --cc=linux-hams@trinnet.net \
    --cc=linux-hams@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    --cc=thomas@osterried.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.