Re: [PATCH 25/25] ipc: don't use [delayed_]work_pending()

[Tejun Heo]

Hello, Borislav.

On Mon, Dec 24, 2012 at 07:55:55PM +0100, Borislav Petkov wrote:
> Basically, with the amount of bloat we're adding to the kernel, the
> couple of instructions we're adding here and there and think they won't
> harm us, tends to crop up with time until we're too damn slow to do
> anything.

Yeah, I think we need them for the operations to hold w/o outer
synchronization.  If you can think of ways to optimize them, please be
my guest.

> Now, you're saying that optimizing cold paths is something that calls
> for serious reconsideration of a person's priorities, and I'm saying
> just don't do anything in code that's not necessary. Fullstop. If this

Not doing anything that's not necessary is an extreme and not
necessary what you want in cold paths.  If doing something extra makes
your code much simpler and more maintainable and the path is cold
enough for the extra to not matter, then that's the right trade-off.

> is leading to convolutions, then the design is suboptimal and needs
> adjustment. If [delayed_]work_pending is being abused, then fix it or
> add a new primitive which only checks for pending work and doesn't
> unconditionally toggle interrupts.

"only checks for pending work" in a way which is race-free and usable
from any context is a tricky thing to do.

> > I don't think we have cases where this actually matters but it could
> > be that we can add work_pending() tests to queue_work_on(). I *think*
> > that shouldn't break work scheduling semantics. Not completely sure
> > tho. Need to think about it more.
> 
> Yes, something like that.

And that's broken.  It seems trivial but it really isn't and trying to
optimize things like that in cold paths is just a bad idea.  Not
enough people will pay attention to them and they will stay subtly
broken for a very long time.  So, having "not doing anything in code
which isn't necessary in code" as priority in cold paths is likely to
hurt you.  A better one would be "doing straight-forward and simple
thing with acceptable overhead".

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/