[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150114100102.55c78715@gandalf.local.home>
Date: Wed, 14 Jan 2015 10:01:02 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Paul Gortmaker <paul.gortmaker@...driver.com>,
linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH 3/7] wait.[ch]: Introduce the simple waitqueue (swait)
implementation
On Wed, 14 Jan 2015 15:31:02 +0100
Peter Zijlstra <peterz@...radead.org> wrote:
> On Wed, Jan 14, 2015 at 09:18:49AM -0500, Steven Rostedt wrote:
> > > +/*
> > > + * Does not allow usage from IRQ disabled, since we must be able to
> > > + * release IRQs to guarantee bounded hold time.
> >
> > Actually we (-rt people) don't care if it's done with irq disabled, as
> > long is it was disabled via spin_lock_irq*() and not
> > raw_spin_lock_irq() or local_irq_save/disable().
> >
> > Because in that case, in -rt, irqs would not be disabled. We could add a
> > check in -rt that makes sure that's the case. I would think this is OK
> > to call from an interrupt handler as long as it is a thread in -rt.
> >
> > But, we can keep this restriction for now and change it if we find that
> > the restriction is preventing us from using it someplace where its
> > needed for -rt.
>
> One of the reasons for the swait thing is to be able to use
> raw_spinlock_t, and iirc raw_spin_lock_irq() will still disable IRQs
> even on RT.
Right, but that's in the internals of swait. The comment is saying that
swait can't be called with irqs disabled. non -rt shouldn't care, and
in -rt, we only care if it was called with real interrupts disabled.
>
> > > + */
> > > +void __swake_up_all(struct swait_queue_head *q, unsigned int state)
> > > +{
> > > + struct swait_queue *curr, *next;
> > > + LIST_HEAD(tmp);
> > > +
> > > + __swait_wakeup_debug(q, state);
> > > +
> > > + if (!swait_active(q))
> > > + return;
> > > +
> > > + raw_spin_lock_irq(&q->lock);
> > > + list_splice_init(&q->task_list, &tmp);
> > > + while (!list_empty(&tmp)) {
> > > + curr = list_first_entry(&tmp, typeof(curr), task_list);
> > > +
> > > + wake_up_state(curr->task, state);
> > > + list_del_init(&curr->task_list);
> >
> > What happens if curr->task does not match state? We just removed it
> > from the list and never woke it up, didn't we?
>
> Then you've messed up. One should not mix and match states with swait
> queues, there's some debug code that tries to make sure you don't do
> this.
Then why even bother with the wake_up_state, and not just call
wake_up_process()?
I bad wakeup is easier to debug than no wakeup.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists