2018-11-18 08:24:22

by Qian Cai

[permalink] [raw]
Subject: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

The current value of ODEBUG_POOL_SIZE is not big enough for large memory
systems with timer or/and workqueue objects because during the early
boot, timer objects needs at least the size equals to

No. CPUs x 2 (worker pool)

start_kernel
workqueue_init_early
init_worker_pool
init_timer_key
debug_object_init

puls, No. CPUs

start_kernel
sched_init
hrtimer_init
debug_object_init

Then, workqueue objects requires even more,

No. CPUs x 2 (worker pool) x 6 (workqueue)

start_kernel
workqueue_init_early
__alloc_workqueue_key
alloc_workqueue
init_pwq
debug_object_init

plus, No, CPUs x 2 (worker pool)

start_kernel
perf_event_init
__init_srcu_struct
init_srcu_struct_fields
__init_work
debug_object_init

As the results, systems have 60+ CPUs with both timer and workqueue
objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".

Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
accordingly if either timer or workqueue objects are selected.

Signed-off-by: Qian Cai <[email protected]>
---
lib/Kconfig.debug | 12 ++++++++++++
lib/debugobjects.c | 7 ++++++-
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af29b8224fd..a4af837649df 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -449,6 +449,18 @@ config DEBUG_OBJECTS
kernel to track the life time of various objects and validate
the operations on those objects.

+config DEBUG_OBJECTS_POOL_SIZE
+ int "Debug objects pool size"
+ depends on DEBUG_OBJECTS_TIMERS || DEBUG_OBJECTS_WORK
+ default 1024
+ help
+ Some debug objects are allocated during the early boot. Enable some
+ config options may requires the sizes below,
+
+ DEBUG_OBJECTS_TIMERS: No. CPUs x 2 (worker pool) + No. CPUs (hrtimer)
+ DEBUG_OBJECTS_WORK: No. CPUs x 2 (worker pool) x 6 (workqueue) +
+ No. CPUs x 2 (worker pool)
+
config DEBUG_OBJECTS_SELFTEST
bool "Debug objects selftest"
depends on DEBUG_OBJECTS
diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 70935ed91125..eb8158538993 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -23,7 +23,12 @@
#define ODEBUG_HASH_BITS 14
#define ODEBUG_HASH_SIZE (1 << ODEBUG_HASH_BITS)

-#define ODEBUG_POOL_SIZE 1024
+#ifdef CONFIG_DEBUG_OBJECTS_POOL_SIZE
+#define ODEBUG_POOL_SIZE CONFIG_DEBUG_OBJECTS_POOL_SIZE
+#else
+#define ODEBUG_POOL_SIZE 1024
+#endif
+
#define ODEBUG_POOL_MIN_LEVEL 256

#define ODEBUG_CHUNK_SHIFT PAGE_SHIFT
--
2.17.2 (Apple Git-113)



2018-11-18 18:24:25

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

Qian,

On Sun, 18 Nov 2018, Qian Cai wrote:

> The current value of ODEBUG_POOL_SIZE is not big enough for large memory
> systems with timer or/and workqueue objects because during the early
> boot, timer objects needs at least the size equals to
>
> No. CPUs x 2 (worker pool)
>
> start_kernel
> workqueue_init_early
> init_worker_pool
> init_timer_key
> debug_object_init
>
> puls, No. CPUs
>
> start_kernel
> sched_init
> hrtimer_init
> debug_object_init
>
> Then, workqueue objects requires even more,
>
> No. CPUs x 2 (worker pool) x 6 (workqueue)
>
> start_kernel
> workqueue_init_early
> __alloc_workqueue_key
> alloc_workqueue
> init_pwq
> debug_object_init
>
> plus, No, CPUs x 2 (worker pool)
>
> start_kernel
> perf_event_init
> __init_srcu_struct
> init_srcu_struct_fields
> __init_work
> debug_object_init
>
> As the results, systems have 60+ CPUs with both timer and workqueue
> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".
>
> Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
> accordingly if either timer or workqueue objects are selected.

why do we need a config option, when the required number can be deduced
already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?

Thanks,

tglx

2018-11-18 18:57:57

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE



