Re: [-mm PATCH] RCU: debug sleep check
From: Dipankar Sarma
Date: Sun Sep 24 2006 - 17:35:51 EST
On Sun, Sep 24, 2006 at 11:56:46AM -0700, Andrew Morton wrote:
> On Mon, 25 Sep 2006 00:05:09 +0530
> Dipankar Sarma <dipankar@xxxxxxxxxx> wrote:
>
> > Add a debug check for rcu read-side critical section code calling
> > a function that might sleep which is illegal. The check is enabled only
> > if CONFIG_DEBUG_SPINLOCK_SLEEP is set.
> >
>
> Does this actually change anything? rcu_read_lock is preempt_disable(), and
> might_sleep() already triggers if called inside preempt_disable().
It makes a difference if CONFIG_PREEMPT=n. AFAICS, preempt_disable()
is a nop then and rcu needs its own check for sleeping while
in read-side critical section. Anyway, I need to fix the rcupreempt
part of this check, update patch is included below.
>
> > -#define rcu_read_lock() __rcu_read_lock()
> > +#define rcu_read_lock() \
> > + do { \
> > + rcu_add_read_count(); \
> > + __rcu_read_lock(); \
>
> I don't have any __rcu_read_lock(). I guess this is against your
> to-be-resent RCU patches?
Yes, they are up in http://www.hill9.org/linux/kernel/patches/2.6.18-mm1/.
However I am still running some tests.
>
> > +DEFINE_PER_CPU(int, rcu_read_count);
>
> Can have static scope.
Done.
Thanks
Dipankar
Add a debug check for rcu read-side critical section code calling
a function that might sleep which is illegal. The check is enabled only
if CONFIG_DEBUG_SPINLOCK_SLEEP is set.
Signed-off-by: Dipankar Sarma <dipankar@xxxxxxxxxx>
---
include/linux/rcuclassic.h | 12 ++++++++++++
include/linux/rcupdate.h | 6 ++++++
kernel/rcuclassic.c | 33 +++++++++++++++++++++++++++++++++
kernel/rcupreempt.c | 7 +++++++
kernel/sched.c | 2 +-
5 files changed, 59 insertions(+), 1 deletion(-)
diff -puN include/linux/rcupdate.h~rcu-reader-sleep-check include/linux/rcupdate.h
--- linux-2.6.18-mm1-rcu/include/linux/rcupdate.h~rcu-reader-sleep-check 2006-09-25 00:49:12.000000000 +0530
+++ linux-2.6.18-mm1-rcu-dipankar/include/linux/rcupdate.h 2006-09-25 00:56:02.000000000 +0530
@@ -64,6 +64,12 @@ struct rcu_head {
(ptr)->next = NULL; (ptr)->func = NULL; \
} while (0)
+#ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
+extern int rcu_read_in_atomic(void);
+#else
+static inline int rcu_read_in_atomic(void) { return 0;}
+#endif
+
/**
* rcu_read_lock - mark the beginning of an RCU read-side critical section.
*
diff -puN kernel/sched.c~rcu-reader-sleep-check kernel/sched.c
--- linux-2.6.18-mm1-rcu/kernel/sched.c~rcu-reader-sleep-check 2006-09-25 00:49:12.000000000 +0530
+++ linux-2.6.18-mm1-rcu-dipankar/kernel/sched.c 2006-09-25 00:49:12.000000000 +0530
@@ -6974,7 +6974,7 @@ void __might_sleep(char *file, int line)
#ifdef in_atomic
static unsigned long prev_jiffy; /* ratelimiting */
- if ((in_atomic() || irqs_disabled()) &&
+ if ((in_atomic() || irqs_disabled() || rcu_read_in_atomic()) &&
system_state == SYSTEM_RUNNING && !oops_in_progress) {
if (time_before(jiffies, prev_jiffy + HZ) && prev_jiffy)
return;
diff -puN kernel/rcuclassic.c~rcu-reader-sleep-check kernel/rcuclassic.c
--- linux-2.6.18-mm1-rcu/kernel/rcuclassic.c~rcu-reader-sleep-check 2006-09-25 00:49:58.000000000 +0530
+++ linux-2.6.18-mm1-rcu-dipankar/kernel/rcuclassic.c 2006-09-25 00:51:45.000000000 +0530
@@ -545,6 +545,39 @@ void __init __rcu_init(void)
register_cpu_notifier(&rcu_nb);
}
+#ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
+static DEFINE_PER_CPU(int, rcu_read_count);
+int rcu_read_in_atomic(void)
+{
+ int val;
+ int cpu = get_cpu();
+ val = per_cpu(rcu_read_count, cpu);
+ put_cpu();
+ return val;
+}
+
+void rcu_add_read_count(void)
+{
+ int cpu, flags;
+ local_irq_save(flags);
+ cpu = smp_processor_id();
+ per_cpu(rcu_read_count, cpu)++;
+ local_irq_restore(flags);
+}
+
+void rcu_sub_read_count(void)
+{
+ int cpu, flags;
+ local_irq_save(flags);
+ cpu = smp_processor_id();
+ per_cpu(rcu_read_count, cpu)--;
+ local_irq_restore(flags);
+}
+EXPORT_SYMBOL_GPL(rcu_read_in_atomic);
+EXPORT_SYMBOL_GPL(rcu_add_read_count);
+EXPORT_SYMBOL_GPL(rcu_sub_read_count);
+#endif
+
module_param(blimit, int, 0);
module_param(qhimark, int, 0);
module_param(qlowmark, int, 0);
diff -puN include/linux/rcuclassic.h~rcu-reader-sleep-check include/linux/rcuclassic.h
--- linux-2.6.18-mm1-rcu/include/linux/rcuclassic.h~rcu-reader-sleep-check 2006-09-25 00:56:24.000000000 +0530
+++ linux-2.6.18-mm1-rcu-dipankar/include/linux/rcuclassic.h 2006-09-25 02:33:03.000000000 +0530
@@ -115,25 +115,37 @@ static inline void rcu_bh_qsctr_inc(int
extern int rcu_pending(int cpu);
extern int rcu_needs_cpu(int cpu);
+#ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
+extern void rcu_add_read_count(void);
+extern void rcu_sub_read_count(void);
+#else
+static inline void rcu_add_read_count(void) {}
+static inline void rcu_sub_read_count(void) {}
+#endif
+
#define __rcu_read_lock() \
do { \
preempt_disable(); \
+ rcu_add_read_count(); \
__acquire(RCU); \
} while(0)
#define __rcu_read_unlock() \
do { \
__release(RCU); \
+ rcu_sub_read_count(); \
preempt_enable(); \
} while(0)
#define __rcu_read_lock_bh() \
do { \
local_bh_disable(); \
+ rcu_add_read_count(); \
__acquire(RCU_BH); \
} while(0)
#define __rcu_read_unlock_bh() \
do { \
__release(RCU_BH); \
+ rcu_sub_read_count(); \
local_bh_enable(); \
} while(0)
diff -puN kernel/rcupreempt.c~rcu-reader-sleep-check kernel/rcupreempt.c
--- linux-2.6.18-mm1-rcu/kernel/rcupreempt.c~rcu-reader-sleep-check 2006-09-25 01:01:11.000000000 +0530
+++ linux-2.6.18-mm1-rcu-dipankar/kernel/rcupreempt.c 2006-09-25 01:07:36.000000000 +0530
@@ -424,6 +424,13 @@ int rcu_read_proc_ctrs_data(char *page)
#endif /* #ifdef CONFIG_RCU_TRACE */
+#ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
+int rcu_read_in_atomic(void)
+{
+ return current->rcu_read_lock_nesting;
+}
+#endif
+
EXPORT_SYMBOL_GPL(call_rcu);
EXPORT_SYMBOL_GPL(rcu_batches_completed);
EXPORT_SYMBOL_GPL(rcu_batches_completed_bh);
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/