[04/12] percpu: manage chunks based on contig_bits instead of free_bytes

Commit Message

Dennis Zhou Feb. 28, 2019, 2:18 a.m. UTC

When a chunk becomes fragmented, it can end up having a large number of
small allocation areas free. The free_bytes sorting of chunks leads to
unnecessary checking of chunks that cannot satisfy the allocation.
Switch to contig_bits sorting to prevent scanning chunks that may not be
able to service the allocation request.

Signed-off-by: Dennis Zhou <[e-mail]>
---
 mm/percpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peng Fan March 2, 2019, 1:48 p.m. UTC | #1

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On
> Behalf Of Dennis Zhou
> Sent: 2019年2月28日 10:19
> To: Dennis Zhou <[e-mail]>; Tejun Heo <[e-mail]>; Christoph
> Lameter <[e-mail]>
> Cc: Vlad Buslov <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]
> Subject: [PATCH 04/12] percpu: manage chunks based on contig_bits instead
> of free_bytes
> 
> When a chunk becomes fragmented, it can end up having a large number of
> small allocation areas free. The free_bytes sorting of chunks leads to
> unnecessary checking of chunks that cannot satisfy the allocation.
> Switch to contig_bits sorting to prevent scanning chunks that may not be able
> to service the allocation request.
> 
> Signed-off-by: Dennis Zhou <[e-mail]>
> ---
>  mm/percpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/percpu.c b/mm/percpu.c
> index b40112b2fc59..c996bcffbb2a 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -234,7 +234,7 @@ static int pcpu_chunk_slot(const struct pcpu_chunk
> *chunk)
>  	if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE || chunk->contig_bits
> == 0)
>  		return 0;
> 
> -	return pcpu_size_to_slot(chunk->free_bytes);
> +	return pcpu_size_to_slot(chunk->contig_bits * PCPU_MIN_ALLOC_SIZE);
>  }
> 
>  /* set the pointer to a chunk in a page struct */

Reviewed-by: Peng Fan <[e-mail]>

Not relevant to this patch, another optimization to percpu might be good
to use per chunk spin_lock, not gobal pcpu_lock.

Thanks,
Peng.

> --
> 2.17.1

Dennis Zhou March 2, 2019, 10:32 p.m. UTC | #2

On Sat, Mar 02, 2019 at 01:48:20PM +0000, Peng Fan wrote:
> 
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On
> > Behalf Of Dennis Zhou
> > Sent: 2019年2月28日 10:19
> > To: Dennis Zhou <[e-mail]>; Tejun Heo <[e-mail]>; Christoph
> > Lameter <[e-mail]>
> > Cc: Vlad Buslov <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]
> > Subject: [PATCH 04/12] percpu: manage chunks based on contig_bits instead
> > of free_bytes
> > 
> > When a chunk becomes fragmented, it can end up having a large number of
> > small allocation areas free. The free_bytes sorting of chunks leads to
> > unnecessary checking of chunks that cannot satisfy the allocation.
> > Switch to contig_bits sorting to prevent scanning chunks that may not be able
> > to service the allocation request.
> > 
> > Signed-off-by: Dennis Zhou <[e-mail]>
> > ---
> >  mm/percpu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index b40112b2fc59..c996bcffbb2a 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -234,7 +234,7 @@ static int pcpu_chunk_slot(const struct pcpu_chunk
> > *chunk)
> >  	if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE || chunk->contig_bits
> > == 0)
> >  		return 0;
> > 
> > -	return pcpu_size_to_slot(chunk->free_bytes);
> > +	return pcpu_size_to_slot(chunk->contig_bits * PCPU_MIN_ALLOC_SIZE);
> >  }
> > 
> >  /* set the pointer to a chunk in a page struct */
> 
> Reviewed-by: Peng Fan <[e-mail]>
> 
> Not relevant to this patch, another optimization to percpu might be good
> to use per chunk spin_lock, not gobal pcpu_lock.
> 

Percpu memory itself is expensive and for the most part shouldn't be
part of the critical path. Ideally, we don't have multiple chunks being
allocated simultaneously because once an allocation is given out, the
chunk is pinned until all allocations are freed.

Thanks,
Dennis

Peng Fan March 3, 2019, 8:42 a.m. UTC | #3

> -----Original Message-----
> From: Dennis Zhou [mailto:[e-mail]]
> Sent: 2019年3月3日 6:32
> To: Peng Fan <[e-mail]>
> Cc: Tejun Heo <[e-mail]>; Christoph Lameter <[e-mail]>; Vlad
> Buslov <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [PATCH 04/12] percpu: manage chunks based on contig_bits
> instead of free_bytes
> 
> On Sat, Mar 02, 2019 at 01:48:20PM +0000, Peng Fan wrote:
> >
> >
> > > -----Original Message-----
> > > From: [e-mail] [mailto:[e-mail]]
> On
> > > Behalf Of Dennis Zhou
> > > Sent: 2019年2月28日 10:19
> > > To: Dennis Zhou <[e-mail]>; Tejun Heo <[e-mail]>;
> > > Christoph Lameter <[e-mail]>
> > > Cc: Vlad Buslov <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]
> > > Subject: [PATCH 04/12] percpu: manage chunks based on contig_bits
> > > instead of free_bytes
> > >
> > > When a chunk becomes fragmented, it can end up having a large number
> > > of small allocation areas free. The free_bytes sorting of chunks
> > > leads to unnecessary checking of chunks that cannot satisfy the allocation.
> > > Switch to contig_bits sorting to prevent scanning chunks that may
> > > not be able to service the allocation request.
> > >
> > > Signed-off-by: Dennis Zhou <[e-mail]>
> > > ---
> > >  mm/percpu.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/percpu.c b/mm/percpu.c index
> > > b40112b2fc59..c996bcffbb2a 100644
> > > --- a/mm/percpu.c
> > > +++ b/mm/percpu.c
> > > @@ -234,7 +234,7 @@ static int pcpu_chunk_slot(const struct
> > > pcpu_chunk
> > > *chunk)
> > >  	if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE ||
> chunk->contig_bits
> > > == 0)
> > >  		return 0;
> > >
> > > -	return pcpu_size_to_slot(chunk->free_bytes);
> > > +	return pcpu_size_to_slot(chunk->contig_bits *
> > > +PCPU_MIN_ALLOC_SIZE);
> > >  }
> > >
> > >  /* set the pointer to a chunk in a page struct */
> >
> > Reviewed-by: Peng Fan <[e-mail]>
> >
> > Not relevant to this patch, another optimization to percpu might be
> > good to use per chunk spin_lock, not gobal pcpu_lock.
> >
> 
> Percpu memory itself is expensive and for the most part shouldn't be part of
> the critical path. Ideally, we don't have multiple chunks being allocated
> simultaneously because once an allocation is given out, the chunk is pinned
> until all allocations are freed.

Thanks for clarification, since not critical patch, use global lock should be ok.

Thanks,
Peng.

> 
> Thanks,
> Dennis

Patch

diff --git a/mm/percpu.c b/mm/percpu.c
index b40112b2fc59..c996bcffbb2a 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -234,7 +234,7 @@  static int pcpu_chunk_slot(const struct pcpu_chunk *chunk)
 	if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE || chunk->contig_bits == 0)
 		return 0;
 
-	return pcpu_size_to_slot(chunk->free_bytes);
+	return pcpu_size_to_slot(chunk->contig_bits * PCPU_MIN_ALLOC_SIZE);
 }
 
 /* set the pointer to a chunk in a page struct */