Hi Dennis,
On 11/14/22 at 08:13pm, Dennis Zhou wrote:
> Hi Vlastimil & Baoquan,
>
> On Mon, Nov 14, 2022 at 06:58:13PM +0100, Vlastimil Babka wrote:
> > On 11/14/22 08:44, Baoquan He wrote:
> > > Hi,
> > >
> > > I reproduced the build failure according to lkp report and made a patch
> > > as below to fix it.
> > >
> > > From dae7dd9705015ce36db757e88c78802584f949b1 Mon Sep 17 00:00:00 2001
> > > From: Baoquan He <bhe@xxxxxxxxxx>
> > > Date: Sun, 13 Nov 2022 18:08:27 +0800
> > > Subject: [PATCH] percpu: adjust the value of PERCPU_DYNAMIC_EARLY_SIZE
> > > Content-type: text/plain
> > >
> > > LKP reported a build failure as below on the patch "mm/slub, percpu:
> > > correct the calculation of early percpu allocation size"
> >
> > Since I have that patch in slab.git exposed to -next, should I take this fix
> > too, to make things simpler? Dennis?
> >
>
> I don't have any problems with you running a fix, but I'm not quite sure
> this is the right fix. Though this might cause a trivial merge conflict
> with: d667c94962c1 ("mm/percpu: remove unused PERCPU_DYNAMIC_EARLY_SLOTS")
> in my percpu#for-6.2 branch.
>
> If I'm understanding this correctly, slub requires additional percpu
> memory due to the use of 64k pages. By increasing
> PERCPU_DYNAMIC_EARLY_SIZE, we solve the problem for 64k page users, but
> require a few unnecessary pages that can bloat the size of subsequent
> percpu chunks. Though, I'm not sure if that's an issue today for
> embedded devices.
Thanks for looking into this.
I guess you are talking about PERCPU_DYNAMIC_EARLY_SIZE will impact the
first dynamic chunk size of page first chunk, because the embed first
chunk will take PERCPU_DYNAMIC_RESERVE. And the impact is done in below
max() invacation.
static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
size_t reserved_size, size_t dyn_size,
size_t atom_size,
pcpu_fc_cpu_distance_fn_t cpu_distance_fn)
{
......
/* calculate size_sum and ensure dyn_size is enough for early alloc */
size_sum = PFN_ALIGN(static_size + reserved_size +
max_t(size_t, dyn_size, PERCPU_DYNAMIC_EARLY_SIZE));
......
}
>
> I think adding parity to PERCPU_DYNAMIC_EARLY_SIZE with
> PERCPU_DYNAMIC_RESERVE is defined by BITS_PER_LONG is a safer option
> here. A small TODO item would be to make PERCPU_DYNAMIC_RESERVE be a +
> value instead of a max() with PERCPU_DYNAMIC_EARLY_SIZE.
Hmm, the below change may not take power arch into account. Please
check arch/powerpc/include/asm/page.h, seems the 32bit ppc could have
256K pages too. Adding PERCPU_DYNAMIC_EARLY_SIZE to 20K may cost extra
memory during boot. But th left space of 1st dynamic chunk will join
the later percpu dynamic allocation, it's not wasted, right?
Not sure if I got your point.
>
> ---
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index f1ec5ad1351c..22ce3271eed2 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -42,7 +42,11 @@
> * larger than PERCPU_DYNAMIC_EARLY_SIZE.
> */
> #define PERCPU_DYNAMIC_EARLY_SLOTS 128
> +#if BITS_PER_LONG > 32
> +#define PERCPU_DYNAMIC_EARLY_SIZE (20 << 10)
> +#else
> #define PERCPU_DYNAMIC_EARLY_SIZE (12 << 10)
> +#endif
>
> /*
> * PERCPU_DYNAMIC_RESERVE indicates the amount of free area to piggy
>
