Re: [PATCH 3/7] slub: Do not use c->page on free

[Jesper Dangaard Brouer]

On Wed, 10 Dec 2014 11:37:56 -0600 (CST) Christoph Lameter <cl@xxxxxxxxx> wrote:

[...]
> 
> There were some other issues so its now:
> 
> 
> Subject: slub: Do not use c->page on free
> 
> Avoid using the page struct address on free by just doing an
> address comparison. That is easily doable now that the page address
> is available in the page struct and we already have the page struct
> address of the object to be freed calculated.
> 
> Reviewed-by: Pekka Enberg <penberg@xxxxxxxxxx>
> Signed-off-by: Christoph Lameter <cl@xxxxxxxxx>
> 
> Index: linux/mm/slub.c
> ===================================================================
> --- linux.orig/mm/slub.c	2014-12-10 11:35:32.538563734 -0600
> +++ linux/mm/slub.c	2014-12-10 11:36:39.032447807 -0600
> @@ -2625,6 +2625,17 @@ slab_empty:
>  	discard_slab(s, page);
>  }
> 
> +static bool is_pointer_to_page(struct page *page, void *p)
> +{
> +	long d = p - page->address;
> +
> +	/*
> +	 * Do a comparison for a MAX_ORDER page first before using
> +	 * compound_order() to determine the actual page size.
> +	 */
> +	return d >= 0 && d < (1 << MAX_ORDER) && d < (compound_order(page) << PAGE_SHIFT);
> +}

My current compiler (gcc 4.9.1), choose not to inline is_pointer_to_page().

 (perf record of [1])
 Samples: 8K of event 'cycles', Event count (approx.): 5737618489
 +   46.13%  modprobe  [kernel.kallsyms]  [k] kmem_cache_free
 +   33.02%  modprobe  [kernel.kallsyms]  [k] kmem_cache_alloc
 +   16.14%  modprobe  [kernel.kallsyms]  [k] is_pointer_to_page

If I explicitly add "inline", then it gets inlined, and performance is good again.

Test[1] cost of kmem_cache_alloc+free:
 * baseline: 47 cycles(tsc) 19.032 ns  (net-next without patchset)
 * patchset: 50 cycles(tsc) 20.028 ns
 * inline  : 45 cycles(tsc) 18.135 ns  (inlined is_pointer_to_page())


>  /*
>   * Fastpath with forced inlining to produce a kfree and kmem_cache_free that
>   * can perform fastpath freeing without additional function calls.
> @@ -2658,7 +2669,7 @@ redo:
>  	tid = c->tid;
>  	preempt_enable();
> 
> -	if (likely(page == c->page)) {
> +	if (likely(is_pointer_to_page(page, c->freelist))) {
>  		set_freepointer(s, object, c->freelist);
> 
>  		if (unlikely(!this_cpu_cmpxchg_double(


[1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_kmem_cache1.c

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer
--
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/