All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Dike <jdike@addtoit.com>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: akpm@osdl.org, linux-kernel@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net
Subject: Re: [uml-devel] [PATCH 1/6] UML - Console locking fixes
Date: Wed, 3 Jan 2007 14:22:59 -0500	[thread overview]
Message-ID: <20070103192259.GA5348@ccure.user-mode-linux.org> (raw)
In-Reply-To: <200701031607.34683.blaisorblade@yahoo.it>

On Wed, Jan 03, 2007 at 04:07:34PM +0100, Blaisorblade wrote:
> > +	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. 

Didn't I?  
The proof goes like this:
	we only take the semaphore if tty->count == 1, in which case
we are opening the device for the first time and there can't be anyone
else looking at it, so the mutex_lock won't sleep.

However, now that you're making me think about it again, I'm wondering
about the sanity of introducing a mutex which is guaranteed not to
sleep.

This is starting to make sense, with (tty->count > 1) being the
OPENING flag:

> 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.

Then, I think the mutex can just be thrown away.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Dike <jdike@addtoit.com>
To: Blaisorblade <blaisorblade@yahoo.it>
Cc: user-mode-linux-devel@lists.sourceforge.net, akpm@osdl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [uml-devel] [PATCH 1/6] UML - Console locking fixes
Date: Wed, 3 Jan 2007 14:22:59 -0500	[thread overview]
Message-ID: <20070103192259.GA5348@ccure.user-mode-linux.org> (raw)
In-Reply-To: <200701031607.34683.blaisorblade@yahoo.it>

On Wed, Jan 03, 2007 at 04:07:34PM +0100, Blaisorblade wrote:
> > +	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. 

Didn't I?  
The proof goes like this:
	we only take the semaphore if tty->count == 1, in which case
we are opening the device for the first time and there can't be anyone
else looking at it, so the mutex_lock won't sleep.

However, now that you're making me think about it again, I'm wondering
about the sanity of introducing a mutex which is guaranteed not to
sleep.

This is starting to make sense, with (tty->count > 1) being the
OPENING flag:

> 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.

Then, I think the mutex can just be thrown away.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

  reply	other threads:[~2007-01-05  2:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-29 23:41 [uml-devel] [PATCH 1/6] UML - Console locking fixes Jeff Dike
2006-12-29 23:41 ` Jeff Dike
2006-12-29 23:48 ` [uml-devel] " Randy Dunlap
2006-12-29 23:48   ` Randy Dunlap
2007-01-01 20:03   ` [uml-devel] " Jeff Dike
2007-01-01 20:03     ` Jeff Dike
2007-01-03 15:07 ` [uml-devel] " Blaisorblade
2007-01-03 15:07   ` Blaisorblade
2007-01-03 19:22   ` Jeff Dike [this message]
2007-01-03 19:22     ` Jeff Dike

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=20070103192259.GA5348@ccure.user-mode-linux.org \
    --to=jdike@addtoit.com \
    --cc=akpm@osdl.org \
    --cc=blaisorblade@yahoo.it \
    --cc=linux-kernel@vger.kernel.org \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /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.