workqueue code needing preemption disabled

workqueue code needing preemption disabled

[Steven Rostedt]

Hi Tejun,

I'm debugging a crash on -rt that has the following:

kernel BUG at kernel/sched/core.c:1731!
invalid opcode: 0000 [#1] PREEMPT SMP 
CPU 5 
Pid: 16637, comm: kworker/5:0 Not tainted 3.6.11-rt30.25.el6rt.x86_64 #1 HP ProLiant DL580 G7
RIP: 0010:[<ffffffff8151ebea>]  [<ffffffff8151ebea>] __schedule+0x89a/0x8c0
RSP: 0018:ffff880fec355c30  EFLAGS: 00010006
RAX: ffff880fff951900 RBX: ffff880fff951900 RCX: ffffffffff48fb8a
RDX: 0000000000000001 RSI: 0000000000000005 RDI: 0000000000000000
RBP: ffff880fec355cc0 R08: 0000000000000001 R09: 0000000000000004
R10: 0000000000000004 R11: 0000000000000002 R12: 0000000000000005
R13: ffff880f61b417a0 R14: ffff883fff051900 R15: ffff880fec355d00
FS:  0000000000000000(0000) GS:ffff880fff940000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000003e0e98bd30 CR3: 0000000fe0348000 CR4: 00000000000007e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process kworker/5:0 (pid: 16637, threadinfo ffff880fec354000, task ffff880fe46d8000)
Stack:
 ffff880fea074d80 ffff880fec354010 ffff880fec354000 ffff880fec354010
 ffff880fec354000 ffff880fec354010 ffff880fec354000 ffff880fec354010
 ffff880fec354000 ffff880fec355fd8 0000000000000286 ffff880fe46d8000
Call Trace:
 [<ffffffff8151ed69>] schedule+0x29/0x70
 [<ffffffff8151f8ed>] rt_spin_lock_slowlock+0x10d/0x310
 [<ffffffff81240500>] ? ioc_destroy_icq+0xe0/0xe0
 [<ffffffff81240500>] ? ioc_destroy_icq+0xe0/0xe0
 [<ffffffff815200e6>] rt_spin_lock+0x26/0x30
 [<ffffffff8106418b>] process_one_work+0x1ab/0x560
 [<ffffffff81065f3b>] worker_thread+0x16b/0x510
 [<ffffffff8151e76b>] ? __schedule+0x41b/0x8c0
 [<ffffffff81065dd0>] ? manage_workers+0x340/0x340
 [<ffffffff8106b246>] kthread+0x96/0xa0
 [<ffffffff81528664>] kernel_thread_helper+0x4/0x10
 [<ffffffff8106b1b0>] ? kthreadd+0x1e0/0x1e0
 [<ffffffff81528660>] ? gs_change+0xb/0xb
Code: c4 01 00 00 00 00 00 40 e9 86 f8 ff ff 83 be 90 02 00 00 00 0f 85
20 f8 ff ff 48 89 f7 e8 df a0 b5 ff e9 13 f8 ff ff 0f 0b eb fe <0f> 0b
0f 1f 40 00 eb fa e8 d9 00 00 00 e9 07 fe ff ff 0f 0b 66 

The bug occurred on this line:

static void try_to_wake_up_local(struct task_struct *p)
{
        struct rq *rq = task_rq(p);

        BUG_ON(rq != this_rq());  <---- bug here
        BUG_ON(p == current);
        lockdep_assert_held(&rq->lock);

        if (!raw_spin_trylock(&p->pi_lock)) {
                raw_spin_unlock(&rq->lock);
                raw_spin_lock(&p->pi_lock);
                raw_spin_lock(&rq->lock);
        }


Now in your code you have the comment:

 * X: During normal operation, modification requires gcwq->lock and
 *    should be done only from local cpu.  Either disabling preemption
 *    on local cpu or grabbing gcwq->lock is enough for read access.
 *    If GCWQ_DISASSOCIATED is set, it's identical to L.

struct worker has flags marked with X.
struct worker_pool has flags and idle_list marked with X.

spin_locks in -rt do not disable preemption, nor do they disable irqs,
but they do disable migration. If there's code that depends on the
spin_lock disabling preemption, we need to either change the code to not
require that, or explicitly disable preemption in the critical paths.
Note, if we explicitly disable preemption, we can not call spin_locks
within those locations as in -rt a spin_lock can block and schedule.

I've tried to figure out the code but I'm not familiar with it enough to
know where the issues are as of yet. I was hoping that you could point
me at the trouble areas that would cause us issues when spin_locks() do
not disable preemption.

Thanks!

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

Hello, Steven.

On Mon, Mar 18, 2013 at 10:36:23AM -0400, Steven Rostedt wrote:
> kernel BUG at kernel/sched/core.c:1731!
> invalid opcode: 0000 [#1] PREEMPT SMP 
> CPU 5 
> Pid: 16637, comm: kworker/5:0 Not tainted 3.6.11-rt30.25.el6rt.x86_64 #1 HP ProLiant DL580 G7
...
> static void try_to_wake_up_local(struct task_struct *p)
> {
>         struct rq *rq = task_rq(p);
> 
>         BUG_ON(rq != this_rq());  <---- bug here

It's the local chain wake-up code used to main concurrency.  ie. when
a worker bound to a CPU schedules out it kicks another worker to take
its place (in concurrency level).

The function is called from inside __schedule() while holding rq->lock
and requires that the target task is on the same rq as the one trying
to wake it up.  When it isn't, the above BUG_ON() triggers.

On non-RT kernel, this usually happens, when I screw up CPU hotplug
code - e.g. enabling concurrency management when all workers are not
rebound to the CPU yet.

> Now in your code you have the comment:
> 
>  * X: During normal operation, modification requires gcwq->lock and
>  *    should be done only from local cpu.  Either disabling preemption
>  *    on local cpu or grabbing gcwq->lock is enough for read access.
>  *    If GCWQ_DISASSOCIATED is set, it's identical to L.
> 
> struct worker has flags marked with X.
> struct worker_pool has flags and idle_list marked with X.

So, the weird 'X' rule is there to guarantee that wq_worker_sleeping()
and try_to_wake_up() can peek the data fields necessary to perform
local wakeup (determining whether and who to wakeup and actuallying
doing it) while holding rq->lock.

> spin_locks in -rt do not disable preemption, nor do they disable irqs,
> but they do disable migration. If there's code that depends on the
> spin_lock disabling preemption, we need to either change the code to not
> require that, or explicitly disable preemption in the critical paths.
> Note, if we explicitly disable preemption, we can not call spin_locks
> within those locations as in -rt a spin_lock can block and schedule.

Maybe I'm confused but I can't really see how the above would be a
problem to workqueue in itself.  Both rq->lock and gcwq->lock are
irq-safe, so spin_lock() not disabling preemption shouldn't be a
problem.  Are CPU hotplug operations involved?

Thanks.

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 09:06 -0700, Tejun Heo wrote:
> Hello, Steven.
> 
> On Mon, Mar 18, 2013 at 10:36:23AM -0400, Steven Rostedt wrote:
> > kernel BUG at kernel/sched/core.c:1731!
> > invalid opcode: 0000 [#1] PREEMPT SMP 
> > CPU 5 
> > Pid: 16637, comm: kworker/5:0 Not tainted 3.6.11-rt30.25.el6rt.x86_64 #1 HP ProLiant DL580 G7
> ...
> > static void try_to_wake_up_local(struct task_struct *p)
> > {
> >         struct rq *rq = task_rq(p);
> > 
> >         BUG_ON(rq != this_rq());  <---- bug here
> 
> It's the local chain wake-up code used to main concurrency.  ie. when
> a worker bound to a CPU schedules out it kicks another worker to take
> its place (in concurrency level).

Yep, I got that much.

> 
> The function is called from inside __schedule() while holding rq->lock
> and requires that the target task is on the same rq as the one trying
> to wake it up.  When it isn't, the above BUG_ON() triggers.

Yeah, that was rather obvious too ;-)

> 
> On non-RT kernel, this usually happens, when I screw up CPU hotplug
> code - e.g. enabling concurrency management when all workers are not
> rebound to the CPU yet.
> 
> > Now in your code you have the comment:
> > 
> >  * X: During normal operation, modification requires gcwq->lock and
> >  *    should be done only from local cpu.  Either disabling preemption
> >  *    on local cpu or grabbing gcwq->lock is enough for read access.
> >  *    If GCWQ_DISASSOCIATED is set, it's identical to L.
> > 
> > struct worker has flags marked with X.
> > struct worker_pool has flags and idle_list marked with X.
> 
> So, the weird 'X' rule is there to guarantee that wq_worker_sleeping()
> and try_to_wake_up() can peek the data fields necessary to perform
> local wakeup (determining whether and who to wakeup and actuallying
> doing it) while holding rq->lock.
> 
> > spin_locks in -rt do not disable preemption, nor do they disable irqs,
> > but they do disable migration. If there's code that depends on the
> > spin_lock disabling preemption, we need to either change the code to not
> > require that, or explicitly disable preemption in the critical paths.
> > Note, if we explicitly disable preemption, we can not call spin_locks
> > within those locations as in -rt a spin_lock can block and schedule.
> 
> Maybe I'm confused but I can't really see how the above would be a
> problem to workqueue in itself.  Both rq->lock and gcwq->lock are
> irq-safe, so spin_lock() not disabling preemption shouldn't be a
> problem.  Are CPU hotplug operations involved?

No CPU hotplug is involved here. But I will note that gcwq->lock in -rt
is not irq -safe. That is, in rt the spin_lock_irq(&gcwq->lock) really
becomes a special "mutex_lock(&gcwq->lock)". Because, in -rt, interrupts
(except for the timer interrupt) are run as threads, and anything that
isn't marked as raw_spin_lock() turns into a mutex. I don't believe it's
safe to turn the gcwq->lock into a raw_spin_lock either, or at least not
short enough to hold it. Anything that holds a spin_lock() for more than
a microsecond is too much for a raw lock.

-- Steve

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 12:23 -0400, Steven Rostedt wrote:

> > Maybe I'm confused but I can't really see how the above would be a
> > problem to workqueue in itself.  Both rq->lock and gcwq->lock are
> > irq-safe, so spin_lock() not disabling preemption shouldn't be a
> > problem.  Are CPU hotplug operations involved?
> 
> No CPU hotplug is involved here. But I will note that gcwq->lock in -rt
> is not irq -safe. That is, in rt the spin_lock_irq(&gcwq->lock) really
> becomes a special "mutex_lock(&gcwq->lock)".

IOW, what can happen in -rt here is:

	spin_lock_irq(&gcwq->lock);
	[...]
	<interrupt>
		-> preempt_schedule();
		   schedule();
		   try_to_wake_up_local();

	[...]
	spin_unlock_irq(&gcwq->lock);

Again, with -rt, spin_lock_irq() does not prevent interrupts nor
preemption.

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

Hey, Steven.

On Mon, Mar 18, 2013 at 12:23:19PM -0400, Steven Rostedt wrote:
> > Maybe I'm confused but I can't really see how the above would be a
> > problem to workqueue in itself.  Both rq->lock and gcwq->lock are
> > irq-safe, so spin_lock() not disabling preemption shouldn't be a
> > problem.  Are CPU hotplug operations involved?
> 
> No CPU hotplug is involved here. But I will note that gcwq->lock in -rt
> is not irq -safe. That is, in rt the spin_lock_irq(&gcwq->lock) really
> becomes a special "mutex_lock(&gcwq->lock)". Because, in -rt, interrupts
> (except for the timer interrupt) are run as threads, and anything that
> isn't marked as raw_spin_lock() turns into a mutex. I don't believe it's
> safe to turn the gcwq->lock into a raw_spin_lock either, or at least not
> short enough to hold it. Anything that holds a spin_lock() for more than
> a microsecond is too much for a raw lock.

Does that mean that a task holding gcwq->lock may be preempted?  If
so, that sure could lead to weird problems.  Maybe gcwq->lock should
be marked non-preemptible somehow?

Thanks.

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 12:27 -0400, Steven Rostedt wrote:

> IOW, what can happen in -rt here is:
> 
> 	spin_lock_irq(&gcwq->lock);
> 	[...]
> 	<interrupt>
> 		-> preempt_schedule();
> 		   schedule();
> 		   try_to_wake_up_local();
> 
> 	[...]
> 	spin_unlock_irq(&gcwq->lock);
> 
> Again, with -rt, spin_lock_irq() does not prevent interrupts nor
> preemption.

If you happen to know the critical areas that require preemption to be
disabled for real, we can encapsulate them with:

	preempt_disable_rt();

	preempt_enable_rt();

These are currently only in the -rt patch, but it annotates locations
that require preemption to be disabled even when -rt converts spin_locks
into mutexes. These obviously can not contain spin_locks() as
spin_locks() can block and schedule out.

-- Steve

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 09:27 -0700, Tejun Heo wrote:

> Does that mean that a task holding gcwq->lock may be preempted?  If
> so, that sure could lead to weird problems.  Maybe gcwq->lock should
> be marked non-preemptible somehow?

If the gcwq->lock is never held for a long time (really, more than a
microsecond on today's processors is considered a long time), and it
does not nest any other spin_locks (raw locks are OK, like the rq lock).
Then we could mark the gcwq->lock as raw as well.

This would require the struct global_cwq lock to have:

	raw_spinlock_t		lock;

and then you would need to do:

s/spin_/raw_spin/ for all gcwq->lock usages.

But, I'm worried about the loops that are done while holding this lock.
Just looking at is_chained_work() that does for_each_busy_worker(), how
big can that list be? If it's bound by # of CPUs then that may be fine,
but if it can be as big as the # of workers assigned, with no real
limit, then its not fine, because that creates an unbound (non
deterministic) latency.

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

Hello, Steven.

On Mon, Mar 18, 2013 at 12:30:43PM -0400, Steven Rostedt wrote:
> If you happen to know the critical areas that require preemption to be
> disabled for real, we can encapsulate them with:
> 
> 	preempt_disable_rt();
> 
> 	preempt_enable_rt();
> 
> These are currently only in the -rt patch, but it annotates locations
> that require preemption to be disabled even when -rt converts spin_locks
> into mutexes. These obviously can not contain spin_locks() as
> spin_locks() can block and schedule out.

Making gcwq locks disable preemption would be much safer / easier, but
if that's not desirable, anything touching gcwq->idle_list would be a
good place to start - worker_enter_idle() and worker_leave_idle().
Hmmm... ignoring CPU hotplug, I think those two might just do it.
Give it a try?  How reproducible is the problem?

Thanks.

-- 
tejun

Re: workqueue code needing preemption disabled

[Tejun Heo]

On Mon, Mar 18, 2013 at 12:41:23PM -0400, Steven Rostedt wrote:
> But, I'm worried about the loops that are done while holding this lock.
> Just looking at is_chained_work() that does for_each_busy_worker(), how
> big can that list be? If it's bound by # of CPUs then that may be fine,
> but if it can be as big as the # of workers assigned, with no real
> limit, then its not fine, because that creates an unbound (non
> deterministic) latency.

In most paths, gcwq->lock shouldn't be held for too long but yes there
are cold paths which just do things without thinking about latency
issues.  is_chained_work() can definitely take pretty long time (note
that it got reimplemented in the current devel branch and the loop is
gone).

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 09:43 -0700, Tejun Heo wrote:

> Making gcwq locks disable preemption would be much safer / easier, but
> if that's not desirable, anything touching gcwq->idle_list would be a
> good place to start - worker_enter_idle() and worker_leave_idle().
> Hmmm... ignoring CPU hotplug, I think those two might just do it.
> Give it a try?  How reproducible is the problem?

Not very :-( I triggered it twice on a 40 CPU box. It can go
approximately 1 month before it triggers. And the box we are testing on
is currently a loaner, and we have it on extension right now. Which
means we wont have it much longer.

But perhaps that's the place to fix things.

Thanks,

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

On Mon, Mar 18, 2013 at 01:08:07PM -0400, Steven Rostedt wrote:
> On Mon, 2013-03-18 at 09:43 -0700, Tejun Heo wrote:
> 
> > Making gcwq locks disable preemption would be much safer / easier, but
> > if that's not desirable, anything touching gcwq->idle_list would be a
> > good place to start - worker_enter_idle() and worker_leave_idle().
> > Hmmm... ignoring CPU hotplug, I think those two might just do it.
> > Give it a try?  How reproducible is the problem?
> 
> Not very :-( I triggered it twice on a 40 CPU box. It can go
> approximately 1 month before it triggers. And the box we are testing on
> is currently a loaner, and we have it on extension right now. Which
> means we wont have it much longer.
> 
> But perhaps that's the place to fix things.

I've been thinking about it and AFAICS the only way that BUG_ON()
could trigger from preemption is if preemption happens while the
idle_list head is becoming or stopping being empty.
ie. pool->worklist is half updated so list_empty() isn't true but the
first next entry is already pointing back to itself.  If there's a
crashdump, it shouldn't be too difficult to verify and wrapping the
above two functions should resolve it.

Thanks.

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 09:43 -0700, Tejun Heo wrote:
> Hello, Steven.
> 
> On Mon, Mar 18, 2013 at 12:30:43PM -0400, Steven Rostedt wrote:
> > If you happen to know the critical areas that require preemption to be
> > disabled for real, we can encapsulate them with:
> > 
> > 	preempt_disable_rt();
> > 
> > 	preempt_enable_rt();
> > 
> > These are currently only in the -rt patch, but it annotates locations
> > that require preemption to be disabled even when -rt converts spin_locks
> > into mutexes. These obviously can not contain spin_locks() as
> > spin_locks() can block and schedule out.
> 
> Making gcwq locks disable preemption would be much safer / easier, but
> if that's not desirable, anything touching gcwq->idle_list would be a
> good place to start - worker_enter_idle() and worker_leave_idle().
> Hmmm... ignoring CPU hotplug, I think those two might just do it.
> Give it a try?  How reproducible is the problem?
> 

Hmm, the issue is that a "use to be" idle thread got migrated, and is
now being woken up by another worker. What can cause an established
worker to migrate without HOTPLUG being active?

Thomas,

I'm thinking that we should also modify the scheduler for -rt:

        if (prev->state && !(preempt_count() & PREEMPT_ACTIVE)) {
                if (unlikely(signal_pending_state(prev->state, prev))) {
                        prev->state = TASK_RUNNING;
                } else {
                        deactivate_task(rq, prev, DEQUEUE_SLEEP);
                        prev->on_rq = 0;

                        /*
                         * If a worker went to sleep, notify and ask workqueue
                         * whether it wants to wake up a task to maintain
                         * concurrency.
                         */
                        if (prev->flags & PF_WQ_WORKER) {
                                struct task_struct *to_wakeup;

                                to_wakeup = wq_worker_sleeping(prev, cpu);
                                if (to_wakeup)
                                        try_to_wake_up_local(to_wakeup);
                        }
                }
                switch_count = &prev->nvcsw;
        }


The code calls another worker when the previous worker sleeps,
presumably on IO or something. But in -rt, it could be sleeping on the
gcwq->lock itself, and this could cause many more wakeups. Easily where
the task being woken up will try to grab the same lock and sleep again.

Perhaps we should check:

	if (prev->flags & PF_WQ_WORKER && !prev->saved_state)

To keep the worker thread from waking up other workers just because it
blocked on a sleeping spin lock.

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

On Mon, Mar 18, 2013 at 02:23:56PM -0400, Steven Rostedt wrote:
> On Mon, 2013-03-18 at 09:43 -0700, Tejun Heo wrote:
> > Hello, Steven.
> > 
> > On Mon, Mar 18, 2013 at 12:30:43PM -0400, Steven Rostedt wrote:
> > > If you happen to know the critical areas that require preemption to be
> > > disabled for real, we can encapsulate them with:
> > > 
> > > 	preempt_disable_rt();
> > > 
> > > 	preempt_enable_rt();
> > > 
> > > These are currently only in the -rt patch, but it annotates locations
> > > that require preemption to be disabled even when -rt converts spin_locks
> > > into mutexes. These obviously can not contain spin_locks() as
> > > spin_locks() can block and schedule out.
> > 
> > Making gcwq locks disable preemption would be much safer / easier, but
> > if that's not desirable, anything touching gcwq->idle_list would be a
> > good place to start - worker_enter_idle() and worker_leave_idle().
> > Hmmm... ignoring CPU hotplug, I think those two might just do it.
> > Give it a try?  How reproducible is the problem?
> > 
> 
> Hmm, the issue is that a "use to be" idle thread got migrated, and is
> now being woken up by another worker. What can cause an established
> worker to migrate without HOTPLUG being active?

It doesn't.  I think it's trying to wakeup the idle_list head.

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 11:26 -0700, Tejun Heo wrote:

> > Hmm, the issue is that a "use to be" idle thread got migrated, and is
> > now being woken up by another worker. What can cause an established
> > worker to migrate without HOTPLUG being active?
> 
> It doesn't.  I think it's trying to wakeup the idle_list head.
> 

That definitely makes sense. I'll go and write up a patch.

Thanks!!!

-- Steve

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 11:21 -0700, Tejun Heo wrote:

> I've been thinking about it and AFAICS the only way that BUG_ON()
> could trigger from preemption is if preemption happens while the
> idle_list head is becoming or stopping being empty.
> ie. pool->worklist is half updated so list_empty() isn't true but the
> first next entry is already pointing back to itself.  If there's a
> crashdump, it shouldn't be too difficult to verify and wrapping the
> above two functions should resolve it.

I like the theory, but it has one flaw. I agree that the update should
be wrapped in preempt_disable() but since this bug happens on the same
CPU, the state of the list will be the same when it was preempted to
when it bugged. That said:

static inline int list_empty(const struct list_head *head)
{
	return head->next == head;
}

That means when the task was preempted, head->next will either be
pointing to the next element or back to the list head. Which means if we
get preempted while updating the list, it will either see the head->next
== head or head->next == the next element.

first_worker() returns list_first_entry() which returns head->next. I
can't see how it would see the list_head and have list_empty() return
false.

-- Steve

Re: workqueue code needing preemption disabled

[Tejun Heo]

On Mon, Mar 18, 2013 at 02:57:30PM -0400, Steven Rostedt wrote:
> I like the theory, but it has one flaw. I agree that the update should
> be wrapped in preempt_disable() but since this bug happens on the same
> CPU, the state of the list will be the same when it was preempted to
> when it bugged. That said:
> 
> static inline int list_empty(const struct list_head *head)
> {
> 	return head->next == head;
> }

Dang... right.  For some reason, I was thinking it was doing
head->next == head->prev.

> That means when the task was preempted, head->next will either be
> pointing to the next element or back to the list head. Which means if we
> get preempted while updating the list, it will either see the head->next
> == head or head->next == the next element.
> 
> first_worker() returns list_first_entry() which returns head->next. I
> can't see how it would see the list_head and have list_empty() return
> false.

Me neither.  Unfortunately, I'm out of ideas at the moment.
Hmm... last year, there was a similar issue, I think it was in AMD
cpufreq, which was caused by work function doing
set_cpus_allowed_ptr(), so the idle worker was on the correct CPU but
the one issuing local wake up was on the wrong one.  It could be that
there's another such usage in kernle which doesn't trigger easily w/o
RT.  As preemption doesn't trigger concurrency management wakeup, as
long as such user doesn't do something explicitly blocking, upstream
would be fine as long as it restores affinity before finishing but in
RT spinlocks become mutexes and can trigger local wakeups, so...

Anyways, having a crashdump would go a long way towards identifying
what's going on.  All we need to know are the work function which was
being executed, whether the worker was on the right CPU and which
worker it was trying to wake up.

-- 
tejun

Re: workqueue code needing preemption disabled

[Steven Rostedt]

On Mon, 2013-03-18 at 12:06 -0700, Tejun Heo wrote:

> Me neither.  Unfortunately, I'm out of ideas at the moment.
> Hmm... last year, there was a similar issue, I think it was in AMD
> cpufreq, which was caused by work function doing
> set_cpus_allowed_ptr(), so the idle worker was on the correct CPU but
> the one issuing local wake up was on the wrong one. 

I should also tell you that -rt is currently based on 3.6.11. Did that
bug get passed on to stable? If not, we could be hitting that same bug
too. Except this is running on Intel. :-/


>  It could be that
> there's another such usage in kernle which doesn't trigger easily w/o
> RT.  As preemption doesn't trigger concurrency management wakeup, as
> long as such user doesn't do something explicitly blocking, upstream
> would be fine as long as it restores affinity before finishing but in
> RT spinlocks become mutexes and can trigger local wakeups, so...

And these wakeups can be triggered by blocking on the gcwq->lock as
well, where it probably happens more often on -rt than mainline.

> 
> Anyways, having a crashdump would go a long way towards identifying
> what's going on.  All we need to know are the work function which was
> being executed, whether the worker was on the right CPU and which
> worker it was trying to wake up.
> 

OK, I'll have my box set up. But I doubt this bug will even trigger
again before I have to return it :-(

-- Steve