All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <matthias.kaehlcke@gmail.com>
To: Arjan van de Ven <arjan@infradead.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: use mutex instead of semaphore in RocketPort driver
Date: Tue, 22 May 2007 22:06:02 +0200	[thread overview]
Message-ID: <20070522200601.GG22360@traven> (raw)
In-Reply-To: <1179853142.3296.1.camel@laptopd505.fenrus.org>

El Tue, May 22, 2007 at 09:59:01AM -0700 Arjan van de Ven ha dit:

> >  
> > -	down_interruptible(&info->write_sem);
> > +	mutex_lock_interruptible(&info->write_mtx);
> >  
> >  #ifdef ROCKET_DEBUG_WRITE
> >  	printk(KERN_INFO "rp_write %d chars...", count);
> > @@ -1773,7 +1776,7 @@ end:
> >  		wake_up_interruptible(&tty->poll_wait);
> >  #endif
> >  	}
> > -	up(&info->write_sem);
> > +	mutex_unlock(&info->write_mtx);
> >  	return retval
> 
> this code is very very buggy.

more buggy than with the use of a semaphore?
 
> mutex_lock_interruptible() may not get the mutex in case a signal
> happens... and yet you unlox the mutex unconditionally!!!

as far as i understand only the thread that locked the mutex can
unlock it (as opposed to semaphores, which can be released by any
thread/process). obviously this doesn't make the code be more
correct. what i don't know is how the kernel behaves when
trying to unlock a mutex the thread doesn't own. another and possibly
more important problem of the code is that in case of being
interrupted by a signal the data that should be protected by the
mutex/semaphore can be accessed/changed by two threads at the same
time.

would the following resolve the problem?

if(mutex_lock_interruptible(&info->write_mtx)) 
        return -ERESTARTSYS

thanks for your comments

-- 
Matthias Kaehlcke
Linux Application Developer
Barcelona

            Nothing is more despicable than respect based on fear
                              (Albert Camus)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-

  reply	other threads:[~2007-05-22 20:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200705081903.l48J3lr1012622@hera.kernel.org>
2007-05-22 16:59 ` use mutex instead of semaphore in RocketPort driver Arjan van de Ven
2007-05-22 20:06   ` Matthias Kaehlcke [this message]
2007-05-22 21:04     ` Simon Arlott
2007-05-22 21:23       ` Jiri Slaby

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=20070522200601.GG22360@traven \
    --to=matthias.kaehlcke@gmail.com \
    --cc=arjan@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.