Skip to contentSkip to conversation

RCU and CONFIG_PREEMPT_RT progress, part 3

RFC In review Paul E. McKenney 3 participants 6 messages Jul 13, 2005

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 RCU and CONFIG_PREEMPT_RT progress, part 3

Hello!

Ported to CONFIG_PREEMPT_RT, and it actually boots! Running tests, working thus far. But thought I would post the patch and get feedback in the meantime, since I am not sure that my approach is correct. The questions:

1. Is use of spin_trylock() and spin_unlock() in hardirq code
(e.g., rcu_check_callbacks() and callees) a Bad Thing? Seems to result in boot-time hangs when I try it, and switching to _raw_spin_trylock() and _raw_spin_unlock() seems to work better. But I don't see why the other primitives hang -- after all, you can call wakeup functions in irq context in stock kernels...

2. Is _raw_spin_lock_irqsave() intended for general use? Its
API differs from that of spin_lock_irqsave(), so am wondering if it is internal-use-only or something. I currently use it from process context to acquire locks shared with rcu_check_callbacks().

3. Since SPIN_LOCK_UNLOCKED now takes the lock itself as an
argument, what is the best way to initialize per-CPU locks? An explicit initialization function, or is there some way that I am missing to make an initializer?

Anyway, here is the current state:

o Still have heavyweight operations in rcu_read_lock() and
rcu_read_unlock(). If the current code actually turns out to be robust, will work on removing the atomic_inc() and atomic_dec() first, with the memory barriers later.

o The code doesn't quite work in stock kernels, despite the
attempt. I will either make it work or remove the glue #ifdefs.

o Global callback queues result in poor SMP performance. On
the list to fix. Will likely require handling CPU hotplug (current code is too stupid to need to care about CPU hotplug).

o Grace-period-detection code is probably too aggressive, but will
worry about that later. This will interact with OOM on systems with small memories.

o There are likely still bugs. But you knew that already.

Current patch follows. As soon as it passes my overnight tests, it should be ready for other people to try out. However, it will still be a bit short on performance compared to Classic RCU.

Thanx, Paul

Not-signed-off-by <[email protected]>
fs/proc/proc_misc.c +37 -0
diff -urpN -X dontdiff linux-2.6.12-realtime-preempt-V0.7.51-27/fs/proc/proc_misc.c linux-2.6.12-realtime-preempt-V0.7.51-27-ctrRCU/fs/proc/proc_misc.c
@@ -599,6 +599,38 @@ void create_seq_entry(char *name, mode_t
 		entry->proc_fops = f;
 }
 
+#ifdef CONFIG_RCU_STATS
+int rcu_read_proc(char *page, char **start, off_t off,
+		  int count, int *eof, void *data)
+{
+	int len;
+	extern int rcu_read_proc_data(char *page);
+
+	len = rcu_read_proc_data(page);
+	return proc_calc_metrics(page, start, off, count, eof, len);
+}
+
+int rcu_read_proc_gp(char *page, char **start, off_t off,
+		     int count, int *eof, void *data)
+{
+	int len;
+	extern int rcu_read_proc_gp_data(char *page);
+
+	len = rcu_read_proc_gp_data(page);
+	return proc_calc_metrics(page, start, off, count, eof, len);
+}
+
+int rcu_read_proc_ptrs(char *page, char **start, off_t off,
+		       int count, int *eof, void *data)
+{
+	int len;
+	extern int rcu_read_proc_ptrs_data(char *page);
+
+	len = rcu_read_proc_ptrs_data(page);
+	return proc_calc_metrics(page, start, off, count, eof, len);
+}
+#endif /* #ifdef CONFIG_RCU_STATS */
+
 void __init proc_misc_init(void)
 {
 	struct proc_dir_entry *entry;
@@ -621,6 +653,11 @@ void __init proc_misc_init(void)
 		{"cmdline",	cmdline_read_proc},
 		{"locks",	locks_read_proc},
 		{"execdomains",	execdomains_read_proc},
+#ifdef CONFIG_RCU_STATS
+		{"rcustats",	rcu_read_proc},
+		{"rcugp",	rcu_read_proc_gp},
+		{"rcuptrs",	rcu_read_proc_ptrs},
+#endif /* #ifdef CONFIG_RCU_STATS */
 		{NULL,}
 	};
 	for (p = simple_ones; p->name; p++)
include/linux/rcupdate.h +25 -2
diff -urpN -X dontdiff linux-2.6.12-realtime-preempt-V0.7.51-27/include/linux/rcupdate.h linux-2.6.12-realtime-preempt-V0.7.51-27-ctrRCU/include/linux/rcupdate.h
@@ -59,6 +59,7 @@ struct rcu_head {
 } while (0)
 
 
