Message-ID: <f99ffea3-598a-4ac9-52ff-b1501a6cfc04@gmail.com>
Date:   Wed, 1 May 2019 09:43:42 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Robin Murphy <robin.murphy@....com>,
        Mark Rutland <mark.rutland@....com>
Cc:     Rob Herring <robh@...nel.org>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Julien Thierry <julien.thierry@....com>,
        Will Deacon <will.deacon@....com>,
        open list <linux-kernel@...r.kernel.org>,
        rmk+kernel@...linux.org.uk, Sudeep Holla <sudeep.holla@....com>,
        Suzuki K Poulose <Suzuki.Poulose@....com>,
        linux-arm-kernel@...ts.infradead.org,
        Steve Capper <steve.capper@....com>
Subject: Re: [PATCH] arm64: Demote boot and shutdown messages to pr_debug

On 5/1/19 4:02 AM, Robin Murphy wrote:
> On 01/05/2019 11:47, Mark Rutland wrote:
>> On Tue, Apr 30, 2019 at 03:38:31PM -0700, Florian Fainelli wrote:
>>> Similar to commits c68b0274fb3cf ("ARM: reduce "Booted secondary
>>> processor" message to debug level") and 035e787543de7 ("ARM: 8644/1:
>>> Reduce "CPU:
>>> shutdown" message to debug level"), demote the secondary_start_kernel()
>>> and __cpu_die() messages from info, respectively notice to debug. While
>>> we are at it, also do this for cpu_psci_cpu_kill() which is redundant
>>> with __cpu_die().
>>>
>>> This helps improve the amount of possible hotplug cycles by around +50%
>>> on ARCH_BRCMSTB.
>>
>> Could you elaborate on why that matters?
> 
> Yeah, in general if you have a slow serial console then removing all the
> prints from the kernel makes lots of things much faster, but that's not
> necessarily a good argument for doing so. If that's a problem that
> really concerns users then I'd have to ask why they aren't using a
> stricter loglevel or a different console to begin with.

See my response to Mark for specific use cases. Teaching users about
changing their default print levels is certainly an option, although
they will likely start wondering why other messages are now gone as
well. There is not unfortunately a choice of a faster console on those
platforms.

The print levels are not necessarily consistent (info vs. notice) and we
have plenty of messages for when the CPU does not come online so telling
us when it does is completely superfluous and does not bring much value
and just gets in the way of being able to do that more often.

> 
>> e.g. is this just for testing, or does this matter in some shutdown or
>> hibernate scenario?
>>
>>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>>> ---
>>>   arch/arm64/kernel/psci.c | 2 +-
>>>   arch/arm64/kernel/smp.c  | 4 ++--
>>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
>>> index 8cdaf25e99cd..a78581046c80 100644
>>> --- a/arch/arm64/kernel/psci.c
>>> +++ b/arch/arm64/kernel/psci.c
>>> @@ -96,7 +96,7 @@ static int cpu_psci_cpu_kill(unsigned int cpu)
>>>       for (i = 0; i < 10; i++) {
>>>           err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>>>           if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) {
>>> -            pr_info("CPU%d killed.\n", cpu);
>>> +            pr_debug("CPU%d killed.\n", cpu);
>>>               return 0;
>>>           }
>>>   diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>>> index 824de7038967..71fd2b5a3f0e 100644
>>> --- a/arch/arm64/kernel/smp.c
>>> +++ b/arch/arm64/kernel/smp.c
>>> @@ -259,7 +259,7 @@ asmlinkage notrace void secondary_start_kernel(void)
>>>        * the CPU migration code to notice that the CPU is online
>>>        * before we continue.
>>>        */
>>> -    pr_info("CPU%u: Booted secondary processor 0x%010lx [0x%08x]\n",
>>> +    pr_debug("CPU%u: Booted secondary processor 0x%010lx [0x%08x]\n",
>>>                        cpu, (unsigned long)mpidr,
>>>                        read_cpuid_id());
>>
>> I generally agree that we don't need to be verbose, and demoting these
>> to debug is fine, but it's a shame that these won't be accessible in
>> defconfig.
>>
>> I wonder if we should enable DYNAMIC_DEBUG so that we can turn these on
>> from the kernel command line, or if we should have something like a
>> verbose_hotplug option specifically for these messages.
> 
> We've had DYNAMIC_DEBUG=y in defconfig for a while already :/
> 
> Robin.
> 
>>
>> Thanks,
>> Mark.
>>
>>>       update_cpu_boot_status(CPU_BOOT_SUCCESS);
>>> @@ -348,7 +348,7 @@ void __cpu_die(unsigned int cpu)
>>>           pr_crit("CPU%u: cpu didn't die\n", cpu);
>>>           return;
>>>       }
>>> -    pr_notice("CPU%u: shutdown\n", cpu);
>>> +    pr_debug("CPU%u: shutdown\n", cpu);
>>>         /*
>>>        * Now that the dying CPU is beyond the point of no return w.r.t.
>>> -- 
>>> 2.17.1
>>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>


-- 
Florian

Powered by blists - more mailing lists