All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Linux Netdev List <netdev@vger.kernel.org>,
	linux kernel <linux-kernel@vger.kernel.org>
Subject: Re: [RFC,PATCH] mutex: mutex_is_owner() helper
Date: Wed, 04 Nov 2009 18:19:35 +0100	[thread overview]
Message-ID: <4AF1B7A7.6030902@gmail.com> (raw)
In-Reply-To: <20091104154015.GA32567@elte.hu>

Ingo Molnar a écrit :

> To make sure this does not extend mutexes to be used a recursive 
> mutexes, mind naming it more clearly, like debug_mutex_is_owned(), and 
> adding a comment that says that this shouldnt be taken?
> 
> Also, it's somewhat imprecise: on !SMP && !DEBUG_MUTEXES we might return 
> a false '1'. Which happens to work for the rtnl usecase - but might not 
> in other cases.
>

Sure, we can chose another name, but what do you mean by a false '1' ?

1 means mutex is locked and that we could not check ownership.
(best effort, ie same imprecise result than mutex_is_locked())


BTW, I was thinking of a mutex_yield() implementation, but could not
cook it without hard thinking, maybe you already have some nice implementation ?

We have some uses of "mutex_unlock();mutex_lock();" things that are
not working nicely because current thread immediately takes again mutex.


a true mutex_yield() would force current thread to go at the end of wait_list.

int mutex_yield(struct mutex *lock)
{
	unsigned long flags;

	// OK to test list without locking
	if (list_empty(&lock->wait_list))
		return 0;

	spin_lock_mutex(&lock->wait_lock, flags);
	if (!list_empty(&lock->wait_list)) {
		atomic_xchg(&lock->count, 1);// free mutex
		list_add_tail(&waiter.list, &lock->wait_list);//insert me at tail of wait_list
		wake head of wait_list
		__mutex_lock_common_condadd(mutex, TASK_UNINTERRUPTIBLE, DONT_ADD_TAIL, ...);
	} else {
		spin_unlock_mutex(&lock->wait_lock, flags);
	}
	return 1;
}


Or maybe we should try something less complex (slowpath anyway)

int mutex_yield(struct mutex *lock)
{
	int ret = 0;

	if (mutex_needbreak(lock) || should_resched()) {
		mutex_unlock(lock); 
		__cond_resched();
		mutex_lock(lock);
		ret = 1;
	}
	return ret;
}

Thanks

  reply	other threads:[~2009-11-04 17:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-04 15:25 [RFC,PATCH] mutex: mutex_is_owner() helper Eric Dumazet
2009-11-04 15:40 ` Ingo Molnar
2009-11-04 17:19   ` Eric Dumazet [this message]
2009-11-09 18:56     ` Peter Zijlstra
2009-11-09 23:21       ` Eric Dumazet
2009-11-10  9:41         ` Peter Zijlstra

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=4AF1B7A7.6030902@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=netdev@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.