Re: [PATCH 0/2 x86] fix some page faults in nmi if kmemcheck is enabled

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Thu, 2012-02-23 at 17:53 +0800, Li Zhong wrote:

> I will think further about it, and would appreciate it if you could give
> some good ideas. 

*sigh*.. or you could do your own damn work..

> > > 2. If CONFIG_KMEMCHECK is enabled, the pages allocated through slab will
> > > be marked as non-present, to capture uninitialized memory access. More
> > > information in Documentation/kmemcheck.txt .
> > 
> > So then kmemcheck is buggy, since the nmiaction structure is initialized
> > in register_nmi_handler(), so it should most definitely not be marked
> > non-present.
> > 
> 
> I'm not sure whether I understand it correctly. Do you mean that
> nmiaction is initialized in register_nmi_handler(), which indicates it
> will be used in nmi, so it shouldn't be marked non-present?

No, you said that it marks memory non-present to detect uninitialized
stuff, but since it is initialized, it shouldn't then be non-present,
right?

> But for kmemcheck, why need it know the information that page fault is
> not allowed in nmi? 

Uh, what?

> > > 3. From the log, there are some memories accessed in nmi, which are in
> > > pages marked as non-present by kmemcheck, as they are allocated by
> > > something like kmalloc(). 
> > 
> > So figure out why and fix that instead of writing ugly ass patches that
> > seemingly work around the problem without actually thinking about it.
> > 
> 
> I think the reason is that kmalloc ( or kzalloc ... ) uses malloc_sizes
> slab caches to allocate memory. The malloc_sizes slab caches is set up
> without SLAB_NOTRACK flag, then kmemcheck marks the pages non-present to
> do its check in page fault handling code. I think we shouldn't disable
> kmemechek for the general malloc_sizes caches. 

Nobody said you should.. there's plenty of solutions that aren't ass
backward stupid nor as ugly.

First you need to figure out why the page is marked non-present since
the data structure is initialized (I've got a fair idea why), then look
if you can tell kmemcheck not to be silly like that.

Alternatively you can change the nmi stuff to use static storage like
other notifiers (see notifier_block).

What you don't ever do is write alternative code paths that are never
ever used except for debugging, that is just asking for problems.