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

[Ingo Molnar <mingo@kernel.org>]

* Christoph Lameter <cl@linux.com> 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