public inbox for linux-serial@vger.kernel.org
 help / color / mirror / Atom feed
From: Ian Jackson <Ian.Jackson@eu.citrix.com>
To: Jiri Kosina <jkosina@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Markus Armbruster <armbru@redhat.com>,
	Michal Hocko <mhocko@suse.cz>
Subject: Re: [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG_TXEN
Date: Thu, 19 Nov 2009 18:15:49 +0000	[thread overview]
Message-ID: <19205.35669.210191.691969@mariner.uk.xensource.com> (raw)
In-Reply-To: <alpine.LSU.2.00.0911191734570.15039@wotan.suse.de>

Jiri Kosina writes ("Re: [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG_TXEN"):
> Does anyone have any comments about this please?
> 
> In a nutshell -- this is needed so that we make the UART_BUG_TXEN really 
> harmless (which I guess it originally inteded to be, but reading IIR has 
> some unwanted sideeffects and is in fact not needed).

Yes.

> We need to have this in to handle properly the cases in which BUG_TXEN is 
> misdetected, and we can't blacklist such systems as we do for some SoL 
> hardware (see commit b6adea334c6c (" 8250: fix boot hang with serial 
> console when using with Serial Over Lan port"). Also there doesn't seem to 
> be any straightforward way to workaround the misdetection, so this seems 
> to be proper fix, unbreaking all the possible scenarios.

Unless the code has changed, the UART_BUG_TXEN detection is
fundamentally broken and racy.  So fixing that too would be good (and
possible, when I looked at it).

But even without fixing the UART_BUG_TXEN detection, the IIR read is
absolutely and definitely wrong.  The only purpose of the IIR read is
to avoid an unnecessary but harmless call to transmit_chars.

It seems to me that the author of the UART_BUG_TXEN patch did not
appreciate that reading the IIR was not side effect free, and that
they were just trying a minor (and perhaps ill-advised) optimisation.

In any case we don't have any useful information (in comments or
commit logs) explaining why they did it this way and it is (a) easily
seen to be buggy and (b) causes actual real problems.

So my patch should be applied.  The last time we discussed this we had
some kind of FUD along the lines of "but real serial ports often have
bugs so you can't reason about them properly".  I don't think that's a
sensible basis for discussion.  How can it be falsified ?

If there is some specific bug that real ports have that might be
relevant, sure, take it into account.  But we should not have the
situation where we hear the mere assertion that there often are bugs;
therefore reasoning doesn't show change to be safe; therefore no
change can be made.

Ian.

      parent reply	other threads:[~2009-11-19 18:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-18 10:08 [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG_TXEN Jiri Kosina
2009-11-19 16:38 ` Jiri Kosina
2009-11-19 16:42   ` Greg KH
2009-11-19 18:15   ` Ian Jackson [this message]

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=19205.35669.210191.691969@mariner.uk.xensource.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=armbru@redhat.com \
    --cc=gregkh@suse.de \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mhocko@suse.cz \
    /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