[PATCH 2/2][RESEND] slab: avoid unnecessary touching of a partial slab

[PATCH 2/2][RESEND] slab: avoid unnecessary touching of a partial slab

[Zhao Jin]

In cache_alloc_refill(), after refilling from a partial slab, if the
slab remains partial, it would be deleted from and then added again to
the partial list. As the slab is the first element in the list before
deletion, such behavior has no effect. This patch avoids touching the
slab in this case.

Signed-off-by: Zhao Jin <cronozhj@gmail.com>
---
 mm/slab.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 82e0a0e..2d7a0f4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3121,11 +3121,10 @@ retry:
 		check_slabp(cachep, slabp);
 
 		/* move slabp to correct slabp list: */
-		list_del(&slabp->list);
 		if (slabp->free == BUFCTL_END)
-			list_add(&slabp->list, &l3->slabs_full);
-		else
-			list_add(&slabp->list, &l3->slabs_partial);
+			list_move(entry, &l3->slabs_full);
+		else if (entry != l3->slabs_partial.next)
+			list_move(entry, &l3->slabs_partial);
 	}
 
 must_grow:
-- 
1.7.4.1

Re: [PATCH 2/2][RESEND] slab: avoid unnecessary touching of a partial slab

[Pekka Enberg]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1135 bytes --]

[ Replying to an old patch that I seem to have missed. ]

On Sat, 27 Aug 2011, Zhao Jin wrote:
> In cache_alloc_refill(), after refilling from a partial slab, if the
> slab remains partial, it would be deleted from and then added again to
> the partial list. As the slab is the first element in the list before
> deletion, such behavior has no effect. This patch avoids touching the
> slab in this case.
>
> Signed-off-by: Zhao Jin <cronozhj@gmail.com>

Looks good to me, Christoph, David?

> ---
> mm/slab.c |    7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 82e0a0e..2d7a0f4 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3121,11 +3121,10 @@ retry:
> 		check_slabp(cachep, slabp);
>
> 		/* move slabp to correct slabp list: */
> -		list_del(&slabp->list);
> 		if (slabp->free == BUFCTL_END)
> -			list_add(&slabp->list, &l3->slabs_full);
> -		else
> -			list_add(&slabp->list, &l3->slabs_partial);
> +			list_move(entry, &l3->slabs_full);
> +		else if (entry != l3->slabs_partial.next)
> +			list_move(entry, &l3->slabs_partial);
> 	}
>
> must_grow:
> -- 
> 1.7.4.1
>
>

Re: [PATCH 2/2][RESEND] slab: avoid unnecessary touching of a partial slab

[Christoph Lameter]

On Mon, 23 Jan 2012, Pekka Enberg wrote:

> > diff --git a/mm/slab.c b/mm/slab.c
> > index 82e0a0e..2d7a0f4 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -3121,11 +3121,10 @@ retry:
> > 		check_slabp(cachep, slabp);
> >
> > 		/* move slabp to correct slabp list: */
> > -		list_del(&slabp->list);
> > 		if (slabp->free == BUFCTL_END)
> > -			list_add(&slabp->list, &l3->slabs_full);
> > -		else
> > -			list_add(&slabp->list, &l3->slabs_partial);
> > +			list_move(entry, &l3->slabs_full);
> > +		else if (entry != l3->slabs_partial.next)

Why is the check for entry necessary? We are holding a lock that
prevents modifications to the list after all.

Just a list_move when the partial slab is drained seems to be sufficient.