[v3,03/12] locking/ww_mutex: Extract stamp comparison to __ww_mutex_stamp_after

Commit Message

Nicolai Hähnle Dec. 21, 2016, 6:46 p.m. UTC

From: Nicolai Hähnle <[e-mail]>

The function will be re-used in subsequent patches.

v3: rename to __ww_ctx_stamp_after (Chris Wilson)

Cc: Peter Zijlstra <[e-mail]>
Cc: Ingo Molnar <[e-mail]>
Cc: Maarten Lankhorst <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Cc: [e-mail]
Signed-off-by: Nicolai Hähnle <[e-mail]>
Reviewed-by: Chris Wilson <[e-mail]>
---
 kernel/locking/mutex.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Chunming Zhou Dec. 22, 2016, 1:58 a.m. UTC | #1

On 2016年12月22日 02:46, Nicolai Hähnle wrote:
> +static inline bool __sched
> +__ww_ctx_stamp_after(struct ww_acquire_ctx *a, struct ww_acquire_ctx *b)
> +{
> +	return a->stamp - b->stamp <= LONG_MAX &&
> +	       (a->stamp != b->stamp || a > b);
I want to ask a stupid question, why a can compare with b? They are 
pointers of structure. Isn't stamp enough for compare?

Thanks,
David Zhou

Nicolai Hähnle Dec. 22, 2016, 8:43 a.m. UTC | #2

On 22.12.2016 02:58, zhoucm1 wrote:
> On 2016年12月22日 02:46, Nicolai Hähnle wrote:
>> +static inline bool __sched
>> +__ww_ctx_stamp_after(struct ww_acquire_ctx *a, struct ww_acquire_ctx *b)
>> +{
>> +    return a->stamp - b->stamp <= LONG_MAX &&
>> +           (a->stamp != b->stamp || a > b);
> I want to ask a stupid question, why a can compare with b? They are
> pointers of structure. Isn't stamp enough for compare?

As far as I understand, the idea is to provide a tie-breaker to ensure 
that there is a strict order between contexts even if the stamp happens 
to be equal. Since we get stamps from atomic increments, this really 
only matters if (a) someone makes a mistake and confuses ww_classes 
(which CONFIG_DEBUG_MUTEXES would flag) or (b) the ww_class stamp 
counter wraps around fully during the lifetime of the acquire context. 
This is extremely unlikely of course.

Nicolai

Patch

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index c02c566..66718d6 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -277,6 +277,13 @@  static __always_inline void ww_mutex_lock_acquired(struct ww_mutex *ww,
 	ww_ctx->acquired++;
 }
 
+static inline bool __sched
+__ww_ctx_stamp_after(struct ww_acquire_ctx *a, struct ww_acquire_ctx *b)
+{
+	return a->stamp - b->stamp <= LONG_MAX &&
+	       (a->stamp != b->stamp || a > b);
+}
+
 /*
  * After acquiring lock with fastpath or when we lost out in contested
  * slowpath, set ctx and wake up any waiters so they can recheck.
@@ -602,8 +609,7 @@  __ww_mutex_lock_check_stamp(struct mutex *lock, struct ww_acquire_ctx *ctx)
 	if (!hold_ctx)
 		return 0;
 
-	if (ctx->stamp - hold_ctx->stamp <= LONG_MAX &&
-	    (ctx->stamp != hold_ctx->stamp || ctx > hold_ctx)) {
+	if (__ww_ctx_stamp_after(ctx, hold_ctx)) {
 #ifdef CONFIG_DEBUG_MUTEXES
 		DEBUG_LOCKS_WARN_ON(ctx->contending_lock);
 		ctx->contending_lock = ww;