Re: [slubllv6 12/17] slub: Avoid disabling interrupts in freeslowpath

[David Rientjes]

On Thu, 26 May 2011, Christoph Lameter wrote:

> Disabling interrupts can be avoided now. However, list operation still require
> disabling interrupts since allocations can occur from interrupt
> contexts and there is no way to perform atomic list operations.
> 
> The acquition of the list_lock therefore has to disable interrupts as well.
> 
> Dropping interrupt handling significantly simplifies the slowpath.
> 
> Signed-off-by: Christoph Lameter <cl@xxxxxxxxx>
> 
> ---
>  mm/slub.c |   16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c	2011-05-24 09:40:57.434875035 -0500
> +++ linux-2.6/mm/slub.c	2011-05-24 09:41:00.194875015 -0500
> @@ -2183,11 +2183,10 @@ static void __slab_free(struct kmem_cach
>  	struct kmem_cache_node *n = NULL;
>  	unsigned long flags;
>  
> -	local_irq_save(flags);
>  	stat(s, FREE_SLOWPATH);
>  
>  	if (kmem_cache_debug(s) && !free_debug_processing(s, page, x, addr))
> -		goto out_unlock;
> +		return;
>  
>  	do {
>  		prior = page->freelist;
> @@ -2206,7 +2205,7 @@ static void __slab_free(struct kmem_cach
>  			 * Otherwise the list_lock will synchronize with
>  			 * other processors updating the list of slabs.
>  			 */
> -                        spin_lock(&n->list_lock);
> +                        spin_lock_irqsave(&n->list_lock, flags);
>  		}
>  		inuse = new.inuse;
>  
> @@ -2222,7 +2221,7 @@ static void __slab_free(struct kmem_cach
>  		 */
>                  if (was_frozen)
>                          stat(s, FREE_FROZEN);
> -                goto out_unlock;
> +                return;
>          }
>  
>  	/*
> @@ -2245,11 +2244,7 @@ static void __slab_free(struct kmem_cach
>  			stat(s, FREE_ADD_PARTIAL);
>  		}
>  	}
> -
> -	spin_unlock(&n->list_lock);
> -
> -out_unlock:
> -	local_irq_restore(flags);
> +	spin_unlock_irqrestore(&n->list_lock, flags);

We know that flags will be initialized at this point in the code, but the 
compiler doesn't and it emits this:

	mm/slub.c: In function â__slab_freeâ:
	mm/slub.c:2198: warning: âflagsâ may be used uninitialized in this function

so it'll need uninitialized_var().

>  	return;
>  
>  slab_empty:
> @@ -2261,8 +2256,7 @@ slab_empty:
>  		stat(s, FREE_REMOVE_PARTIAL);
>  	}
>  
> -	spin_unlock(&n->list_lock);
> -	local_irq_restore(flags);
> +	spin_unlock_irqrestore(&n->list_lock, flags);
>  	stat(s, FREE_SLAB);
>  	discard_slab(s, page);
>  }
> 
>