Re: tty's use of file_list_lock and file_move

From: Jon Smirl
Date: Tue Jul 11 2006 - 18:07:19 EST


On 7/11/06, Russell King <rmk+lkml@xxxxxxxxxxxxxxxx> wrote:
On Mon, Jul 10, 2006 at 09:29:04PM -0400, Theodore Tso wrote:
> On Mon, Jul 10, 2006 at 07:49:31PM -0400, Jon Smirl wrote:
> > How about the use of lock/unlock_kernel(). Is there some hidden global
> > synchronization going on? Every time lock/unlock_kernel() is used
> > there is a tty_struct available. My first thought would be to turn
> > this into a per tty spinlock. Looking at where it is used it looks
> > like it was added to protect all of the VFS calls. I see no obvious
> > coordination with other ttys that isn't handled by other locks.
>
> No, it was just a case of not being worth it to get rid of the BKL for
> the tty subsystem, since opening and closing tty's isn't exactly a
> common event. Switching it to use a per-tty spinlock makes sense if
> we're going to rototill the code, but to be honest it's probably not
> going to make a noticeable difference on any benchmark and most
> workloads.

It's not that simple - remember that you must be able to open a tty
in non-blocking mode while someone else is opening the same tty in
blocking mode, _and_ succeed. (iow, the getty waiting for call-in
and you want to dial out case.)

If we go for merely replacing BKL with some other lock, each tty
driver has to be able to drop that lock when it decides to sleep due
to no carrier in its open method... which is kind'a yuck.

Directly replacing BKL was my first strategy but that looks to be
hard. Some of the sleeps are in the line discipline code which means I
have to find all of them. After replacing BKL I'd try to simplify the
locking.

What about adjusting things so the BKL isn't required? I tried
completely removing it and died in release_dev. tty_mutex is already
locks a lot of stuff, maybe it can be adjusted to allow removal of the
BKL.

I'm also trying to decide if read/write really need BKL when they are
called. Read/write already uses the atomic_read/write_lock mutexes.

read_chan has this comment:
* Perform reads for the line discipline. We are guaranteed that the
* line discipline will not be closed under us but we may get multiple
* parallel readers and must handle this ourselves. We may also get
* a hangup. Always called in user context, may sleep.

But read_chan is always called from tty_io.c with BKL held. So is
atomic_read_lock really doing anything or is it masked by BKL?

I see why no one works on this code, it is very intertwined with the
rest of the kernel and a lot of the reasons for locking are
non-obvious.

--
Jon Smirl
jonsmirl@xxxxxxxxx
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/