> On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]> wrote:
>
> Qian,
>
> On Sun, 18 Nov 2018, Qian Cai wrote:
>
>> The current value of ODEBUG_POOL_SIZE is not big enough for large memory
>> systems with timer or/and workqueue objects because during the early
>> boot, timer objects needs at least the size equals to
>>
>> No. CPUs x 2 (worker pool)
>>
>> start_kernel
>> workqueue_init_early
>> init_worker_pool
>> init_timer_key
>> debug_object_init
>>
>> puls, No. CPUs
>>
>> start_kernel
>> sched_init
>> hrtimer_init
>> debug_object_init
>>
>> Then, workqueue objects requires even more,
>>
>> No. CPUs x 2 (worker pool) x 6 (workqueue)
>>
>> start_kernel
>> workqueue_init_early
>> __alloc_workqueue_key
>> alloc_workqueue
>> init_pwq
>> debug_object_init
>>
>> plus, No, CPUs x 2 (worker pool)
>>
>> start_kernel
>> perf_event_init
>> __init_srcu_struct
>> init_srcu_struct_fields
>> __init_work
>> debug_object_init
>>
>> As the results, systems have 60+ CPUs with both timer and workqueue
>> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".
>>
>> Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
>> accordingly if either timer or workqueue objects are selected.
>
> why do we need a config option, when the required number can be deduced
> already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
>
It because I am worry about the coupling between the implementation details of
timers and workqueue objects, and the computation in the code you mentioned
here. For example, people could change workqueue.c to have different number
of worekqueues initialized during the early boot in the future which is going to
affect the required pool size, and I am not sure if people are going to adjust the
code in debugobjects.c here as well when they made changes like that.

Also, the computation could become so complex depends on lots of config
options like perf, hrtimer, and combinations that I have not tested so far which is
difficult to exhausted all the possibilities.

Hence, I feel like the Kconfig option is more flexible and less error-prone.

2018-11-18 22:18:54

by Qian Cai

[permalink] [raw]
Subject: [PATCH v2] debugobjects: add a new Kconfig for POOL_SIZE

The current value of ODEBUG_POOL_SIZE is not big enough for systems with
large number of CPUs with timer or/and workqueue objects because during
the early boot, timer objects needs the size equals to

No. CPUs x 2 (worker pool)

start_kernel
workqueue_init_early
init_worker_pool
init_timer_key
debug_object_init

puls, No. CPUs (if have the hrtimer option)

start_kernel
sched_init
hrtick_rq_init
hrtimer_init
debug_object_init

Then, workqueue objects requires even more,

No. CPUs x 6 (workqueue)

start_kernel
workqueue_init_early
alloc_workqueue
__alloc_workqueue_key
alloc_and_link_pwqs
init_pwq
debug_object_init

plus, No, CPUs x 2 (worker pool) (if have the perf option)

start_kernel
perf_event_init
__init_srcu_struct
init_srcu_struct_fields
__init_work
debug_object_init

As the results, systems have 60+ CPUs with both timer and workqueue
objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".

Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
accordingly if either timer or workqueue objects are selected.

Signed-off-by: Qian Cai <[email protected]>
---

Changes since v1:
* Fixed a few typos.
* Added missing call stacks.
* Removed implementation details from the Kconfig help.

lib/Kconfig.debug | 11 +++++++++++
lib/debugobjects.c | 4 ++++
2 files changed, 15 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1af29b8224fd..0749dc627eb5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -449,6 +449,17 @@ config DEBUG_OBJECTS
kernel to track the life time of various objects and validate
the operations on those objects.

+config DEBUG_OBJECTS_POOL_SIZE
+ int "Debug objects pool size"
+ depends on DEBUG_OBJECTS_TIMERS || DEBUG_OBJECTS_WORK
+ default 1024
+ help
+ Some debug objects are allocated during the early boot. Enabling some
+ options below like timers or workqueue objects may increase the size
+ required significantly with large number of CPUs. If the kernel
+ reports "ODEBUG: Out of memory. ODEBUG disabled", please increase
+ this value.
+
config DEBUG_OBJECTS_SELFTEST
bool "Debug objects selftest"
depends on DEBUG_OBJECTS
diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 70935ed91125..b0e8e90f6ad1 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -23,7 +23,11 @@
#define ODEBUG_HASH_BITS 14
#define ODEBUG_HASH_SIZE (1 << ODEBUG_HASH_BITS)

