Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751780AbdHLOKL (ORCPT ); Sat, 12 Aug 2017 10:10:11 -0400 Received: from mail-wr0-f196.google.com ([209.85.128.196]:34876 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750948AbdHLOKJ (ORCPT ); Sat, 12 Aug 2017 10:10:09 -0400 Date: Sat, 12 Aug 2017 16:10:06 +0200 From: Frederic Weisbecker To: Luiz Capitulino Cc: LKML , Peter Zijlstra , Chris Metcalf , Thomas Gleixner , Christoph Lameter , "Paul E . McKenney" , Ingo Molnar , Mike Galbraith , Rik van Riel , Wanpeng Li Subject: Re: [RFC PATCH 7/9] housekeeping: Use own boot option, independant from nohz Message-ID: <20170812141004.GA21918@lerouge> References: <1500643290-25842-1-git-send-email-fweisbec@gmail.com> <1500643290-25842-8-git-send-email-fweisbec@gmail.com> <20170811123927.33e094f3@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170811123927.33e094f3@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3226 Lines: 71 On Fri, Aug 11, 2017 at 03:09:57PM -0400, Luiz Capitulino wrote: > On Fri, 21 Jul 2017 15:21:28 +0200 > Frederic Weisbecker wrote: > > -void __init housekeeping_init(void) > > +/* Parse the boot-time housekeeping CPU list from the kernel parameters. */ > > +static int __init housekeeping_setup(char *str) > > { > > - if (!tick_nohz_full_enabled()) > > - return; > > - > > - if (!alloc_cpumask_var(&housekeeping_mask, GFP_KERNEL)) { > > - WARN(1, "NO_HZ: Can't allocate not-full dynticks cpumask\n"); > > - cpumask_clear(tick_nohz_full_mask); > > - tick_nohz_full_running = false; > > - return; > > + alloc_bootmem_cpumask_var(&housekeeping_mask); > > + if (cpulist_parse(str, housekeeping_mask) < 0) { > > + pr_warn("Housekeeping: Incorrect cpumask\n"); > > + free_bootmem_cpumask_var(housekeeping_mask); > > + return 1; > > } > > > > - cpumask_andnot(housekeeping_mask, > > - cpu_possible_mask, tick_nohz_full_mask); > > - > > static_branch_enable(&housekeeping_overriden); > > > > /* We need at least one CPU to handle housekeeping work */ > > WARN_ON_ONCE(cpumask_empty(housekeeping_mask)); > > + > > + return 1; > > } > > +__setup("housekeeping=", housekeeping_setup); > > Am I right that from now on nohz_full= users will also have > to specify housekeeping= in order to get nohz_full working? > If that's correct, then won't this patch break nohz_full for > existing setups? nohz_full= will still work but will only imply tick stop. A few isolation details that were enabled by nohz_full= won't be handled anymore such as: unbound timers affinity, watchdog disablement, rcu threads affinity, sched idle load balancing... Those are now handled by housekeeping= So yes in a sense, this can break some setup that assume nohz_full= does more than stopping the tick. Perhaps I should remove the nohz_full= parameter altogether and let nohz_full controlled by housekeeping= only. How much can kernel parameters be considered as kernel ABIs? Also I'm wondering if "housekeeping=" is a clear name for users. "isolation=" or "cpu_isolation=" would be better and more obvious. Housekeeping based naming would only be internal implementation detail. And deactivating the tick through "cpu_isolation=" would be clearer than if we did through "housekeeping=". Of course the problem is that we already have "isolcpus=". But re-implementing isolcpus on top of housekeeping might be a good idea. I believe that the current implementation on top of NULL domains isn't much beloved. A less controversial implementation might even allow us to control it though cpusets. > > Also, I just give this series a try and got this: > > [ 0.000000] Kernel command line: BOOT_IMAGE=/vmlinuz-4.13.0-rc4+ root=/dev/mapper/rhel_virtlab508-root ro crashkernel=auto rd.lvm.lv=rhel_virtlab508/root rd.lvm.lv=rhel_virtlab508/swap console=ttyS1,115200 LANG=en_US.UTF-8 housekeeping=0,2,4,6,8,10,12,14,1 isolcpus=15 nohz_full=15 intel_pstate=disable > [ 0.000000] static_key_slow_inc used before call to jump_label_init > [ 0.000000] ------------[ cut here ]------------ > [ 0.000000] WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:108 static_key_slow_inc+0x86/0xa0 Oops ^_^ Thanks.