Re: [patch] #ifdef very expensive debug check in page fault path

[Nick Piggin <npiggin@suse.de>]

On Wed, Jan 16, 2008 at 07:01:28PM +0100, Carsten Otte wrote:
> This patch puts #ifdef CONFIG_DEBUG_VM around a check in vm_normal_page
> that verifies that a pfn is valid. This patch increases performance of
> the page fault microbenchmark in lmbench by 13% and overall dbench
> performance by 7% on s390x.  pfn_valid() is an expensive operation on
> s390 that needs a high double digit amount of CPU cycles.
> Nick Piggin suggested that pfn_valid() involves an array lookup on
> systems with sparsemem, and therefore is an expensive operation there
> too.
> The check looks like a clear debug thing to me, it should never trigger
> on regular kernels. And if a pte is created for an invalid pfn, we'll
> find out once the memory gets accessed later on anyway. Please consider
> inclusion of this patch into mm.
> 
> Signed-off-by: Carsten Otte <cotte@de.ibm.com>

Wow, that's a big performance hit for a few instructions ;)
I haven't seen it to be quite so expensive on x86, but it definitely is
not zero cost, especially with NUMA kernels. Thanks for getting those
numbers.

I posted a version which got rid of that big comment block too, but
no feedback as yet.

http://marc.info/?l=linux-arch&m=120046068604222&w=2

The one actual upside of this code is that if there is pte corruption
detected, the failure should be a little more graceful... but there
is also lots of pte corruption that could go undetected and cause much
worse problems anyway so I don't feel it is something that needs to
be turned on in production kernels. It could be a good debugging aid
to mm/ or device driver writers though.

Anyway, again I've cc'ed Hugh, because he nacked this same patch a
while back. So let's try to get him on board before merging anything.

If we get an ack, why not send this upstream for 2.6.24? Those s390
numbers are pretty insane.

> --- 
> Index: linux-2.6/mm/memory.c
> ===================================================================
> --- linux-2.6.orig/mm/memory.c
> +++ linux-2.6/mm/memory.c
> @@ -392,6 +392,7 @@ struct page *vm_normal_page(struct vm_ar
>  			return NULL;
>  	}
>  
> +#ifdef CONFIG_DEBUG_VM
>  	/*
>  	 * Add some anal sanity checks for now. Eventually,
>  	 * we should just do "return pfn_to_page(pfn)", but
> @@ -402,6 +403,7 @@ struct page *vm_normal_page(struct vm_ar
>  		print_bad_pte(vma, pte, addr);
>  		return NULL;
>  	}
> +#endif
>  
>  	/*
>  	 * NOTE! We still have PageReserved() pages in the page 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>