Messages in this thread | | | Date | Tue, 02 Mar 2010 22:43:20 +0900 | From | Hitoshi Mitake <> | Subject | Re: Question about policy of calling lockdep functions in trylocks |
| |
On 03/02/10 18:13, Peter Zijlstra wrote: > On Tue, 2010-03-02 at 17:44 +0900, Hitoshi Mitake wrote: >> 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; >> } > > Did you mean to call acquire twice?
Sorry, this is my mistake. I've forgot to remove spin_acquire() after do_raw_spin_trylock().
> >> >> 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. > > Well, trylocks cannot contend, so the lock_contended() call doesn't make > sense, I don't think it will confuse lockstat, since acquire will simply > reset the state again, but it will waste cycles, nor is there a reason > to call acquired without first having call contended. So no. > > What exactly is the problem, the lack of callbacks for a failed trylock? > Why would you want one? > > Because other than that I see no problem, you get an acquire(.try=1) and > a release() to match and lockstat measure and accounts the hold-time.
Ah, I've forgot about try and read parameters of lock_acquire(). These parameters are enough things for me. perf lock can separate event sequences of normal locks and trylocks with these.
Thanks! Hitoshi
| |