+#ifdef CONFIG_DEBUG_OBJECTS_POOL_SIZE
+#define ODEBUG_POOL_SIZE CONFIG_DEBUG_OBJECTS_POOL_SIZE
+#else
#define ODEBUG_POOL_SIZE 1024
+#endif
#define ODEBUG_POOL_MIN_LEVEL 256

#define ODEBUG_CHUNK_SHIFT PAGE_SHIFT
--
2.17.2 (Apple Git-113)


2018-11-19 08:11:25

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

Qian,

On Sun, 18 Nov 2018, Qian Cai wrote:
> > On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]> wrote:
> > On Sun, 18 Nov 2018, Qian Cai wrote:
> >> As the results, systems have 60+ CPUs with both timer and workqueue
> >> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".
> >>
> >> Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
> >> accordingly if either timer or workqueue objects are selected.
> >
> > why do we need a config option, when the required number can be deduced
> > already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
> >
> It because I am worry about the coupling between the implementation details of
> timers and workqueue objects, and the computation in the code you mentioned
> here. For example, people could change workqueue.c to have different number
> of worekqueues initialized during the early boot in the future which is going to
> affect the required pool size, and I am not sure if people are going to adjust the
> code in debugobjects.c here as well when they made changes like that.
>
> Also, the computation could become so complex depends on lots of config
> options like perf, hrtimer, and combinations that I have not tested so far which is
> difficult to exhausted all the possibilities.
>
> Hence, I feel like the Kconfig option is more flexible and less error-prone.

Quite the contrary. Config options are a pain and truly error-prone if you
want to compile general purpose kernels as distributions do.

Its not really a problem to have a larger initial static pool which gets
freed after init anyway. So we can size it generously depending on the
config options and be done with it.

Thanks,

tglx


2018-11-19 13:29:22

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On 11/19/18 at 3:09 AM, Thomas Gleixner wrote:

> Qian,
>
> On Sun, 18 Nov 2018, Qian Cai wrote:
> > > On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]> wrote:
> > > On Sun, 18 Nov 2018, Qian Cai wrote:
> > >> As the results, systems have 60+ CPUs with both timer and workqueue
> > >> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".
> > >>
> > >> Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
> > >> accordingly if either timer or workqueue objects are selected.
> > >
> > > why do we need a config option, when the required number can be deduced
> > > already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
> > >
> > It because I am worry about the coupling between the implementation details of
> > timers and workqueue objects, and the computation in the code you mentioned
> > here. For example, people could change workqueue.c to have different number
> > of worekqueues initialized during the early boot in the future which is going to
> > affect the required pool size, and I am not sure if people are going to adjust the
> > code in debugobjects.c here as well when they made changes like that.
> >
> > Also, the computation could become so complex depends on lots of config
> > options like perf, hrtimer, and combinations that I have not tested so far which is
> > difficult to exhausted all the possibilities.
> >
> > Hence, I feel like the Kconfig option is more flexible and less error-prone.
>
> Quite the contrary. Config options are a pain and truly error-prone if you
> want to compile general purpose kernels as distributions do.
>
Ah, I never thought distributions people would
enable those debugging options.
> Its not really a problem to have a larger initial static pool which gets
> freed after init anyway. So we can size it generously depending on the
> config options and be done with it.
>
That’s a good point. I’ll send out a patch shortly.

2018-11-19 14:52:28

by Waiman Long

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On 11/19/2018 08:27 AM, Qian Cai wrote:
> On 11/19/18 at 3:09 AM, Thomas Gleixner wrote:
>
>> Qian,
>>
>> On Sun, 18 Nov 2018, Qian Cai wrote:
>>>> On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]> wrote:
>>>> On Sun, 18 Nov 2018, Qian Cai wrote:
>>>>> As the results, systems have 60+ CPUs with both timer and workqueue
>>>>> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled".
>>>>>
>>>>> Hence, add a new Kconfig option so users could adjust ODEBUG_POOL_SIZE
>>>>> accordingly if either timer or workqueue objects are selected.
>>>> why do we need a config option, when the required number can be deduced
>>>> already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
>>>>
>>> It because I am worry about the coupling between the implementation details of
>>> timers and workqueue objects, and the computation in the code you mentioned
>>> here. For example, people could change workqueue.c to have different number
>>> of worekqueues initialized during the early boot in the future which is going to
>>> affect the required pool size, and I am not sure if people are going to adjust the
>>> code in debugobjects.c here as well when they made changes like that.
>>>
>>> Also, the computation could become so complex depends on lots of config
>>> options like perf, hrtimer, and combinations that I have not tested so far which is
>>> difficult to exhausted all the possibilities.
>>>
>>> Hence, I feel like the Kconfig option is more flexible and less error-prone.
>> Quite the contrary. Config options are a pain and truly error-prone if you
>> want to compile general purpose kernels as distributions do.
>>
> Ah, I never thought distributions people would
> enable those debugging options.

