Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v4] debugobjects: scale the static pool size
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     akpm@linux-foundation.org, longman@redhat.com,
        yang.shi@linux.alibaba.com, arnd@arndb.de,
        linux-kernel@vger.kernel.org
References: <20181120232810.2503-1-cai@gmx.us>
 <20181121021157.3061-1-cai@gmx.us>
 <alpine.DEB.2.21.1811222238270.1665@nanos.tec.linutronix.de>
From:   Qian Cai <cai@gmx.us>
Message-ID: <245eeb16-e953-9375-5c54-4b9396959340@gmx.us>
Date:   Sun, 25 Nov 2018 15:48:13 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0)
 Gecko/20100101 Thunderbird/60.3.1
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.21.1811222238270.1665@nanos.tec.linutronix.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 11/22/18 4:56 PM, Thomas Gleixner wrote:
> On Tue, 20 Nov 2018, Qian Cai wrote:
> 
> Looking deeper at that.
> 
>> diff --git a/lib/debugobjects.c b/lib/debugobjects.c
>> index 70935ed91125..140571aa483c 100644
>> --- a/lib/debugobjects.c
>> +++ b/lib/debugobjects.c
>> @@ -23,9 +23,81 @@
>>   #define ODEBUG_HASH_BITS	14
>>   #define ODEBUG_HASH_SIZE	(1 << ODEBUG_HASH_BITS)
>>   
>> -#define ODEBUG_POOL_SIZE	1024
>> +#define ODEBUG_DEFAULT_POOL	512
>>   #define ODEBUG_POOL_MIN_LEVEL	256
>>   
>> +/*
>> + * Some debug objects are allocated during the early boot. Enabling some options
>> + * like timers or workqueue objects may increase the size required significantly
>> + * with large number of CPUs. For example (as today, 20 Nov. 2018),
>> + *
>> + * No. CPUs x 2 (worker pool) objects:
>> + *
>> + * start_kernel
>> + *   workqueue_init_early
>> + *     init_worker_pool
>> + *       init_timer_key
>> + *         debug_object_init
>> + *
>> + * No. CPUs objects (CONFIG_HIGH_RES_TIMERS):
>> + *
>> + * sched_init
>> + *   hrtick_rq_init
>> + *     hrtimer_init
>> + *
>> + * CONFIG_DEBUG_OBJECTS_WORK:
>> + * No. CPUs x 6 (workqueue) objects:
>> + *
>> + * workqueue_init_early
>> + *   alloc_workqueue
>> + *     __alloc_workqueue_key
>> + *       alloc_and_link_pwqs
>> + *         init_pwq
>> + *
>> + * Also, plus No. CPUs objects:
>> + *
>> + * perf_event_init
>> + *    __init_srcu_struct
>> + *      init_srcu_struct_fields
>> + *        init_srcu_struct_nodes
>> + *          __init_work
> 
> None of the things are actually used or required _BEFORE_
> debug_objects_mem_init() is invoked.
> 
> The reason why the call is at this place in start_kernel() is
> historical. It's because back in the days when debugobjects were added the
> memory allocator was enabled way later than today. So we can just move the
> debug_objects_mem_init() call right before sched_init() I think.
> 
>> + * Increase the number a bit more in case the implmentatins are changed in the
>> + * future, as it is better to avoid OOM than spending a bit more kernel memory
>> + * that will/can be freed.
>> + *
>> + * With all debug objects config options selected except the workqueue and the
>> + * timers, kernel reports,
>> + * 64-CPU:  ODEBUG: 4 of 4 active objects replaced
>> + * 256-CPU: ODEBUG: 4 of 4 active objects replaced
>> + *
>> + * all the options except the workqueue:
>> + * 64-CPU:  ODEBUG: 466 of 466 active objects replaced
>> + * 256-CPU: ODEBUG: 1810 of 1810 active objects replaced
>> + *
>> + * all the options except the timers:
>> + * 64-CPU:  ODEBUG: 652 of 652 active objects replaced
>> + * 256-CPU: ODEBUG: 2572 of 2572 active objects replaced
>> + *
>> + * all the options:
>> + * 64-CPU:  ODEBUG: 1114 of 1114 active objects replaced
>> + * 256-CPU: ODEBUG: 4378 of 4378 active objects replaced
>> + */
>> +#ifdef CONFIG_DEBUG_OBJECTS_WORK
>> +#define ODEBUG_WORK_PCPU_CNT	10
>> +#else
>> +#define ODEBUG_WORK_PCPU_CNT	0
>> +#endif /* CONFIG_DEBUG_OBJECTS_WORK */
>> +
>> +#ifdef CONFIG_DEBUG_OBJECTS_TIMERS
>> +#define ODEBUG_TIMERS_PCPU_CNT	10
>> +#else
>> +#define ODEBUG_TIMERS_PCPU_CNT	0
>> +#endif /* CONFIG_DEBUG_OBJECTS_TIMERS */
> 
> Btw, the scaling here is way off.
> 
>> +#define ODEBUG_POOL_SIZE	(ODEBUG_DEFAULT_POOL + CONFIG_NR_CPUS * \
>> +				(ODEBUG_TIMERS_PCPU_CNT + ODEBUG_WORK_PCPU_CNT))
> 
> CONFIG_NR_CPUS	Pool size		Storage size
> 
> 256		256512			4M
> 		
> According to your list above it uses 4378 object for 256 CPUs. That's off
> by an factor of ~58.
> 

Hmm, it is not clear to me why this is off and where did the pool size 256512 
come from.

CONFIG_NR_CPUS: 256
Pool size: 1024 + 256 * (10 + 10) = 6144

Am I missing anything?