lkml.org 
[lkml]   [2012]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 25/25] ipc: don't use [delayed_]work_pending()
On Fri, Dec 21, 2012 at 06:22:10PM -0800, Tejun Heo wrote:
> Hello, Andrew.
>
> On Fri, Dec 21, 2012 at 06:15:23PM -0800, Andrew Morton wrote:
> > On Fri, 21 Dec 2012 17:57:15 -0800 Tejun Heo <tj@kernel.org> wrote:
> >
> > > There's no need to test whether a (delayed) work item in pending
> > > before queueing, flushing or cancelling it. Most uses are unnecessary
> > > and quite a few of them are buggy.
> >
> > > - if (!work_pending(&ipc_memory_wq))
> > > - schedule_work(&ipc_memory_wq);
> > > + schedule_work(&ipc_memory_wq);
> >
> > Well, the new code is a ton slower than the old code if the work is
> > frequently pending, so some care is needed with such a conversion.
>
> Yeah, I mentioned it in the head message. it comes down to
> test_and_set_bit() vs. test_bit() and none of the current users seems
> to be hot enough for that to matter at all.
>
> In very hot paths, such optimization *could* be valid. The problem is
> that [delayed_]work_pending() seem to be abused much more than they
> are put to any actual usefulness. Maybe we should rename them to
> something really ugly. I don't know.

Hmm, we're also disabling local interrupts for no reason, if there's no
work pending (this is queue_work_on()):

2d1a: 9c pushfq
2d1b: 41 5c pop %r12
2d1d: fa cli
2d1e: e8 00 00 00 00 callq 2d23 <queue_work_on+0x33>
2d23: f0 0f ba 2b 00 lock btsl $0x0,(%rbx)

so there's IRQ disable + locked operation in schedule_work vs a simple
test_bit which doesn't even require the LOCK prefix.

Now you say those paths are not fast paths, but the reverse of
this optimization is also true: what happens if people start using
schedule_work() in fast paths without checking whether work is pending?
A useless IRQ disable + locked operation + IRQ enable.

I don't know but this could hurt in some situations, I'm thinking of RT
folk especially here.

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--


\
 
 \ /
  Last update: 2012-12-22 12:21    [W:0.489 / U:3.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site