Distros like RHEL usually ship two kernels - one for production and one
for debug. The debug kernel does have debugobjects enabled.

Cheers,
Longman

2018-11-19 15:19:35

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On Mon, 2018-11-19 at 09:51 -0500, Waiman Long wrote:
> On 11/19/2018 08:27 AM, Qian Cai wrote:
> > On 11/19/18 at 3:09 AM, Thomas Gleixner wrote:
> >
> > > Qian,
> > >
> > > On Sun, 18 Nov 2018, Qian Cai wrote:
> > > > > On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]>
> > > > > wrote:
> > > > > On Sun, 18 Nov 2018, Qian Cai wrote:
> > > > > > As the results, systems have 60+ CPUs with both timer and workqueue
> > > > > > objects enabled could trigger "ODEBUG: Out of memory. ODEBUG
> > > > > > disabled".
> > > > > >
> > > > > > Hence, add a new Kconfig option so users could adjust
> > > > > > ODEBUG_POOL_SIZE
> > > > > > accordingly if either timer or workqueue objects are selected.
> > > > >
> > > > > why do we need a config option, when the required number can be
> > > > > deduced
> > > > > already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
> > > > >
> > > >
> > > > It because I am worry about the coupling between the implementation
> > > > details of
> > > > timers and workqueue objects, and the computation in the code you
> > > > mentioned
> > > > here. For example, people could change workqueue.c to have different
> > > > number
> > > > of worekqueues initialized during the early boot in the future which is
> > > > going to
> > > > affect the required pool size, and I am not sure if people are going to
> > > > adjust the
> > > > code in debugobjects.c here as well when they made changes like that.
> > > >
> > > > Also, the computation could become so complex depends on lots of config
> > > > options like perf, hrtimer, and combinations that I have not tested so
> > > > far which is
> > > > difficult to exhausted all the possibilities.
> > > >
> > > > Hence, I feel like the Kconfig option is more flexible and less error-
> > > > prone.
> > >
> > > Quite the contrary. Config options are a pain and truly error-prone if you
> > > want to compile general purpose kernels as distributions do.
> > >
> >
> > Ah, I never thought distributions people would
> > enable those debugging options.
>
> Distros like RHEL usually ship two kernels - one for production and one
> for debug. The debug kernel does have debugobjects enabled.
>
Right, I can remember that now . However, if I understand correctly, since the
early static pool size needs to be determined during the compilation time, it
depends on the No. CPUs are from the machines that built the distro kernels.
Then, when users use those distro kernels, they are not going to have correct
the pool size according to the No. CPUs on their test machines.

2018-11-19 16:22:38

