Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1353296 > unrolled thread

[PATCH 2/3] rtmutex: Add rt_mutex_init_waiter helper

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] rtmutex: Add rt_mutex_init_waiter helper Davidlohr Bueso <dave@stgolabs.net> - 2016-03-08 19:30 +0100
    Re: [PATCH 2/3] rtmutex: Add rt_mutex_init_waiter helper Peter Zijlstra <peterz@infradead.org> - 2016-03-14 14:20 +0100

#1353296 — [PATCH 2/3] rtmutex: Add rt_mutex_init_waiter helper

From: Davidlohr Bueso <dbueso@suse.de>

... encapsulates debug and regular waiter initialization. In the
case of rtmutexes, we now also set the waiter to nil until later
explicitly set to whatever task. This is safe as the waiter is on
the stack and we are doing very basic initialization anyway.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/futex.c                  |  5 +----
 kernel/locking/rtmutex.c        |  4 +---
 kernel/locking/rtmutex_common.h | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index bae542e..0ff94a7 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -2778,10 +2778,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
 	 * The waiter is allocated on our stack, manipulated by the requeue
 	 * code while we sleep on uaddr.
 	 */
-	debug_rt_mutex_init_waiter(&rt_waiter);
-	RB_CLEAR_NODE(&rt_waiter.pi_tree_entry);
-	RB_CLEAR_NODE(&rt_waiter.tree_entry);
-	rt_waiter.task = NULL;
+	rt_mutex_init_waiter(&rt_waiter);
 
 	ret = get_futex_key(uaddr2, flags & FLAGS_SHARED, &key2, VERIFY_WRITE);
 	if (unlikely(ret != 0))
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 3e74660..60fa79a 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -1171,9 +1171,7 @@ rt_mutex_slowlock(struct rt_mutex *lock, int state,
 	unsigned long flags;
 	int ret = 0;
 
-	debug_rt_mutex_init_waiter(&waiter);
-	RB_CLEAR_NODE(&waiter.pi_tree_entry);
-	RB_CLEAR_NODE(&waiter.tree_entry);
+	rt_mutex_init_waiter(&waiter);
 
 	/*
 	 * Technically we could use raw_spin_[un]lock_irq() here, but this can
diff --git a/kernel/locking/rtmutex_common.h b/kernel/locking/rtmutex_common.h
index 4f5f83c..c714737 100644
--- a/kernel/locking/rtmutex_common.h
+++ b/kernel/locking/rtmutex_common.h
@@ -119,4 +119,19 @@ extern void rt_mutex_adjust_prio(struct task_struct *task);
 # include "rtmutex.h"
 #endif
 
+/*
+ * Waiter structure basic initialization. A waiter is not considered
+ * actually usable until it after calling task_blocks_on_rt_mutex()
+ * which setups up the relevant entries.
+ */
+static inline void
+rt_mutex_init_waiter(struct rt_mutex_waiter *waiter)
+{
+       debug_rt_mutex_init_waiter(waiter);
+
+       RB_CLEAR_NODE(&waiter->pi_tree_entry);
+       RB_CLEAR_NODE(&waiter->tree_entry);
+       waiter->task = NULL;
+}
+
 #endif
-- 
2.1.4

[toc] | [next] | [standalone]

#1357231

On Tue, Mar 08, 2016 at 10:20:22AM -0800, Davidlohr Bueso wrote:

> +++ b/kernel/futex.c
> @@ -2778,10 +2778,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
>  	 * The waiter is allocated on our stack, manipulated by the requeue
>  	 * code while we sleep on uaddr.
>  	 */
> -	debug_rt_mutex_init_waiter(&rt_waiter);
> -	RB_CLEAR_NODE(&rt_waiter.pi_tree_entry);
> -	RB_CLEAR_NODE(&rt_waiter.tree_entry);
> -	rt_waiter.task = NULL;
> +	rt_mutex_init_waiter(&rt_waiter);
>  
>  	ret = get_futex_key(uaddr2, flags & FLAGS_SHARED, &key2, VERIFY_WRITE);
>  	if (unlikely(ret != 0))


> +static inline void
> +rt_mutex_init_waiter(struct rt_mutex_waiter *waiter)
> +{
> +       debug_rt_mutex_init_waiter(waiter);
> +
> +       RB_CLEAR_NODE(&waiter->pi_tree_entry);
> +       RB_CLEAR_NODE(&waiter->tree_entry);
> +       waiter->task = NULL;
> +}


Just thinking, would not something like:

#define DEFINE_RT_WAITER(name)					\
  struct rt_mutex_waiter name = {				\
	.tree_entry = __INIT_RB_NODE(name.tree_entry),		\
	.pi_tree_entry = __INIT_RB_NODE(name.pi_tree_entry),	\
	__INIT_RT_WAITER_DEBUG(name)				\
  }

Be nicer? That way we're sure the whole structure is initialized.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web