pcpu_create_chunk in percpu-km

pcpu_create_chunk in percpu-km

[Peng Fan]

Hi,

I am reading the percpu-km source code and found that in
pcpu_create_chunk, only pcpu_group_sizes[0] is taken into
consideration, I am wondering why other pcpu_group_sizes[x]
are not used?

Is the following piece code the correct logic?

@@ -47,12 +47,15 @@ static void pcpu_depopulate_chunk(struct pcpu_chunk *chunk,

 static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
 {
-       const int nr_pages = pcpu_group_sizes[0] >> PAGE_SHIFT;
+       int nr_pages = 0;
        struct pcpu_chunk *chunk;
        struct page *pages;
        unsigned long flags;
        int i;

+       for (i = 0; i < pcpu_nr_groups; i++)
+               nr_pages += pcpu_group_sizes[i] >> PAGE_SHIFT;
+
        chunk = pcpu_alloc_chunk(gfp);
        if (!chunk)
                return NULL;

Thanks,
Peng.

Re: pcpu_create_chunk in percpu-km

[Dennis Zhou]

Hi Peng,

On Wed, Feb 06, 2019 at 12:23:44PM +0000, Peng Fan wrote:
> Hi,
> 
> I am reading the percpu-km source code and found that in
> pcpu_create_chunk, only pcpu_group_sizes[0] is taken into
> consideration, I am wondering why other pcpu_group_sizes[x]
> are not used?
> 
> Is the following piece code the correct logic?
> 
> @@ -47,12 +47,15 @@ static void pcpu_depopulate_chunk(struct pcpu_chunk *chunk,
> 
>  static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
>  {
> -       const int nr_pages = pcpu_group_sizes[0] >> PAGE_SHIFT;
> +       int nr_pages = 0;
>         struct pcpu_chunk *chunk;
>         struct page *pages;
>         unsigned long flags;
>         int i;
> 
> +       for (i = 0; i < pcpu_nr_groups; i++)
> +               nr_pages += pcpu_group_sizes[i] >> PAGE_SHIFT;
> +
>         chunk = pcpu_alloc_chunk(gfp);
>         if (!chunk)
>                 return NULL;
> 
> Thanks,
> Peng.
> 

The include for percpu-km.c vs percpu-vm.c is based on
CONFIG_NEED_PER_CPU_KM. This is set in mm/Kconfig which is dependent on
!SMP. Given that, it will only be called with the UP (uniprocessor)
version of setup_per_cpu_areas() which inits based on
pcpu_alloc_alloc_info(1, 1).  So, because of this, we know there will
not be other groups. In the UP case, percpu just identity maps percpu
variables.

Thanks,
Dennis

RE: pcpu_create_chunk in percpu-km

[Peng Fan]

Hi Dennis,

> -----Original Message-----
> From: Dennis Zhou [mailto:dennis@kernel.org]
> Sent: 2019年2月7日 1:18
> To: Peng Fan <peng.fan@nxp.com>
> Cc: dennis@kernel.org; tj@kernel.org; cl@linux.com; linux-mm@kvack.org
> Subject: Re: pcpu_create_chunk in percpu-km
> 
> Hi Peng,
> 
> On Wed, Feb 06, 2019 at 12:23:44PM +0000, Peng Fan wrote:
> > Hi,
> >
> > I am reading the percpu-km source code and found that in
> > pcpu_create_chunk, only pcpu_group_sizes[0] is taken into
> > consideration, I am wondering why other pcpu_group_sizes[x] are not
> > used?
> >
> > Is the following piece code the correct logic?
> >
> > @@ -47,12 +47,15 @@ static void pcpu_depopulate_chunk(struct
> > pcpu_chunk *chunk,
> >
> >  static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)  {
> > -       const int nr_pages = pcpu_group_sizes[0] >> PAGE_SHIFT;
> > +       int nr_pages = 0;
> >         struct pcpu_chunk *chunk;
> >         struct page *pages;
> >         unsigned long flags;
> >         int i;
> >
> > +       for (i = 0; i < pcpu_nr_groups; i++)
> > +               nr_pages += pcpu_group_sizes[i] >> PAGE_SHIFT;
> > +
> >         chunk = pcpu_alloc_chunk(gfp);
> >         if (!chunk)
> >                 return NULL;
> >
> > Thanks,
> > Peng.
> >
> 
> The include for percpu-km.c vs percpu-vm.c is based on
> CONFIG_NEED_PER_CPU_KM. This is set in mm/Kconfig which is dependent
> on !SMP. Given that, it will only be called with the UP (uniprocessor) version of
> setup_per_cpu_areas() which inits based on pcpu_alloc_alloc_info(1, 1).  So,
> because of this, we know there will not be other groups. In the UP case,
> percpu just identity maps percpu variables.

Thanks for the clarification, it is clear to me.

Thanks,
Peng.

> 
> Thanks,
> Dennis