by Waiman Long

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On 11/19/2018 10:17 AM, Qian Cai wrote:
> On Mon, 2018-11-19 at 09:51 -0500, Waiman Long wrote:
>> On 11/19/2018 08:27 AM, Qian Cai wrote:
>>> On 11/19/18 at 3:09 AM, Thomas Gleixner wrote:
>>>
>>>> Qian,
>>>>
>>>> On Sun, 18 Nov 2018, Qian Cai wrote:
>>>>>> On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <[email protected]>
>>>>>> wrote:
>>>>>> On Sun, 18 Nov 2018, Qian Cai wrote:
>>>>>>> As the results, systems have 60+ CPUs with both timer and workqueue
>>>>>>> objects enabled could trigger "ODEBUG: Out of memory. ODEBUG
>>>>>>> disabled".
>>>>>>>
>>>>>>> Hence, add a new Kconfig option so users could adjust
>>>>>>> ODEBUG_POOL_SIZE
>>>>>>> accordingly if either timer or workqueue objects are selected.
>>>>>> why do we need a config option, when the required number can be
>>>>>> deduced
>>>>>> already from the active CONFIG_DEBUG_OBJECTS_* and NR_CPUS?
>>>>>>
>>>>> It because I am worry about the coupling between the implementation
>>>>> details of
>>>>> timers and workqueue objects, and the computation in the code you
>>>>> mentioned
>>>>> here. For example, people could change workqueue.c to have different
>>>>> number
>>>>> of worekqueues initialized during the early boot in the future which is
>>>>> going to
>>>>> affect the required pool size, and I am not sure if people are going to
>>>>> adjust the
>>>>> code in debugobjects.c here as well when they made changes like that.
>>>>>
>>>>> Also, the computation could become so complex depends on lots of config
>>>>> options like perf, hrtimer, and combinations that I have not tested so
>>>>> far which is
>>>>> difficult to exhausted all the possibilities.
>>>>>
>>>>> Hence, I feel like the Kconfig option is more flexible and less error-
>>>>> prone.
>>>> Quite the contrary. Config options are a pain and truly error-prone if you
>>>> want to compile general purpose kernels as distributions do.
>>>>
>>> Ah, I never thought distributions people would
>>> enable those debugging options.
>> Distros like RHEL usually ship two kernels - one for production and one
>> for debug. The debug kernel does have debugobjects enabled.
>>
> Right, I can remember that now . However, if I understand correctly, since the
> early static pool size needs to be determined during the compilation time, it
> depends on the No. CPUs are from the machines that built the distro kernels.
> Then, when users use those distro kernels, they are not going to have correct
> the pool size according to the No. CPUs on their test machines.

I see your point. Perhaps you can make ODEBUG_POOL_SIZE scales with
CONFIG_NR_CPUS like

#define ODEBUG_POOL_SIZE    (1024 + CONFIG_NR_CPUS * 2)

CONFIG_NR_CPUS is usually set to a lot higher than the actual number of
CPUs in a typical system. So you don't want to set the multiplier too high.

Cheers,
Longman


2018-11-19 16:28:34

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On Mon, 19 Nov 2018, Waiman Long wrote:
> On 11/19/2018 10:17 AM, Qian Cai wrote:
> > Right, I can remember that now . However, if I understand correctly, since the
> > early static pool size needs to be determined during the compilation time, it
> > depends on the No. CPUs are from the machines that built the distro kernels.
> > Then, when users use those distro kernels, they are not going to have correct
> > the pool size according to the No. CPUs on their test machines.
>
> I see your point. Perhaps you can make ODEBUG_POOL_SIZE scales with
> CONFIG_NR_CPUS like
>
> #define ODEBUG_POOL_SIZE    (1024 + CONFIG_NR_CPUS * 2)
>
> CONFIG_NR_CPUS is usually set to a lot higher than the actual number of
> CPUs in a typical system. So you don't want to set the multiplier too high.

The number of CPUs on the build machine is totally irrelevant and not
influencing the build at all. The sizing solely depends on CONFIG_NR_CPUS.

And even if the initial pool is a bit oversized, it's init data and freed,
so no real harm done.

Thanks,

tglx

2018-11-19 18:00:09

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH] debugobjects: add a new Kconfig for POOL_SIZE

On Mon, 2018-11-19 at 17:25 +0100, Thomas Gleixner wrote:
> On Mon, 19 Nov 2018, Waiman Long wrote:
> > On 11/19/2018 10:17 AM, Qian Cai wrote:
> > > Right, I can remember that now . However, if I understand correctly, since
> > > the
> > > early static pool size needs to be determined during the compilation time,
> > > it
> > > depends on the No. CPUs are from the machines that built the distro
> > > kernels.
> > > Then, when users use those distro kernels, they are not going to have
> > > correct
> > > the pool size according to the No. CPUs on their test machines.
> >
> > I see your point. Perhaps you can make ODEBUG_POOL_SIZE scales with
> > CONFIG_NR_CPUS like
> >
> > #define ODEBUG_POOL_SIZE    (1024 + CONFIG_NR_CPUS * 2)
> >
> > CONFIG_NR_CPUS is usually set to a lot higher than the actual number of
> > CPUs in a typical system. So you don't want to set the multiplier too high.
>
> The number of CPUs on the build machine is totally irrelevant and not
> influencing the build at all. The sizing solely depends on CONFIG_NR_CPUS.
>
> And even if the initial pool is a bit oversized, it's init data and freed,
> so no real harm done.
>
Ah, I thought you meant the NR_CPUS macro. It is CONFIG_NR_CPUS, and we are
on the same page.