mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Question about policy of calling lockdep functions in trylocks
Date: Tue, 02 Mar 2010 17:44:44 +0900	[thread overview]
Message-ID: <4B8CCFFC.2020704@dcl.info.waseda.ac.jp> (raw)


Hi,

I have a question about policy of callings lockdep functions in trylocks.

Normal locks like __raw_spin_lock are defined like this:

static inline void __raw_spin_lock(raw_spinlock_t *lock)
{
	preempt_disable();
	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
}

And LOCK_CONTENDED is defined:
#define LOCK_CONTENDED(_lock, try, lock)			\
do {								\
	if (!try(_lock)) {					\
		lock_contended(&(_lock)->dep_map, _RET_IP_);	\
		lock(_lock);					\
	}							\
	lock_acquired(&(_lock)->dep_map, _RET_IP_);			\
} while (0)

So, acquiring and releasing lock with no contention calls lockdep
functions like this:

     lock_acquire -> lock_acquired -> lock_release

And acquiring and releasing lock with contention calls lockdep functions
like this:

     lock_acquire -> lock_contended -> lock_acquired -> lock_release

But I found that locks with try like __raw_spin_trylock is defined like
this:

static inline int __raw_spin_trylock(raw_spinlock_t *lock)
{
	preempt_disable();
	if (do_raw_spin_trylock(lock)) {
		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
		return 1;
	}
	preempt_enable();
	return 0;
}

So, trying acquiring and releasing lock with no contention calls lockdep
functions like this:

     lock_acquire -> lock_release

And failed trying acquiring calls no lockdep function.

I felt that policy of calling lockdep functions is strange.
Trylocks should be like this:

static inline int __raw_spin_trylock(raw_spinlock_t *lock)
{
	preempt_disable();
	spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
	if (do_raw_spin_trylock(lock)) {
		spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
		lock_acquired(&lock->dep_map, _RET_IP_);
		return 1;
	}
	lock_contended(&lock->dep_map, _RET_IP_);
	preempt_enable();
	return 0;
}

This is my question.
Are there some reasons current calling lockdep functions of trylocks?
If not, can I change these trylocks like I described above?

The reason why I'm asking about it is perf lock.
For state machine of perf lock, these event sequenses are very confusable.
Because sequence of trylock is subset of normal lock. This is ambiguity.

Thanks,
	Hitoshi

             reply	other threads:[~2010-03-02  8:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-02  8:44 Hitoshi Mitake [this message]
2010-03-02  9:13 ` Peter Zijlstra
2010-03-02 13:43   ` Hitoshi Mitake
2010-03-10  8:36     ` Hitoshi Mitake

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=4B8CCFFC.2020704@dcl.info.waseda.ac.jp \
    --to=mitake@dcl.info.waseda.ac.jp \
    --cc=a.p.zijlstra@chello.nl \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®