From: Arnd Bergmann <arnd@arndb.de>
To: Paul Fulghum <paulkf@microgate.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] n_hdlc fix read and write locking
Date: Mon, 25 Oct 2010 22:31:06 +0200 [thread overview]
Message-ID: <201010252231.06794.arnd@arndb.de> (raw)
In-Reply-To: <1288030959.19909.28.camel@x2.microgate.com>
On Monday 25 October 2010 20:22:39 Paul Fulghum wrote:
> Fix locking in read and write code of n_hdlc line discipline.
>
> 2.6.36 replaced lock_kernel() with tty_lock().
> The tty mutex is not dropped automatically when the thread
> sleeps like the BKL. This results in a blocked read or write holding
> the tty mutex and stalling operations by other devices that use
> the tty mutex.
>
> A review of n_hdlc read and write code shows:
> 1. neither BKL or tty mutex are required for correct operation
> 2. read can block while read data is available if data is posted
> between availability check and call to interruptible_sleep_on()
> 3. write does not set process state to TASK_INTERRUPTIBLE
> on each pass through the processing loop which can cause
> unneeded scheduling of the thread
Right. I must have missed this when I was not checking for
interruptible_sleep_on(). I did systematically check for
this problem with the wait_event family as well as
work_queues, mutexes, semaphores and hand-written schedule
loops, but for some reason I did not check for sleep_on :(
I've double-checked it now, and it seems that all other
instances of sleep_on are waiting for close_wait in
block_til_ready or open functions, and I remember that
I did check those and convinced myself that they are fine.
> Write corrected to set process state to
> TASK_INTERRUPTIBLE on each pass through processing loop.
Would it be possible to express the same using
wait_event_interruptible()?
Arnd
next prev parent reply other threads:[~2010-10-25 20:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-25 18:22 [PATCH] n_hdlc fix read and write locking Paul Fulghum
2010-10-25 20:05 ` Andrew Morton
2010-10-25 20:29 ` Paul Fulghum
2010-10-25 21:19 ` Paul Fulghum
2010-10-25 20:31 ` Arnd Bergmann [this message]
2010-10-25 23:18 ` Paul Fulghum
2010-10-26 10:40 ` Arnd Bergmann
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=201010252231.06794.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=akpm@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=paulkf@microgate.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 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.