Re: [pchecks v2 2/2] percpu: Add preemption checks to __this_cpu ops

From: Ingo Molnar
Date: Thu Oct 03 2013 - 11:35:31 EST



* Christoph Lameter <cl@xxxxxxxxx> wrote:

> On Thu, 3 Oct 2013, Ingo Molnar wrote:
>
> > It was important to me and other maintainers as well back then and today
> > as well, as me and others complained about it out numerous times.
>
> Yes there were some complaints and in discussions about what to do. I
> suggested how this could be addressed. But no patches showed up [...]

_You_ added the facility with broken (== non-existent) preemption
debugging for __this_cpu ops, _you_ caused Peter Zijstra and others to
waste time due to you ignoring those requests to add debugging. Everyone
rightfully expected _you_ to fix the problem you introduced.

And now you blame the victims of your sloppiness, that they should have
fixed the problem you introduced?

> [...] and there were always other more pressing things. Especially since
> this is a minor issue related to CONFIG_PREEMPT which seems to be not in
> use in the kernels that I see in HPC, FIS and the industry at large.

People wasting time and the kernel becoming less robust is not a minor
issue at all.

> > I can fix that omission easily: consider all your __this_cpu* patches
> > NAK-ed by me until the (trivial) preemption debug checks are upstream
> > worthy:
> >
> > - tested
> > - complete
> > - don't produce false warnings when enabled.
>
> Not sure what tests you will like to see run and if it is even possible
> to test all possible kernel runtime configurations. You seem to have
> some setup to do some testing along these lines I believe?

As a starting point it would be fine if you tested it on your own systems
with all relevant debugging enabled...

> These two patches will allow this testing to be done. And I do not see
> any mention of technical issues with the code. [...]

Here's the list of open technical problems:

- Lack of testing - you have not stated it whether any warnings trigger
with those two patches applied and debugging enabled, on your systems.

- I pointed out in detail how your last submission was broken in several
places which show lack of time and care on the patch series.

- Your statement in the discussion that warnings will trigger with the
debug option enabled points to an obvious technical problem as well -
all warnings known to trigger by you should be fixed by you, as part of
the series.

Please resolve these technical problems and resend a clean, tested,
working series.

Until all the problems are addressed my NAK stands and I suspect Peter
Zijlstra's as well.

Thanks,

Ingo
--
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/