linux-kernel - Question about policy of calling lockdep functions in trylocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B8CCFFC.2020704@dcl.info.waseda.ac.jp>
Date:	Tue, 02 Mar 2010 17:44:44 +0900
From:	Hitoshi Mitake <mitake@....info.waseda.ac.jp>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
CC:	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Question about policy of calling lockdep functions in trylocks

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