linux-kernel - Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a3QNjOSodb-ND=nZ=ThC9xZhEyUOaUsWCDF70s9g80r-w@mail.gmail.com>
Date:   Mon, 25 Jul 2022 16:22:24 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     John Garry <john.garry@...wei.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Olof Johansson <olof@...om.net>, SoC Team <soc@...nel.org>,
        jpoimboe@...nel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/2] arm64 defconfig: Get faddr2line working

On Mon, Jul 25, 2022 at 4:03 PM John Garry <john.garry@...wei.com> wrote:
> >
> >>> CONFIG_MAILBOX=y
> >> In commit fc739069aa92, config MAILBOX was enabled in the defconfig but
> >> it was already being enabled from elsewhere. There was definitely no
> >> sync of the savedefconfig going on there:)
> > I see it's selected by a couple of drivers using mailboxes, and I
> > agree we shouldn't
> > need it here. It might be good to just hide the symbol in this case
> > and select it
> > consistently from all drivers using it.
>
> Uhh, we have ~15x "select" and ~18x "depends on"...

> >>> CONFIG_INTERCONNECT=y
> >> Since commit 06f079816d4c, config TEGRA_MC added a kconfig select on
> >> config INTERCONNECT, so no need to explicitly enable from the kconfig
> > We have one driver using 'depends on INTERCONNECT' and two drivers
> > using 'select INTERCONNECT', so at least one of them is wrong.
>
> INTERCONNECT has no dependencies, so "select" - like for MAILBOX -
> should be fine, I suppose

There are a couple of trade-offs between the two approaches.
The main advantage of 'select' is that you can enable drivers more
easily and all the required subsystems are there automatically.
The advantage of 'depends on' is that it becomes easier to disable
entire subsystems that one may not need.

Which of those two is more important is of course a matter of perspective,
I like to be able to turn things off more easily because that makes it
possible to test the corner cases with randconfig more easily, and it
helps produce size-reduced kernels for embedded systems.

Another aspect is that we overall have more 'depends on' than 'select',
and sticking with the more common way avoids circular dependencies,
both within an area of the kernel and overall.

The rule that I tend to follow with 'select' is to only use it on symbols
that you don't even want to show to users. If a feature is part of
a library (think zlib), then each user just needs to select the symbol
but you never actually have to decide whether to show it or not.

> >> And would each config item deletion merit a separate patch?
> > You send a combined patch for the obvious ones (secccomp
> > and mailbox AFAICT) or send them separately. For the other ones I think
> > we should try fixing the Kconfig files first, otherwise we just end up
> > putting them back afterwards.
>
> ok, fine. I'll deal with the obvious changes first plus
> CONFIG_DEBUG_INFO and then the non-obvious, non-trivial ones. I'll base
> on your arm/defconfig branch (for defconfig changes).

The CONFIG_DEBUG_INFO one should be fixed by my series from
last week already, do you still see another issue with that? I actually
have another patch to fix up all the non-Arm defconfigs for this one as
well, but haven't sent that one yet.

     Arnd