+#ifndef CONFIG_PREEMPT_RCU
 
 /* Global control variables for rcupdate callback mechanism. */
 struct rcu_ctrlblk {
@@ -209,6 +210,18 @@ static inline int rcu_pending(int cpu)
 # define rcu_read_unlock preempt_enable
 #endif
 
+#else /* #ifndef CONFIG_PREEMPT_RCU */
+
+#define rcu_qsctr_inc(cpu)
+#define rcu_bh_qsctr_inc(cpu)
+#define call_rcu_bh(head, rcu) call_rcu(head, rcu)
+
+extern void rcu_read_lock(void);
+extern void rcu_read_unlock(void);
+extern int rcu_pending(int cpu);
+
+#endif /* #else #ifndef CONFIG_PREEMPT_RCU */
+
 /*
  * So where is rcu_write_lock()?  It does not exist, as there is no
  * way for writers to lock out RCU readers.  This is a feature, not
@@ -230,16 +243,22 @@ static inline int rcu_pending(int cpu)
  * can use just rcu_read_lock().
  *
  */
-//#define rcu_read_lock_bh()	local_bh_disable()
+#ifndef CONFIG_PREEMPT_RCU
+#define rcu_read_lock_bh()	local_bh_disable()
+#else /* #ifndef CONFIG_PREEMPT_RCU */
 #define rcu_read_lock_bh()	{ rcu_read_lock(); local_bh_disable(); }
+#endif /* #else #ifndef CONFIG_PREEMPT_RCU */
 
 /*
  * rcu_read_unlock_bh - marks the end of a softirq-only RCU critical section
  *
  * See rcu_read_lock_bh() for more information.
  */
-//#define rcu_read_unlock_bh()	local_bh_enable()
+#ifndef CONFIG_PREEMPT_RCU
+#define rcu_read_unlock_bh()	local_bh_enable()
+#else /* #ifndef CONFIG_PREEMPT_RCU */
 #define rcu_read_unlock_bh()	{ local_bh_enable(); rcu_read_unlock(); }
+#endif /* #else #ifndef CONFIG_PREEMPT_RCU */
 
 /**
  * rcu_dereference - fetch an RCU-protected pointer in an
@@ -288,7 +307,11 @@ static inline int rcu_pending(int cpu)
  * synchronize_kernel() API.  In contrast, synchronize_rcu() only
  * guarantees that rcu_read_lock() sections will have completed.
  */
+#ifndef CONFIG_PREEMPT_RCU
 #define synchronize_sched() synchronize_rcu()
+#else /* #ifndef CONFIG_PREEMPT_RCU */
+extern void synchronize_sched(void);
+#endif /* #else #ifndef CONFIG_PREEMPT_RCU */
 
 extern void rcu_init(void);
 extern void rcu_check_callbacks(int cpu, int user);
include/linux/sched.h +2 -1
diff -urpN -X dontdiff linux-2.6.12-realtime-preempt-V0.7.51-27/include/linux/sched.h linux-2.6.12-realtime-preempt-V0.7.51-27-ctrRCU/include/linux/sched.h
@@ -917,7 +917,8 @@ struct task_struct {
 #endif
 #ifdef CONFIG_PREEMPT_RCU
 	int rcu_read_lock_nesting;
-	struct rcu_data *rcu_data;
+	atomic_t *rcu_flipctr1;
+	atomic_t *rcu_flipctr2;
 #endif
 };
 
kernel/rcupdate.c +396 -58

484 lines not on this page · raw message

#1 lore raw
Quoted 3 lines · Paul E. McKenney

Hello!

Ported to CONFIG_PREEMPT_RT, and it actually boots! Running tests,

Good! :)

Quoted 11 lines

working thus far. But thought I would post the patch and get feedback in the meantime, since I am not sure that my approach is correct. The questions:

1. Is use of spin_trylock() and spin_unlock() in hardirq code
(e.g., rcu_check_callbacks() and callees) a Bad Thing? Seems to result in boot-time hangs when I try it, and switching to _raw_spin_trylock() and _raw_spin_unlock() seems to work better. But I don't see why the other primitives hang -- after all, you can call wakeup functions in irq context in stock kernels...

I never use _raw_spin_*. I just declare the lock as a raw_spinlock_t and the macro's determine to use them instead. So I just keep the spin_lock in the code. Or do you mean that you get problems using the spin_locks when the code is already defined as raw_spinlock_t?

Quoted 6 lines

2. Is _raw_spin_lock_irqsave() intended for general use? Its
API differs from that of spin_lock_irqsave(), so am wondering if it is internal-use-only or something. I currently use it from process context to acquire locks shared with rcu_check_callbacks().

