Hello, Steven.
On Tue, Jun 25, 2013 at 07:19:04PM -0400, Steven Rostedt wrote:
> Why is that silly? It actually makes plenty of sense. Now if
> preempt_disable/enable was nested in spin_lock_irq_save/restore() now
> that would be pretty silly.
If you know you're gonna be disabling irq pretty soon, you don't need
to do that, so...
> Just looking at the first part of that function:
>
> local_irq_disable();
> pool = get_work_pool(work);
> if (!pool) {
> local_irq_enable();
> return false;
> }
>
> On the case of poll == NULL, we disabled interrupts for no reason.
It's much more likely that get_work_pool() there returns !NULL. I
didn't think it'd matter enough to put likely(). Sure, it's nice to
not disable interrupts but really, in upstream, I don't think the
above matters in the upstream kernel. The extra coverage is at the
worst idr_find() into single level idr.
> It may take a bit of understanding the code before I send a patch. But
> I'll start looking into it.
Wrapping from local_irq_disable() to spin_unlock_irq() with RCU sched
read lock/unlock should do, I think.
Thanks!
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
