[PATCH] debugobjects: add a new Kconfig for POOL_SIZE

[PATCH] debugobjects: add a new Kconfig for POOL_SIZE

[Qian Cai]

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 <cai@gmx.us>
---
 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)

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

[Thomas Gleixner]

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

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

[Qian Cai]

> On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <tglx@linutronix.de> 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.

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

[Thomas Gleixner]

Qian,

On Sun, 18 Nov 2018, Qian Cai wrote:
> > On Nov 18, 2018, at 1:21 PM, Thomas Gleixner <tglx@linutronix.de> 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

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

[Qian Cai]

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 <tglx@linutronix.de> 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.

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

[Waiman Long]

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 <tglx@linutronix.de> 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

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

[Qian Cai]

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 <tglx@linutronix.de>
> > > > > 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.

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

[Waiman Long]

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 <tglx@linutronix.de>
>>>>>> 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

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

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

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

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

[Qian Cai]

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.

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

[Qian Cai]

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 <cai@gmx.us>
---

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)