lkml.org 
[lkml]   [2007]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [uml-devel] [PATCH 1/6] UML - Console locking fixes
Date
On Saturday 30 December 2006 00:41, Jeff Dike wrote:
> Clean up the console driver locking. There are various problems here,
> including sleeping under a spinlock and spinlock recursion, some of
> which are fixed here. This patch deals with the locking involved with
> opens and closes. The problem is that an mconsole request to change a
> console's configuration can race with an open. Changing a
> configuration should only be done when a console isn't opened. Also,
> an open must be looking at a stable configuration. In addition, a get
> configuration request must observe the same locking since it must also
> see a stable configuration. With the old locking, it was possible for
> this to hang indefinitely in some cases because open would block for a
> long time waiting for a connection from the host while holding the
> lock needed by the mconsole request.
>
> As explained in the long comment, this is fixed by adding a spinlock
> for the use count and configuration and a mutex for the actual open
> and close.
>
> Signed-off-by: Jeff Dike <jdike@addtoit.com>

> +
> int line_open(struct line *lines, struct tty_struct *tty)
> {
> - struct line *line;
> + struct line *line = &lines[tty->index];
> int err = -ENODEV;
>
> - line = &lines[tty->index];
> - tty->driver_data = line;
> + spin_lock(&line->count_lock);
> + if(!line->valid)
> + goto out_unlock;
> +
> + err = 0;
> + if(tty->count > 1)
> + goto out_unlock;
>
> - /* The IRQ which takes this lock is not yet enabled and won't be run
> - * before the end, so we don't need to use spin_lock_irq.*/
> - spin_lock(&line->lock);
> + mutex_lock(&line->open_mutex);
> + spin_unlock(&line->count_lock);

This is an obnoxious thing to do unless you specifically prove otherwise. You
cannot take a mutex (and possibly sleep) while holding a spinlock.

You must have either:
+ spin_unlock(&line->count_lock);
+ mutex_lock(&line->open_mutex);

or take count_lock inside open_mutex (which looks like being correct here).

In the first solution, you can create a OPENING flag (via a state variable),
and add the rule that (unlike the count) nobody but the original setter is
allowed to change it, and that who finds it set (say a concurrent open) must
return without touching it.

The state diagram is like:
CLOSED -> OPENING -> OPEN
(only the function which triggered the transition from CLOSED to OPENING can
trigger the transition from OPENING to OPEN). It can probably be simplified
to OPENING <-> ! OPENING.
--
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade

Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com

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

\
 
 \ /
  Last update: 2007-01-03 18:29    [W:0.053 / U:1.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site