linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: George Spelvin <linux@horizon.com>
Cc: jslaby@suse.cz, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org
Subject: Re: 3.8-rc regression with pps-ldisc due to 70ece7a731
Date: Wed, 06 Feb 2013 15:31:09 -0500	[thread overview]
Message-ID: <1360182669.5226.36.camel@thor.lan> (raw)
In-Reply-To: <20130206194514.16104.qmail@science.horizon.com>

On Wed, 2013-02-06 at 14:45 -0500, George Spelvin wrote:
> > Tight coupling is what caused this to break in the first place -- I
> > don't think tighter coupling is the right answer.
> 
> Agreed.  But given that n_tty already knows there are wrappers, it would
> have been possible to find a cleaner way to access an "aux pointer" in
> the tty structure, if that's what was desired.

I know. Like n_tty_get/set_data() with a void* in struct n_tty_data. But
I'm sure you'd agree that's just an expedient. If there were multiple
uses for this requirement, it'd be better to have this supported by the
base interface for all ldiscs.

> > You are not supposed to receive ldisc->dcd_change() calls outside
> > the open()/close() pair.
> 
> Yes, I figured that out.  I was wondering because I couldn't see any
> way that the serial interrupt hander was blocked or masked, but
> then I figured out that it's not, but instead the TTY_LDISC flag
> is used.
> 
> At the time the open() method is called, the flag is cleared, which makes
> tty_ldisc_ref() return NULL, which prevents calling the ldisc methods.

Exactly.

> > In the patch series I sent, I changed the BUG_ON() to WARN_ON_ONCE().
> > Please reply to that patch with the snipped kernel log output if it
> > warns in your testing and we'll go from there.
> 
> I really doubt it will.  The entire code change and comment was
> just caution on my part.

There were race conditions in the existing ldisc layer which allowed
references past halts. Just 2 days ago, I put up a 23-part
patch series to address it. It's very complicated and it's possible I
missed something.

Regards,
Peter Hurley



  reply	other threads:[~2013-02-06 20:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-04  1:03 3.8-rc regression with pps-ldisc due to 70ece7a731 George Spelvin
2013-02-04  4:18 ` George Spelvin
2013-02-04  7:08   ` George Spelvin
2013-02-06 16:15     ` Peter Hurley
2013-02-06 15:53 ` Peter Hurley
2013-02-06 19:45   ` George Spelvin
2013-02-06 20:31     ` Peter Hurley [this message]
2013-02-06 15:55 ` [PATCH 0/4] tty, pps: decouple pps Peter Hurley
2013-02-06 15:55   ` [PATCH 1/4] pps: Decouple N_PPS from N_TTY Peter Hurley
2013-02-06 15:55   ` [PATCH 2/4] pps: Don't crash the machine when exiting will do Peter Hurley
2013-02-06 15:55   ` [PATCH 3/4] pps: Use lookup list to reduce ldisc coupling Peter Hurley
2013-02-06 16:20     ` Jiri Slaby
2013-02-06 16:41       ` Peter Hurley
2013-02-06 19:34     ` George Spelvin
2013-02-06 20:09       ` Peter Hurley
2013-02-06 22:19         ` George Spelvin
2013-02-06 23:15           ` Peter Hurley
2013-02-06 15:55   ` [PATCH 4/4] tty: Remove ancient hardpps() Peter Hurley

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=1360182669.5226.36.camel@thor.lan \
    --to=peter@hurleysoftware.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@horizon.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).