On Mittwoch, 30. Juli 2008, Chirag Jog wrote:
> * J?rgen Mell <[email protected]> [2008-07-30 11:01:32]:
> > On Wednesday, 30. July 2008, Thomas Gleixner wrote:
> > > We are pleased to announce the 2.6.26-rt1 tree, which can be
> > > downloaded from the location:
> >
> > The bad news: It produces continuously lots of bug messages in the
> > error logs (cf. attached dmesg.tgz). The error at rtmutex.c:743 was
> > already present in 2.6.25-rt* when ACPI was enabled. The 'using
> > smp_processor_id () in preemptible code' is new here with 2.6.26.
> >
>
> This patch should solve some of the bug messages.
> It does two things:
> 1. Change rt_runtime_lock to be a raw spinlock as the comment above it
> says: it is nested inside the rq lock.
>
> 2. Change mnt_writers to be a per_cpu locked variable.
> This eliminates the need for the codepath to disable preemption and
> then potentially sleep, leading to the BUG messages
>
> Signed-Off-By: Chirag <[email protected]>
>
>
>
> Index: linux-2.6.26-rt1/kernel/sched.c
> ===================================================================
> --- linux-2.6.26-rt1.orig/kernel/sched.c 2008-07-30 22:37:19.000000000
> +0530 +++ linux-2.6.26-rt1/kernel/sched.c 2008-07-30 22:37:24.000000000
> +0530 @@ -208,7 +208,7 @@
>
> struct rt_bandwidth {
> /* nests inside the rq lock: */
> - spinlock_t rt_runtime_lock;
> + raw_spinlock_t rt_runtime_lock;
> ktime_t rt_period;
> u64 rt_runtime;
> struct hrtimer rt_period_timer;
> @@ -472,7 +472,7 @@
> u64 rt_time;
> u64 rt_runtime;
> /* Nests inside the rq lock: */
> - spinlock_t rt_runtime_lock;
> + raw_spinlock_t rt_runtime_lock;
>
> #ifdef CONFIG_RT_GROUP_SCHED
> unsigned long rt_nr_boosted;
> Index: linux-2.6.26-rt1/fs/namespace.c
> ===================================================================
> --- linux-2.6.26-rt1.orig/fs/namespace.c 2008-07-30 22:39:30.000000000
> +0530 +++ linux-2.6.26-rt1/fs/namespace.c 2008-07-30 22:39:36.000000000
> +0530 @@ -178,13 +178,13 @@
> unsigned long count;
> struct vfsmount *mnt;
> } ____cacheline_aligned_in_smp;
> -static DEFINE_PER_CPU(struct mnt_writer, mnt_writers);
> +static DEFINE_PER_CPU_LOCKED(struct mnt_writer, mnt_writers);
>
> static int __init init_mnt_writers(void)
> {
> int cpu;
> for_each_possible_cpu(cpu) {
> - struct mnt_writer *writer = &per_cpu(mnt_writers, cpu);
> + struct mnt_writer *writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_lock_init(&writer->lock);
> lockdep_set_class(&writer->lock, &writer->lock_class);
> writer->count = 0;
> @@ -199,7 +199,7 @@
> struct mnt_writer *cpu_writer;
>
> for_each_possible_cpu(cpu) {
> - cpu_writer = &per_cpu(mnt_writers, cpu);
> + cpu_writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_unlock(&cpu_writer->lock);
> }
> }
> @@ -251,8 +251,8 @@
> {
> int ret = 0;
> struct mnt_writer *cpu_writer;
> -
> - cpu_writer = &get_cpu_var(mnt_writers);
> + int cpu = 0;
> + cpu_writer = &get_cpu_var_locked(mnt_writers, &cpu);
> spin_lock(&cpu_writer->lock);
> if (__mnt_is_readonly(mnt)) {
> ret = -EROFS;
> @@ -262,7 +262,7 @@
> cpu_writer->count++;
> out:
> spin_unlock(&cpu_writer->lock);
> - put_cpu_var(mnt_writers);
> + put_cpu_var_locked(mnt_writers, cpu);
> return ret;
> }
> EXPORT_SYMBOL_GPL(mnt_want_write);
> @@ -273,7 +273,7 @@
> struct mnt_writer *cpu_writer;
>
> for_each_possible_cpu(cpu) {
> - cpu_writer = &per_cpu(mnt_writers, cpu);
> + cpu_writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_lock(&cpu_writer->lock);
> __clear_mnt_count(cpu_writer);
> cpu_writer->mnt = NULL;
> @@ -332,8 +332,8 @@
> {
> int must_check_underflow = 0;
> struct mnt_writer *cpu_writer;
> -
> - cpu_writer = &get_cpu_var(mnt_writers);
> + int cpu = 0;
> + cpu_writer = &get_cpu_var_locked(mnt_writers, &cpu);
> spin_lock(&cpu_writer->lock);
>
> use_cpu_writer_for_mount(cpu_writer, mnt);
> @@ -360,7 +360,7 @@
> * __mnt_writers can underflow. Without it,
> * we could theoretically wrap __mnt_writers.
> */
> - put_cpu_var(mnt_writers);
> + put_cpu_var_locked(mnt_writers, cpu);
> }
> EXPORT_SYMBOL_GPL(mnt_drop_write);
>
> @@ -612,7 +612,7 @@
> * can come in.
> */
> for_each_possible_cpu(cpu) {
> - struct mnt_writer *cpu_writer = &per_cpu(mnt_writers, cpu);
> + struct mnt_writer *cpu_writer = &per_cpu_var_locked(mnt_writers,
> cpu); if (cpu_writer->mnt != mnt)
> continue;
> spin_lock(&cpu_writer->lock);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-rt-users" in the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Yes, the number of bug messages is reduced very much by this patch,
although both messages are still present in the log (cf. attached dmesg).
But with the patch they only occur sporadically after the system has
completed start-up and no longer continuously. Thanks a lot!

Bye,
J?rgen

Attachments:

(No filename) (4.86 kB)
dmesg.tgz (10.29 kB)
Download all attachments

2008-07-31 06:07:01

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH] Fix Bug messages

On Wed, 2008-07-30 at 22:48 +0530, Chirag Jog wrote:
> * J?rgen Mell <[email protected]> [2008-07-30 11:01:32]:
>
> > Hello Thomas,
> >
> > On Wednesday, 30. July 2008, Thomas Gleixner wrote:
> > > We are pleased to announce the 2.6.26-rt1 tree, which can be
> > > downloaded from the location:
> >
> > I have tried the new kernel and have some good news and some bad news:
> >
> > The good news: The machine boots and seems to run without major problems.
> >
> > The bad news: It produces continuously lots of bug messages in the error
> > logs (cf. attached dmesg.tgz). The error at rtmutex.c:743 was already
> > present in 2.6.25-rt* when ACPI was enabled. The 'using smp_processor_id
> > () in preemptible code' is new here with 2.6.26.
> >
> > Machine is an old Athlon XP (single core) on an EPOX mainboard with VIA
> > chipset.
> >
> > If I can help with testing, please let me know.
> >
> > Bye,
> > Jürgen
> >
> >
> This patch should solve some of the bug messages.
> It does two things:
> 1. Change rt_runtime_lock to be a raw spinlock as the comment above it
> says: it is nested inside the rq lock.
>
> 2. Change mnt_writers to be a per_cpu locked variable.
> This eliminates the need for the codepath to disable preemption and
> then potentially sleep, leading to the BUG messages

Looks sane, thanks Chirag!

> Signed-Off-By: Chirag <[email protected]>
>
>
>
> Index: linux-2.6.26-rt1/kernel/sched.c
> ===================================================================
> --- linux-2.6.26-rt1.orig/kernel/sched.c 2008-07-30 22:37:19.000000000 +0530
> +++ linux-2.6.26-rt1/kernel/sched.c 2008-07-30 22:37:24.000000000 +0530
> @@ -208,7 +208,7 @@
>
> struct rt_bandwidth {
> /* nests inside the rq lock: */
> - spinlock_t rt_runtime_lock;
> + raw_spinlock_t rt_runtime_lock;
> ktime_t rt_period;
> u64 rt_runtime;
> struct hrtimer rt_period_timer;
> @@ -472,7 +472,7 @@
> u64 rt_time;
> u64 rt_runtime;
> /* Nests inside the rq lock: */
> - spinlock_t rt_runtime_lock;
> + raw_spinlock_t rt_runtime_lock;
>
> #ifdef CONFIG_RT_GROUP_SCHED
> unsigned long rt_nr_boosted;
> Index: linux-2.6.26-rt1/fs/namespace.c
> ===================================================================
> --- linux-2.6.26-rt1.orig/fs/namespace.c 2008-07-30 22:39:30.000000000 +0530
> +++ linux-2.6.26-rt1/fs/namespace.c 2008-07-30 22:39:36.000000000 +0530
> @@ -178,13 +178,13 @@
> unsigned long count;
> struct vfsmount *mnt;
> } ____cacheline_aligned_in_smp;
> -static DEFINE_PER_CPU(struct mnt_writer, mnt_writers);
> +static DEFINE_PER_CPU_LOCKED(struct mnt_writer, mnt_writers);
>
> static int __init init_mnt_writers(void)
> {
> int cpu;
> for_each_possible_cpu(cpu) {
> - struct mnt_writer *writer = &per_cpu(mnt_writers, cpu);
> + struct mnt_writer *writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_lock_init(&writer->lock);
> lockdep_set_class(&writer->lock, &writer->lock_class);
> writer->count = 0;
> @@ -199,7 +199,7 @@
> struct mnt_writer *cpu_writer;
>
> for_each_possible_cpu(cpu) {
> - cpu_writer = &per_cpu(mnt_writers, cpu);
> + cpu_writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_unlock(&cpu_writer->lock);
> }
> }
> @@ -251,8 +251,8 @@
> {
> int ret = 0;
> struct mnt_writer *cpu_writer;
> -
> - cpu_writer = &get_cpu_var(mnt_writers);
> + int cpu = 0;
> + cpu_writer = &get_cpu_var_locked(mnt_writers, &cpu);
> spin_lock(&cpu_writer->lock);
> if (__mnt_is_readonly(mnt)) {
> ret = -EROFS;
> @@ -262,7 +262,7 @@
> cpu_writer->count++;
> out:
> spin_unlock(&cpu_writer->lock);
> - put_cpu_var(mnt_writers);
> + put_cpu_var_locked(mnt_writers, cpu);
> return ret;
> }
> EXPORT_SYMBOL_GPL(mnt_want_write);
> @@ -273,7 +273,7 @@
> struct mnt_writer *cpu_writer;
>
> for_each_possible_cpu(cpu) {
> - cpu_writer = &per_cpu(mnt_writers, cpu);
> + cpu_writer = &per_cpu_var_locked(mnt_writers, cpu);
> spin_lock(&cpu_writer->lock);
> __clear_mnt_count(cpu_writer);
> cpu_writer->mnt = NULL;
> @@ -332,8 +332,8 @@
> {
> int must_check_underflow = 0;
> struct mnt_writer *cpu_writer;
> -
> - cpu_writer = &get_cpu_var(mnt_writers);
> + int cpu = 0;
> + cpu_writer = &get_cpu_var_locked(mnt_writers, &cpu);
> spin_lock(&cpu_writer->lock);
>
> use_cpu_writer_for_mount(cpu_writer, mnt);
> @@ -360,7 +360,7 @@
> * __mnt_writers can underflow. Without it,
> * we could theoretically wrap __mnt_writers.
> */
> - put_cpu_var(mnt_writers);
> + put_cpu_var_locked(mnt_writers, cpu);
> }
> EXPORT_SYMBOL_GPL(mnt_drop_write);
>
> @@ -612,7 +612,7 @@
> * can come in.
> */
> for_each_possible_cpu(cpu) {
> - struct mnt_writer *cpu_writer = &per_cpu(mnt_writers, cpu);
> + struct mnt_writer *cpu_writer = &per_cpu_var_locked(mnt_writers, cpu);
> if (cpu_writer->mnt != mnt)
> continue;
> spin_lock(&cpu_writer->lock);
>
>

2008-07-31 08:00:08

by Sébastien Dugué

[permalink] [raw]

Subject: Re: [PATCH] Fix Bug messages

On Wed, 30 Jul 2008 22:48:42 +0530 Chirag Jog <[email protected]> wrote:

> * J?rgen Mell <[email protected]> [2008-07-30 11:01:32]:
>
> > Hello Thomas,
> >
> > On Wednesday, 30. July 2008, Thomas Gleixner wrote:
> > > We are pleased to announce the 2.6.26-rt1 tree, which can be
> > > downloaded from the location:
> >
> > I have tried the new kernel and have some good news and some bad news:
> >
> > The good news: The machine boots and seems to run without major problems.
> >
> > The bad news: It produces continuously lots of bug messages in the error
> > logs (cf. attached dmesg.tgz). The error at rtmutex.c:743 was already
> > present in 2.6.25-rt* when ACPI was enabled. The 'using smp_processor_id
> > () in preemptible code' is new here with 2.6.26.
> >
> > Machine is an old Athlon XP (single core) on an EPOX mainboard with VIA
> > chipset.
> >
> > If I can help with testing, please let me know.
> >
> > Bye,
> > Jürgen
> >
> >
> This patch should solve some of the bug messages.
> It does two things:
> 1. Change rt_runtime_lock to be a raw spinlock as the comment above it
> says: it is nested inside the rq lock.
>
> 2. Change mnt_writers to be a per_cpu locked variable.
> This eliminates the need for the codepath to disable preemption and
> then potentially sleep, leading to the BUG messages
>
> Signed-Off-By: Chirag <[email protected]>

Neat, the only remaining BUGs I see are from sock_prot_inuse_add()

BUG: using smp_processor_id() in preemptible [00000000] code: arping/1916
caller is .sock_prot_inuse_add+0x30/0x80
Call Trace:
[c0000000eed2f910] [c000000000010304] .show_stack+0x70/0x1bc (unreliable)
[c0000000eed2f9c0] [c0000000001a2340] .debug_smp_processor_id+0x138/0x168
[c0000000eed2fa70] [c0000000002181f4] .sock_prot_inuse_add+0x30/0x80
[c0000000eed2fb10] [c00000000026d96c] .udp_lib_get_port+0x2a8/0x320
[c0000000eed2fbc0] [c000000000275b30] .inet_bind+0x168/0x248
[c0000000eed2fc60] [c000000000215024] .sys_bind+0x98/0xdc
[c0000000eed2fd90] [c0000000002370bc] .compat_sys_socketcall+0xcc/0x214
[c0000000eed2fe30] [c0000000000086ac] syscall_exit+0x0/0x40
BUG: arping:1916 task might have lost a preemption check!
Call Trace:
[c0000000eed2f890] [c000000000010304] .show_stack+0x70/0x1bc (unreliable)
[c0000000eed2f940] [c00000000004e298] .preempt_enable_no_resched+0x60/0x78
[c0000000eed2f9c0] [c0000000001a2348] .debug_smp_processor_id+0x140/0x168
[c0000000eed2fa70] [c0000000002181f4] .sock_prot_inuse_add+0x30/0x80
[c0000000eed2fb10] [c00000000026d96c] .udp_lib_get_port+0x2a8/0x320
[c0000000eed2fbc0] [c000000000275b30] .inet_bind+0x168/0x248
[c0000000eed2fc60] [c000000000215024] .sys_bind+0x98/0xdc
[c0000000eed2fd90] [c0000000002370bc] .compat_sys_socketcall+0xcc/0x214
[c0000000eed2fe30] [c0000000000086ac] syscall_exit+0x0/0x40

...

Thanks,

Sebastien.

2008-07-31 10:13:44

On Wed, Aug 13, 2008 at 03:30:32PM +0200, Juergen Beisert wrote:
> On Freitag, 1. August 2008, Paul E. McKenney wrote:
> > On Wed, Jul 30, 2008 at 11:01:32AM +0200, J?rgen Mell wrote:
> > > On Wednesday, 30. July 2008, Thomas Gleixner wrote:
> > > > We are pleased to announce the 2.6.26-rt1 tree, which can be
> > > > downloaded from the location:
> > >
> > > I have tried the new kernel and have some good news and some bad news:
> > >
> > > The good news: The machine boots and seems to run without major problems.
> > >
> > > The bad news: It produces continuously lots of bug messages in the error
> > > logs (cf. attached dmesg.tgz). The error at rtmutex.c:743 was already
> > > present in 2.6.25-rt* when ACPI was enabled. The 'using smp_processor_id
> > > () in preemptible code' is new here with 2.6.26.
> > >
> > > Machine is an old Athlon XP (single core) on an EPOX mainboard with VIA
> > > chipset.
> > >
> > > If I can help with testing, please let me know.
> >
> > The following patch should clear things up. It is against 2.6.25.8-rt7,
> > but should still apply.
> >
> > ------------------------------------------------------------------------
> >
> > Disable preemption around statistics for RCU boost and unboost calls.
> >
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > ---
> >
> > rcupreempt-boost.c | 20 ++++++++++++++++----
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff -urpNa -X dontdiff linux-2.6.25.8-rt7/kernel/rcupreempt-boost.c
> > linux-2.6.25.8-rt7-preemptfix/kernel/rcupreempt-boost.c ---
> > linux-2.6.25.8-rt7/kernel/rcupreempt-boost.c 2008-06-24 14:14:52.000000000
> > -0700 +++
> > linux-2.6.25.8-rt7-preemptfix/kernel/rcupreempt-boost.c 2008-06-24
> > 14:38:59.000000000 -0700 @@ -205,11 +205,23 @@
> > RCU_BOOST_TRACE_FUNC_DECL(boost_readers)
> > RCU_BOOST_TRACE_FUNC_DECL(try_unboost_readers)
> > RCU_BOOST_TRACE_FUNC_DECL(unboost_readers)
> > RCU_BOOST_TRACE_FUNC_DECL(over_taken)
> > +static void rcu_trace_boost_boost_called_preempt(void)
> > +{
> > + preempt_disable();
> > + rcu_trace_boost_boost_called(RCU_BOOST_ME);
> > + preempt_enable();
> > +}
> > +static void rcu_trace_boost_unboost_called_preempt(void)
> > +{
> > + preempt_disable();
> > + rcu_trace_boost_unboost_called(RCU_BOOST_ME);
> > + preempt_enable();
> > +}
> > #else /* CONFIG_RCU_TRACE */
> > /* These were created by the above macro "RCU_BOOST_TRACE_FUNC_DECL" */
> > -# define rcu_trace_boost_task_boost_called(rbd) do { } while (0)
> > +# define rcu_trace_boost_task_boost_called_preempt(rbd) do { } while (0)
> > # define rcu_trace_boost_task_boosted(rbd) do { } while (0)
> > -# define rcu_trace_boost_boost_called(rbd) do { } while (0)
> > +# define rcu_trace_boost_boost_called_preempt(rbd) do { } while (0)
> > # define rcu_trace_boost_try_boost(rbd) do { } while (0)
> > # define rcu_trace_boost_boosted(rbd) do { } while (0)
> > # define rcu_trace_boost_unboost_called(rbd) do { } while (0)
> > @@ -261,7 +273,7 @@ void __rcu_preempt_boost(void)
> >
> > WARN_ON(!current->rcu_read_lock_nesting);
> >
> > - rcu_trace_boost_boost_called(RCU_BOOST_ME);
> > + rcu_trace_boost_boost_called_preempt();
> >
> > /* check to see if we are already boosted */
> > if (unlikely(rcu_is_boosted(curr)))
> > @@ -313,7 +325,7 @@ void __rcu_preempt_unboost(void)
> > int prio;
> > unsigned long flags;
> >
> > - rcu_trace_boost_unboost_called(RCU_BOOST_ME);
> > + rcu_trace_boost_unboost_called_preempt();
> >
> > /* if not boosted, then ignore */
> > if (likely(!rcu_is_boosted(curr)))
> >
> Using this patch on top of 2.6.26-rt1 results into:
>
> [...]
> CC kernel/rcupreempt-boost.o
> kernel/rcupreempt-boost.c: In function 'rcu_boost_task':
> kernel/rcupreempt-boost.c:249: error: implicit declaration of function 'rcu_trace_boost_task_boost_called'
> kernel/rcupreempt-boost.c: In function '__rcu_preempt_unboost':
> kernel/rcupreempt-boost.c:328: error: implicit declaration of function 'rcu_trace_boost_unboost_called_preempt'
>
> when CONFIG_PREEMPT_RCU_BOOST is enabled and target is a PowerPC of
> type MPC5200B.

Ah, 2.6.26-rt1 took a different approach to fix the issue my patch
was targeted at. Please accept my apologies for the noise!

The other thing to try would be Chirag Jog's patch sent out to
linux-rt-users on July 30.

Thanx, Paul