Messages in this thread | | | Date | Sat, 22 Dec 2012 12:09:29 +0100 | From | Borislav Petkov <> | Subject | Re: [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. --
| |