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

From: Borislav Petkov
Date: Mon Dec 24 2012 - 13:56:40 EST


On Mon, Dec 24, 2012 at 10:33:34AM -0800, Tejun Heo wrote:
> If one looks at something happening in a path as cold as memory
> hotplug and thinks about optimizing a coupld memory accesses, the
> person's priorities need serious reconsideration. I think approaches
> like that are actively harmful. They lead to unnecessary (and thus
> difficult to comprehend) convolutions which don't really help anything
> while deteoriorating code base.

Maybe I'm not expressing myself clearly enough so let me try again: all
I'm saying is, I don't like the idea of needlessly toggling interrupts
and having a locked operation just to understand that there's work
pending.

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.

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
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.

> 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.

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
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/