From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752106AbbCPHl5 (ORCPT ); Mon, 16 Mar 2015 03:41:57 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:37987 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750736AbbCPHlx (ORCPT ); Mon, 16 Mar 2015 03:41:53 -0400 Date: Mon, 16 Mar 2015 08:41:46 +0100 From: Ingo Molnar To: Heinrich Schuchardt Cc: Andrew Morton , Aaron Tomlin , Andy Lutomirski , Davidlohr Bueso , David Rientjes , "David S. Miller" , Fabian Frederick , Guenter Roeck , "H. Peter Anvin" , Jens Axboe , Joe Perches , Johannes Weiner , Jonathan Corbet , 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, linux-doc@vger.kernel.org Subject: Re: [PATCH 2/4 v6] kernel/fork.c: avoid division by zero Message-ID: <20150316074146.GB15458@gmail.com> References: <1426436018-5159-1-git-send-email-xypron.glpk@gmx.de> <1426436018-5159-3-git-send-email-xypron.glpk@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426436018-5159-3-git-send-email-xypron.glpk@gmx.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Heinrich Schuchardt wrote: > PAGE_SIZE is not guaranteed to be equal to or less than 8 times the > THREAD_SIZE. > > E.g. architecture hexagon may have page size 1M and thread size 4096. > This would lead to a division by zero in the calculation of max_threads. > > With 32-bit calculation there is no solution which delivers valid results > for all possible combinations of the parameters. > The code is only called once. > Hence a 64-bit calculation can be used as solution. > > Signed-off-by: Heinrich Schuchardt > --- > kernel/fork.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index bf1ff00..69ff08f 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -88,6 +88,16 @@ > #include > > /* > + * Minimum number of threads to boot the kernel > + */ > +#define MIN_THREADS 20 > + > +/* > + * Maximum number of threads > + */ > +#define MAX_THREADS FUTEX_TID_MASK > + > +/* > * Protected counters by write_lock_irq(&tasklist_lock) > */ > unsigned long total_forks; /* Handle normal Linux uptimes. */ > @@ -258,18 +268,25 @@ void __init __weak arch_task_cache_init(void) { } > */ > static void set_max_threads(void) > { > - /* > - * 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); > + u64 threads; > > /* > - * we need to allow at least 20 threads to boot a system > + * The number of threads shall be limited such that the thread > + * structures may only consume a small part of the available memory. > */ > - if (max_threads < 20) > - max_threads = 20; > + if (fls64(totalram_pages) + fls64(PAGE_SIZE) > 64) > + threads = MAX_THREADS; > + else > + threads = div64_u64((u64) totalram_pages * (u64) PAGE_SIZE, > + (u64) THREAD_SIZE * 8UL); > + > + if (threads > MAX_THREADS) > + threads = MAX_THREADS; > + > + if (threads < MIN_THREADS) > + threads = MIN_THREADS; > + > + max_threads = (int) threads; > } So why does this patch do two things: - parametrizes set_max_threads() via defines - fixes a bug ? Those two things should be done in two separate patches, first the introduction of parameters, then the fixing of the bug. I suggested this in my first review: separate out and keep the fix portion of the series minimal. Thanks, Ingo