From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752789AbbBXVZe (ORCPT ); Tue, 24 Feb 2015 16:25:34 -0500 Received: from mout.gmx.net ([212.227.17.20]:60252 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752549AbbBXVZd (ORCPT ); Tue, 24 Feb 2015 16:25:33 -0500 Message-ID: <54ECEBD0.9060800@gmx.de> Date: Tue, 24 Feb 2015 22:23:28 +0100 From: Heinrich Schuchardt User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.4.0 MIME-Version: 1.0 To: David Rientjes CC: Andrew Morton , Aaron Tomlin , Andy Lutomirski , Davidlohr Bueso , "David S. Miller" , Fabian Frederick , Guenter Roeck , "H. Peter Anvin" , Ingo Molnar , Jens Axboe , Joe Perches , Johannes Weiner , Kees Cook , Michael Marineau , Oleg Nesterov , "Paul E. McKenney" , Peter Zijlstra , Prarit Bhargava , Rik van Riel , Rusty Russell , Steven Rostedt , Thomas Gleixner , Vladimir Davydov , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3 v5] kernel/fork.c: new function for max_threads References: <1424722477-23758-1-git-send-email-xypron.glpk@gmx.de> <1424806701-30099-1-git-send-email-xypron.glpk@gmx.de> <1424806701-30099-2-git-send-email-xypron.glpk@gmx.de> In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:5vsAaPmT2/vqLBHeQV1QxKa7JpzrCYn3f8ydLkwyQo5ZkiiFNmF FytYbJhnnyx2g7o2i9wCKZxN61a+2AYLK5EPQZuPppvmkztKyEbmGpsP2uq4/B1Q383pd73 /fWvK6GnpU8CAZ77YETsHimq2hPy1JhNRQ69Qi46CngMX5jIhrfSM7si6TTfyhRpbjQl8N4 bOp7kbYe6WgiDcQGehdiw== X-UI-Out-Filterresults: notjunk:1; Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24.02.2015 22:03, David Rientjes wrote: > On Tue, 24 Feb 2015, Heinrich Schuchardt wrote: > >> diff --git a/init/main.c b/init/main.c >> index 61b99376..21394ec 100644 >> --- a/init/main.c >> +++ b/init/main.c >> @@ -94,7 +94,7 @@ >> static int kernel_init(void *); >> >> extern void init_IRQ(void); >> -extern void fork_init(unsigned long); >> +extern void fork_init(void); >> extern void radix_tree_init(void); >> #ifndef CONFIG_DEBUG_RODATA >> static inline void mark_rodata_ro(void) { } >> @@ -655,7 +655,7 @@ asmlinkage __visible void __init start_kernel(void) >> #endif >> thread_info_cache_init(); >> cred_init(); >> - fork_init(totalram_pages); >> + fork_init(); >> proc_caches_init(); >> buffer_init(); >> key_init(); >> diff --git a/kernel/fork.c b/kernel/fork.c >> index 4dc2dda..460b044 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -253,7 +253,27 @@ EXPORT_SYMBOL_GPL(__put_task_struct); >> >> void __init __weak arch_task_cache_init(void) { } >> >> -void __init fork_init(unsigned long mempages) >> +/* >> + * set_max_threads >> + * The argument is ignored. >> + */ >> +static void set_max_threads(unsigned int max_threads_suggested) >> +{ >> + /* >> + * The default maximum number of threads is set to a safe >> + * value: the thread structures can take up at most half >> + * of memory. >> + */ >> + max_threads = totalram_pages / (8 * THREAD_SIZE / PAGE_SIZE); >> + >> + /* >> + * we need to allow at least 20 threads to boot a system >> + */ >> + if (max_threads < 20) >> + max_threads = 20; >> +} >> + >> +void __init fork_init(void) >> { >> #ifndef CONFIG_ARCH_TASK_STRUCT_ALLOCATOR >> #ifndef ARCH_MIN_TASKALIGN >> @@ -268,18 +288,7 @@ void __init fork_init(unsigned long mempages) >> /* do the arch specific task caches init */ >> arch_task_cache_init(); >> >> - /* >> - * The default maximum number of threads is set to a safe >> - * value: the thread structures can take up at most half >> - * of memory. >> - */ >> - max_threads = mempages / (8 * THREAD_SIZE / PAGE_SIZE); >> - >> - /* >> - * we need to allow at least 20 threads to boot a system >> - */ >> - if (max_threads < 20) >> - max_threads = 20; >> + set_max_threads(UINT_MAX); >> >> init_task.signal->rlim[RLIMIT_NPROC].rlim_cur = max_threads/2; >> init_task.signal->rlim[RLIMIT_NPROC].rlim_max = max_threads/2; > > I'm afraid I don't understand this. The intent of the patch is to > separate the max_threads logic into a new function, correct? If that's > true, then I don't understand why UINT_MAX is being introduced into this > path and passed to the new function when it is ignored. > > I think it would be better to simply keep passing mempages to fork_init() > and then pass it to set_max_threads() where max_threads actually gets set > using the argument passed. At least, the code would then match the intent > of the patch. > Please, read patch 2/3 which provides support for the argument, and patch 3/3 that finally needs it. Best regards Heinrich