I would assume not, but Ingo would be better at answering this.

Quoted 5 lines

3. Since SPIN_LOCK_UNLOCKED now takes the lock itself as an
argument, what is the best way to initialize per-CPU locks? An explicit initialization function, or is there some way that I am missing to make an initializer?

Ouch, I just notice that (been using an older version for some time).

Ingo, is this to force the initialization of the lists instead of at runtime?

-- Steve

#2 lore raw
Quoted 6 lines · Steven Rostedt
Quoted 3 lines · Paul E. McKenney

Hello!

Ported to CONFIG_PREEMPT_RT, and it actually boots! Running tests,

Good! :)

Hey, it even passed the touch-test (five kernbenches and an LTP). Now to start the torture tests...

Quoted 12 lines
Quoted 11 lines

working thus far. But thought I would post the patch and get feedback in the meantime, since I am not sure that my approach is correct. The questions:

1. Is use of spin_trylock() and spin_unlock() in hardirq code
(e.g., rcu_check_callbacks() and callees) a Bad Thing? Seems to result in boot-time hangs when I try it, and switching to _raw_spin_trylock() and _raw_spin_unlock() seems to work better. But I don't see why the other primitives hang -- after all, you can call wakeup functions in irq context in stock kernels...

Quoted 4 lines

I never use _raw_spin_*. I just declare the lock as a raw_spinlock_t and the macro's determine to use them instead. So I just keep the spin_lock in the code. Or do you mean that you get problems using the spin_locks when the code is already defined as raw_spinlock_t?

Wasn't aware of this possibility, will try it. Thanks for the tip!

Quoted 7 lines
Quoted 5 lines

2. Is _raw_spin_lock_irqsave() intended for general use? Its
API differs from that of spin_lock_irqsave(), so am wondering if it is internal-use-only or something. I currently use it from process context to acquire locks shared with rcu_check_callbacks().

I would assume not, but Ingo would be better at answering this.

Seems to work, FWIW. ;-)

Thanx, Paul

#3 lore raw
In reply to Steven Rostedt
Quoted 9 lines · Steven Rostedt
Quoted 4 lines

3. Since SPIN_LOCK_UNLOCKED now takes the lock itself as an
argument, what is the best way to initialize per-CPU locks? An explicit initialization function, or is there some way that I am missing to make an initializer?

Ouch, I just notice that (been using an older version for some time).

Ingo, is this to force the initialization of the lists instead of at runtime?

ANSI C99 is missing a concept of "self" during auto-intialization. The explicit passing of the lvalue is needed so that it can be propagated downward to other macros in the initialization structure. list_head initialization is one of those things if I remember correctly.

bill

#4 lore raw
In reply to Paul E. McKenney
Quoted 7 lines · Paul E. McKenney

1. Is use of spin_trylock() and spin_unlock() in hardirq code
(e.g., rcu_check_callbacks() and callees) a Bad Thing? Seems to result in boot-time hangs when I try it, and switching to _raw_spin_trylock() and _raw_spin_unlock() seems to work better. But I don't see why the other primitives hang -- after all, you can call wakeup functions in irq context in stock kernels...

The implementation of "printk" does funky stuff like this so I'm assuming it's sort of acceptable.

Some of those function bypass latency tracing and preemption violation checks. Don't see a reason why you should be touching those functions unless you're going to modify implementation of spinlocks directly. Just use spinlock_t/raw_spinlock_t to take advantage of the type parametrics in Ingo's spinlock code to determine which lock you're using and you should be fine.

bill

#5 lore raw
Quoted 9 lines · Bill Huey
Quoted 7 lines · Paul E. McKenney

1. Is use of spin_trylock() and spin_unlock() in hardirq code
(e.g., rcu_check_callbacks() and callees) a Bad Thing? Seems to result in boot-time hangs when I try it, and switching to _raw_spin_trylock() and _raw_spin_unlock() seems to work better. But I don't see why the other primitives hang -- after all, you can call wakeup functions in irq context in stock kernels...

Quoted 8 lines

The implementation of "printk" does funky stuff like this so I'm assuming it's sort of acceptable.

Some of those function bypass latency tracing and preemption violation checks. Don't see a reason why you should be touching those functions unless you're going to modify implementation of spinlocks directly. Just use spinlock_t/raw_spinlock_t to take advantage of the type parametrics in Ingo's spinlock code to determine which lock you're using and you should be fine.

Using raw_spinlock_t along with spin_lock_irqsave() and friends does seem to be working, thanks to both you and Steve for suggesting it!

Thanx, Paul