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

Groups > linux.kernel > #1169896 > unrolled thread

[RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem

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

  [RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem Peter Zijlstra <peterz@infradead.org> - 2015-06-22 14:30 +0200
    Re: [RFC][PATCH 09/13] hotplug: Replace hotplug lock with  percpu-rwsem Oleg Nesterov <oleg@redhat.com> - 2015-06-23 01:00 +0200
      [PATCH 0/3] percpu-rwsem: introduce percpu_rw_semaphore->recursive  mode Oleg Nesterov <oleg@redhat.com> - 2015-06-29 02:00 +0200
        [PATCH 2/3] percpu-rwsem: don't use percpu_rw_semaphore->rw_sem to  exclude writers Oleg Nesterov <oleg@redhat.com> - 2015-06-29 02:00 +0200
        [PATCH 3/3] percpu-rwsem: introduce percpu_rw_semaphore->recursive  mode Oleg Nesterov <oleg@redhat.com> - 2015-06-29 02:00 +0200
        [PATCH 1/3] rcusync: introduce rcu_sync_struct->exclusive mode Oleg Nesterov <oleg@redhat.com> - 2015-06-29 02:00 +0200

#1169896 — [RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem

The cpu hotplug lock is a rwsem with read-in-write and read-in-read
recursion. Implement it as such.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/cpu.h          |    6 +
 include/linux/percpu-rwsem.h |   10 ++-
 include/linux/sched.h        |    4 +
 init/main.c                  |    1 
 kernel/cpu.c                 |  133 +++++++++++++------------------------------
 kernel/fork.c                |    2 
 lib/Kconfig                  |    5 +
 7 files changed, 66 insertions(+), 95 deletions(-)

--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -224,6 +224,9 @@ extern struct bus_type cpu_subsys;
 #ifdef CONFIG_HOTPLUG_CPU
 /* Stop CPUs going up and down. */
 
+extern void cpu_hotplug_init(void);
+extern void cpu_hotplug_init_task(struct task_struct *p);
+
 extern void cpu_hotplug_begin(void);
 extern void cpu_hotplug_done(void);
 extern void get_online_cpus(void);
@@ -242,6 +245,9 @@ int cpu_down(unsigned int cpu);
 
 #else		/* CONFIG_HOTPLUG_CPU */
 
+static inline void cpu_hotplug_init(void) {}
+static inline void cpu_hotplug_init_task(struct task_struct *p) {}
+
 static inline void cpu_hotplug_begin(void) {}
 static inline void cpu_hotplug_done(void) {}
 #define get_online_cpus()	do { } while (0)
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -20,12 +20,10 @@ extern void __percpu_down_read(struct pe
 extern bool __percpu_down_read_trylock(struct percpu_rw_semaphore *);
 extern void __percpu_up_read(struct percpu_rw_semaphore *);
 
-static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
+static inline void _percpu_down_read(struct percpu_rw_semaphore *sem)
 {
 	might_sleep();
 
-	rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
-
 	preempt_disable();
 	/*
 	 * We are in an RCU-sched read-side critical section, so the writer
@@ -46,6 +44,12 @@ static inline void percpu_down_read(stru
 	 */
 }
 
+static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
+{
+	rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
+	_percpu_down_read(sem);
+}
+
 static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 {
 	bool ret = true;
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1389,6 +1389,10 @@ struct task_struct {
 	unsigned int btrace_seq;
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+	int cpuhp_ref;
+#endif
+
 	unsigned int policy;
 	int nr_cpus_allowed;
 	cpumask_t cpus_allowed;
--- a/init/main.c
+++ b/init/main.c
@@ -588,6 +588,7 @@ asmlinkage __visible void __init start_k
 	sched_clock_postinit();
 	perf_event_init();
 	profile_init();
+	cpu_hotplug_init();
 	call_function_init();
 	WARN(!irqs_disabled(), "Interrupts were enabled early\n");
 	early_boot_irqs_disabled = false;
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -22,6 +22,7 @@
 #include <linux/lockdep.h>
 #include <linux/tick.h>
 #include <trace/events/power.h>
+#include <linux/percpu-rwsem.h>
 
 #include "smpboot.h"
 
@@ -50,7 +51,8 @@ EXPORT_SYMBOL(cpu_notifier_register_done
 
 static RAW_NOTIFIER_HEAD(cpu_chain);
 
-/* If set, cpu_up and cpu_down will return -EBUSY and do nothing.
+/*
+ * If set, cpu_up and cpu_down will return -EBUSY and do nothing.
  * Should always be manipulated under cpu_add_remove_lock
  */
 static int cpu_hotplug_disabled;
@@ -58,126 +60,72 @@ static int cpu_hotplug_disabled;
 #ifdef CONFIG_HOTPLUG_CPU
 
 static struct {
-	struct task_struct *active_writer;
-	/* wait queue to wake up the active_writer */
-	wait_queue_head_t wq;
-	/* verifies that no writer will get active while readers are active */
-	struct mutex lock;
-	/*
-	 * Also blocks the new readers during
-	 * an ongoing cpu hotplug operation.
-	 */
-	atomic_t refcount;
-
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	struct lockdep_map dep_map;
-#endif
-} cpu_hotplug = {
-	.active_writer = NULL,
-	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(cpu_hotplug.wq),
-	.lock = __MUTEX_INITIALIZER(cpu_hotplug.lock),
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	.dep_map = {.name = "cpu_hotplug.lock" },
-#endif
-};
-
-/* Lockdep annotations for get/put_online_cpus() and cpu_hotplug_begin/end() */
-#define cpuhp_lock_acquire_read() lock_map_acquire_read(&cpu_hotplug.dep_map)
-#define cpuhp_lock_acquire_tryread() \
-				  lock_map_acquire_tryread(&cpu_hotplug.dep_map)
-#define cpuhp_lock_acquire()      lock_map_acquire(&cpu_hotplug.dep_map)
-#define cpuhp_lock_release()      lock_map_release(&cpu_hotplug.dep_map)
+	struct percpu_rw_semaphore	rwsem;
+	struct task_struct		*writer;
+} cpu_hotplug = { .writer = &init_task, };
+
+void cpu_hotplug_init(void)
+{
+	percpu_init_rwsem(&cpu_hotplug.rwsem);
+	cpu_hotplug.writer = NULL;
+}
 
+void cpu_hotplug_init_task(struct task_struct *p)
+{
+	p->cpuhp_ref = 0;
+}
 
 void get_online_cpus(void)
 {
 	might_sleep();
-	if (cpu_hotplug.active_writer == current)
+
+	/* read in write recursion */
+	if (cpu_hotplug.writer == current)
+		return;
+
+	/* read in read recursion */
+	if (current->cpuhp_ref++)
 		return;
-	cpuhp_lock_acquire_read();
-	mutex_lock(&cpu_hotplug.lock);
-	atomic_inc(&cpu_hotplug.refcount);
-	mutex_unlock(&cpu_hotplug.lock);
+
+	lock_map_acquire_read(&cpu_hotplug.rwsem.rw_sem.dep_map);
+	_percpu_down_read(&cpu_hotplug.rwsem);
 }
 EXPORT_SYMBOL_GPL(get_online_cpus);
 
 bool try_get_online_cpus(void)
 {
-	if (cpu_hotplug.active_writer == current)
+	if (cpu_hotplug.writer == current)
 		return true;
-	if (!mutex_trylock(&cpu_hotplug.lock))
-		return false;
-	cpuhp_lock_acquire_tryread();
-	atomic_inc(&cpu_hotplug.refcount);
-	mutex_unlock(&cpu_hotplug.lock);
-	return true;
+
+	if (current->cpuhp_ref++)
+		return true;
+
+	return percpu_down_read_trylock(&cpu_hotplug.rwsem);
 }
 EXPORT_SYMBOL_GPL(try_get_online_cpus);
 
 void put_online_cpus(void)
 {
-	int refcount;
-
-	if (cpu_hotplug.active_writer == current)
+	if (cpu_hotplug.writer == current)
 		return;
 
-	refcount = atomic_dec_return(&cpu_hotplug.refcount);
-	if (WARN_ON(refcount < 0)) /* try to fix things up */
-		atomic_inc(&cpu_hotplug.refcount);
-
-	if (refcount <= 0 && waitqueue_active(&cpu_hotplug.wq))
-		wake_up(&cpu_hotplug.wq);
-
-	cpuhp_lock_release();
+	if (--current->cpuhp_ref)
+		return;
 
+	percpu_up_read(&cpu_hotplug.rwsem);
 }
 EXPORT_SYMBOL_GPL(put_online_cpus);
 
-/*
- * This ensures that the hotplug operation can begin only when the
- * refcount goes to zero.
- *
- * Note that during a cpu-hotplug operation, the new readers, if any,
- * will be blocked by the cpu_hotplug.lock
- *
- * Since cpu_hotplug_begin() is always called after invoking
- * cpu_maps_update_begin(), we can be sure that only one writer is active.
- *
- * Note that theoretically, there is a possibility of a livelock:
- * - Refcount goes to zero, last reader wakes up the sleeping
- *   writer.
- * - Last reader unlocks the cpu_hotplug.lock.
- * - A new reader arrives at this moment, bumps up the refcount.
- * - The writer acquires the cpu_hotplug.lock finds the refcount
- *   non zero and goes to sleep again.
- *
- * However, this is very difficult to achieve in practice since
- * get_online_cpus() not an api which is called all that often.
- *
- */
 void cpu_hotplug_begin(void)
 {
-	DEFINE_WAIT(wait);
-
-	cpu_hotplug.active_writer = current;
-	cpuhp_lock_acquire();
-
-	for (;;) {
-		mutex_lock(&cpu_hotplug.lock);
-		prepare_to_wait(&cpu_hotplug.wq, &wait, TASK_UNINTERRUPTIBLE);
-		if (likely(!atomic_read(&cpu_hotplug.refcount)))
-				break;
-		mutex_unlock(&cpu_hotplug.lock);
-		schedule();
-	}
-	finish_wait(&cpu_hotplug.wq, &wait);
+	percpu_down_write(&cpu_hotplug.rwsem);
+	cpu_hotplug.writer = current;
 }
 
 void cpu_hotplug_done(void)
 {
-	cpu_hotplug.active_writer = NULL;
-	mutex_unlock(&cpu_hotplug.lock);
-	cpuhp_lock_release();
+	cpu_hotplug.writer = NULL;
+	percpu_up_write(&cpu_hotplug.rwsem);
 }
 
 /*
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1410,6 +1410,8 @@ static struct task_struct *copy_process(
 	p->sequential_io_avg	= 0;
 #endif
 
+	cpu_hotplug_init_task(p);
+
 	/* Perform scheduler related setup. Assign this task to a CPU. */
 	retval = sched_fork(clone_flags, p);
 	if (retval)
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -56,6 +56,11 @@ config STMP_DEVICE
 config PERCPU_RWSEM
 	bool
 
+config PERCPU_RWSEM_HOTPLUG
+	def_bool y
+	depends on HOTPLUG_CPU
+	select PERCPU_RWSEM
+
 config ARCH_USE_CMPXCHG_LOCKREF
 	bool
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1170269 — Re: [RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem

On 06/22, Peter Zijlstra wrote:
>
> The cpu hotplug lock is a rwsem with read-in-write and read-in-read
> recursion. Implement it as such.

And this patch fixes the problem afaics. Currently cpu_hotplug_begin()
can livelock because it doesn't stop the new readers. With this patch
this is no longer possible.


> -static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
> +static inline void _percpu_down_read(struct percpu_rw_semaphore *sem)
>  {
>  	might_sleep();
>
> -	rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
> -
>  	preempt_disable();
>  	/*
>  	 * We are in an RCU-sched read-side critical section, so the writer
> @@ -46,6 +44,12 @@ static inline void percpu_down_read(stru
>  	 */
>  }
>
> +static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
> +{
> +	rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
> +	_percpu_down_read(sem);
> +}

...

>  void get_online_cpus(void)
>  {
>  	might_sleep();
> -	if (cpu_hotplug.active_writer == current)
> +
> +	/* read in write recursion */
> +	if (cpu_hotplug.writer == current)
> +		return;
> +
> +	/* read in read recursion */
> +	if (current->cpuhp_ref++)
>  		return;
> -	cpuhp_lock_acquire_read();
> -	mutex_lock(&cpu_hotplug.lock);
> -	atomic_inc(&cpu_hotplug.refcount);
> -	mutex_unlock(&cpu_hotplug.lock);
> +
> +	lock_map_acquire_read(&cpu_hotplug.rwsem.rw_sem.dep_map);
> +	_percpu_down_read(&cpu_hotplug.rwsem);
>  }

Confused... Why do we need _percpu_down_read()? Can't get_online_cpus()
just use percpu_down_read() ?

Yes, percpu_down_read() is not recursive, like the normal down_read().
But this does not matter because we rely on ->cpuhp_ref anyway?


> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1410,6 +1410,8 @@ static struct task_struct *copy_process(
>  	p->sequential_io_avg	= 0;
>  #endif
>
> +	cpu_hotplug_init_task(p);

This is probably unnecessary, copy_process() should not be called under
get_online_cpus().

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1173584 — [PATCH 0/3] percpu-rwsem: introduce percpu_rw_semaphore->recursive mode

On 06/24, Oleg Nesterov wrote:
>
> So we need percpu_down_write_dont_block_readers(). I already thought
> about this before, I'll try to make the patch tomorrow on top of your
> changes.

Never say tomorrow...

> This means that we do not need task_struct->cpuhp_ref, but we can't
> avoid livelock we currently have: cpu_hotplug_begin() can never succeed
> if the new readers come fast enough.

Like with any other "recursive" lock.

Peter, I know you don't like the 1st patch. And yes, we could add another
mutex into percpu_rw_semaphore instead. But I think it would be better
to rely on rcu_sync_enter(). As for completion, we can remove it later.
Nevermind, the actual change is 3/3 and it looks simple.

Oleg.

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

[toc] | [prev] | [next] | [standalone]

#1173585 — [PATCH 2/3] percpu-rwsem: don't use percpu_rw_semaphore->rw_sem to exclude writers

percpu_down_write() does down_write() to exclude both the readers and
other writers. We can rely on rcu_sync_enter() in exclusive mode and
take ->rw_sem right before wait_event().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/locking/percpu-rwsem.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index 014d2f4..609c13b 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -139,8 +139,6 @@ static bool readers_active_check(struct percpu_rw_semaphore *sem)
 
 void percpu_down_write(struct percpu_rw_semaphore *sem)
 {
-	down_write(&sem->rw_sem);
-
 	/* Notify readers to take the slow path. */
 	rcu_sync_enter(&sem->rss);
 
@@ -158,6 +156,7 @@ void percpu_down_write(struct percpu_rw_semaphore *sem)
 	 * therefore will wait for them.
 	 */
 
+	down_write(&sem->rw_sem);
 	/* Wait for all now active readers to complete. */
 	wait_event(sem->writer, readers_active_check(sem));
 }
-- 
1.5.5.1

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

[toc] | [prev] | [next] | [standalone]

#1173587 — [PATCH 3/3] percpu-rwsem: introduce percpu_rw_semaphore->recursive mode

Add percpu_rw_semaphore->recursive boolean. If it is true then the
recursive percpu_down_read() is safe, percpu_down_write() doesn't
exclude the new readers, like cpu_hotplug_begin().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/percpu-rwsem.h  |   15 ++++++++++-----
 kernel/events/uprobes.c       |    2 +-
 kernel/locking/percpu-rwsem.c |   15 +++++++++++----
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
index 9202e73..9441abd 100644
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -13,16 +13,18 @@ struct percpu_rw_semaphore {
 	int			state;
 	struct rcu_sync_struct	rss;
 	wait_queue_head_t	writer;
+	bool			recursive;
 	struct rw_semaphore	rw_sem;
 };
 
-#define DEFINE_STATIC_PERCPU_RWSEM(name)				\
+#define DEFINE_STATIC_PERCPU_RWSEM(name, rec)				\
 static DEFINE_PER_CPU(unsigned int, __percpu_rwsem_refcount_##name);	\
 static struct percpu_rw_semaphore name = {				\
 	.refcount = &__percpu_rwsem_refcount_##name,			\
 	.state = 0,							\
 	.rss = __RCU_SYNC_INITIALIZER(name.rss, RCU_SCHED_SYNC, 1),	\
 	.writer = __WAIT_QUEUE_HEAD_INITIALIZER(name.writer),		\
+	.recursive = rec,						\
 	.rw_sem = __RWSEM_INITIALIZER(name.rw_sem),			\
 }
 
@@ -37,7 +39,10 @@ static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
 {
 	might_sleep();
 
-	rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
+	if (sem->recursive)
+		rwlock_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
+	else
+		rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
 
 	preempt_disable();
 	/*
@@ -97,14 +102,14 @@ static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 extern void percpu_down_write(struct percpu_rw_semaphore *);
 extern void percpu_up_write(struct percpu_rw_semaphore *);
 
-extern int __percpu_init_rwsem(struct percpu_rw_semaphore *,
+extern int __percpu_init_rwsem(struct percpu_rw_semaphore *, bool,
 				const char *, struct lock_class_key *);
 extern void percpu_free_rwsem(struct percpu_rw_semaphore *);
 
-#define percpu_init_rwsem(sem)					\
+#define percpu_init_rwsem(sem, recursive)			\
 ({								\
 	static struct lock_class_key rwsem_key;			\
-	__percpu_init_rwsem(sem, #sem, &rwsem_key);		\
+	__percpu_init_rwsem(sem, recursive, #sem, &rwsem_key);	\
 })
 
 #endif
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index cb346f2..a4813a1 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1985,7 +1985,7 @@ static int __init init_uprobes(void)
 	for (i = 0; i < UPROBES_HASH_SZ; i++)
 		mutex_init(&uprobes_mmap_mutex[i]);
 
-	if (percpu_init_rwsem(&dup_mmap_sem))
+	if (percpu_init_rwsem(&dup_mmap_sem, false))
 		return -ENOMEM;
 
 	return register_die_notifier(&uprobe_exception_nb);
diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index 609c13b..3db7c45 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -10,7 +10,7 @@
 
 enum { readers_slow, readers_block };
 
-int __percpu_init_rwsem(struct percpu_rw_semaphore *sem,
+int __percpu_init_rwsem(struct percpu_rw_semaphore *sem, bool recursive,
 			const char *name, struct lock_class_key *rwsem_key)
 {
 	sem->refcount = alloc_percpu(unsigned int);
@@ -20,6 +20,7 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *sem,
 	sem->state = readers_slow;
 	rcu_sync_init(&sem->rss, RCU_SCHED_SYNC, true);
 	init_waitqueue_head(&sem->writer);
+	sem->recursive = recursive;
 	__init_rwsem(&sem->rw_sem, name, rwsem_key);
 
 	return 0;
@@ -124,9 +125,15 @@ void __percpu_up_read(struct percpu_rw_semaphore *sem)
  */
 static bool readers_active_check(struct percpu_rw_semaphore *sem)
 {
-	if (per_cpu_sum(*sem->refcount) != 0)
+	if (sem->recursive && !down_write_trylock(&sem->rw_sem))
 		return false;
 
+	if (per_cpu_sum(*sem->refcount) != 0) {
+		if (sem->recursive)
+			up_write(&sem->rw_sem);
+		return false;
+	}
+
 	/*
 	 * If we observed the decrement; ensure we see the entire critical
 	 * section.
@@ -155,8 +162,8 @@ void percpu_down_write(struct percpu_rw_semaphore *sem)
 	 * then we are guaranteed to see their sem->refcount increment, and
 	 * therefore will wait for them.
 	 */
-
-	down_write(&sem->rw_sem);
+	if (!sem->recursive)
+		down_write(&sem->rw_sem);
 	/* Wait for all now active readers to complete. */
 	wait_event(sem->writer, readers_active_check(sem));
 }
-- 
1.5.5.1

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

[toc] | [prev] | [next] | [standalone]

#1173589 — [PATCH 1/3] rcusync: introduce rcu_sync_struct->exclusive mode

Add rcu_sync_struct->exclusive boolean set by rcu_sync_init(), it
obviously controls the exclusiveness of rcu_sync_enter(). This is
what percpu_down_write() actually wants.

We turn ->gp_wait into "struct completion gp_comp", it is used as
a resource counter in "exclusive" mode. Otherwise we only use its
completion->wait member for wait_event/wake_up_all. We never mix
the completion/wait_queue_head_t operations.

TODO: we can cleanup this logic and avoid "struct completion", but
this needs a bit more changes.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/percpu-rwsem.h  |    2 +-
 include/linux/rcusync.h       |   29 ++++++++++++++++-------------
 kernel/locking/percpu-rwsem.c |    2 +-
 kernel/rcu/sync.c             |   25 ++++++++++++++++++++-----
 4 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
index e12ce86..9202e73 100644
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -21,7 +21,7 @@ static DEFINE_PER_CPU(unsigned int, __percpu_rwsem_refcount_##name);	\
 static struct percpu_rw_semaphore name = {				\
 	.refcount = &__percpu_rwsem_refcount_##name,			\
 	.state = 0,							\
-	.rss = __RCU_SYNC_INITIALIZER(name.rss, RCU_SCHED_SYNC),	\
+	.rss = __RCU_SYNC_INITIALIZER(name.rss, RCU_SCHED_SYNC, 1),	\
 	.writer = __WAIT_QUEUE_HEAD_INITIALIZER(name.writer),		\
 	.rw_sem = __RWSEM_INITIALIZER(name.rw_sem),			\
 }
diff --git a/include/linux/rcusync.h b/include/linux/rcusync.h
index 0135838..aaea86a 100644
--- a/include/linux/rcusync.h
+++ b/include/linux/rcusync.h
@@ -1,7 +1,7 @@
 #ifndef _LINUX_RCUSYNC_H_
 #define _LINUX_RCUSYNC_H_
 
-#include <linux/wait.h>
+#include <linux/completion.h>
 #include <linux/rcupdate.h>
 
 enum rcu_sync_type { RCU_SYNC, RCU_SCHED_SYNC, RCU_BH_SYNC };
@@ -9,11 +9,12 @@ enum rcu_sync_type { RCU_SYNC, RCU_SCHED_SYNC, RCU_BH_SYNC };
 struct rcu_sync_struct {
 	int			gp_state;
 	int			gp_count;
-	wait_queue_head_t	gp_wait;
+	struct completion	gp_comp;
 
 	int			cb_state;
 	struct rcu_head		cb_head;
 
+	bool			exclusive;
 	enum rcu_sync_type	gp_type;
 };
 
@@ -28,30 +29,32 @@ static inline bool rcu_sync_is_idle(struct rcu_sync_struct *rss)
 #endif
 }
 
-extern void rcu_sync_init(struct rcu_sync_struct *, enum rcu_sync_type);
+extern void rcu_sync_init(struct rcu_sync_struct *,
+				enum rcu_sync_type, bool excl);
 extern void rcu_sync_enter(struct rcu_sync_struct *);
 extern void rcu_sync_exit(struct rcu_sync_struct *);
 extern void rcu_sync_dtor(struct rcu_sync_struct *);
 
-#define __RCU_SYNC_INITIALIZER(name, type) {				\
+#define __RCU_SYNC_INITIALIZER(name, type, excl) {			\
 		.gp_state = 0,						\
 		.gp_count = 0,						\
-		.gp_wait = __WAIT_QUEUE_HEAD_INITIALIZER(name.gp_wait),	\
+		.gp_comp = COMPLETION_INITIALIZER(name.gp_comp),	\
 		.cb_state = 0,						\
+		.exclusive = excl,					\
 		.gp_type = type,					\
 	}
 
-#define	__DEFINE_RCU_SYNC(name, type)	\
-	struct rcu_sync_struct name = __RCU_SYNC_INITIALIZER(name, type)
+#define	__DEFINE_RCU_SYNC(name, type, excl)	\
+	struct rcu_sync_struct name = __RCU_SYNC_INITIALIZER(name, type, excl)
 
-#define DEFINE_RCU_SYNC(name)		\
-	__DEFINE_RCU_SYNC(name, RCU_SYNC)
+#define DEFINE_RCU_SYNC(name, excl)		\
+	__DEFINE_RCU_SYNC(name, RCU_SYNC, excl)
 
-#define DEFINE_RCU_SCHED_SYNC(name)	\
-	__DEFINE_RCU_SYNC(name, RCU_SCHED_SYNC)
+#define DEFINE_RCU_SCHED_SYNC(name, excl)	\
+	__DEFINE_RCU_SYNC(name, RCU_SCHED_SYNC, excl)
 
-#define DEFINE_RCU_BH_SYNC(name)	\
-	__DEFINE_RCU_SYNC(name, RCU_BH_SYNC)
+#define DEFINE_RCU_BH_SYNC(name, excl)	\
+	__DEFINE_RCU_SYNC(name, RCU_BH_SYNC, excl)
 
 #endif /* _LINUX_RCUSYNC_H_ */
 
diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index 915646c..014d2f4 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -18,7 +18,7 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *sem,
 		return -ENOMEM;
 
 	sem->state = readers_slow;
-	rcu_sync_init(&sem->rss, RCU_SCHED_SYNC);
+	rcu_sync_init(&sem->rss, RCU_SCHED_SYNC, true);
 	init_waitqueue_head(&sem->writer);
 	__init_rwsem(&sem->rw_sem, name, rwsem_key);
 
diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
index 8835ad1..03ddc61 100644
--- a/kernel/rcu/sync.c
+++ b/kernel/rcu/sync.c
@@ -38,7 +38,8 @@ static const struct {
 enum { GP_IDLE = 0, GP_PENDING, GP_PASSED };
 enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };
 
-#define	rss_lock	gp_wait.lock
+#define	rss_lock	gp_comp.wait.lock
+#define	gp_wait		gp_comp.wait
 
 #ifdef CONFIG_PROVE_RCU
 bool __rcu_sync_is_idle(struct rcu_sync_struct *rss)
@@ -49,10 +50,12 @@ bool __rcu_sync_is_idle(struct rcu_sync_struct *rss)
 EXPORT_SYMBOL_GPL(__rcu_sync_is_idle);
 #endif
 
-void rcu_sync_init(struct rcu_sync_struct *rss, enum rcu_sync_type type)
+void rcu_sync_init(struct rcu_sync_struct *rss,
+			enum rcu_sync_type type, bool excl)
 {
 	memset(rss, 0, sizeof(*rss));
-	init_waitqueue_head(&rss->gp_wait);
+	init_completion(&rss->gp_comp);
+	rss->exclusive = excl;
 	rss->gp_type = type;
 }
 
@@ -72,9 +75,13 @@ void rcu_sync_enter(struct rcu_sync_struct *rss)
 	if (need_sync) {
 		gp_ops[rss->gp_type].sync();
 		rss->gp_state = GP_PASSED;
-		wake_up_all(&rss->gp_wait);
+		if (!rss->exclusive)
+			wake_up_all(&rss->gp_wait);
 	} else if (need_wait) {
-		wait_event(rss->gp_wait, rss->gp_state == GP_PASSED);
+		if (!rss->exclusive)
+			wait_event(rss->gp_wait, rss->gp_state == GP_PASSED);
+		else
+			wait_for_completion(&rss->gp_comp);
 	} else {
 		/*
 		 * Possible when there's a pending CB from a rcu_sync_exit().
@@ -119,6 +126,12 @@ static void rcu_sync_func(struct rcu_head *rcu)
 	spin_unlock_irqrestore(&rss->rss_lock, flags);
 }
 
+static inline void __complete_locked(struct completion *x)
+{
+	x->done++;
+	__wake_up_locked(&x->wait, TASK_NORMAL, 1);
+}
+
 void rcu_sync_exit(struct rcu_sync_struct *rss)
 {
 	spin_lock_irq(&rss->rss_lock);
@@ -129,6 +142,8 @@ void rcu_sync_exit(struct rcu_sync_struct *rss)
 		} else if (rss->cb_state == CB_PENDING) {
 			rss->cb_state = CB_REPLAY;
 		}
+	} else if (rss->exclusive) {
+		__complete_locked(&rss->gp_comp);
 	}
 	spin_unlock_irq(&rss->rss_lock);
 }
-- 
1.5.5.1

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web