From: Arnd Bergmann <email@example.com> To: Dave Martin <Dave.Martin@arm.com> Cc: "Catalin Marinas" <Catalin.Marinas@arm.com>, "Will Deacon" <Will.Deacon@arm.com>, "Luis R. Rodriguez" <firstname.lastname@example.org>, "Kees Cook" <email@example.com>, "Alex Bennée" <firstname.lastname@example.org>, "Eric W. Biederman" <email@example.com>, "Andrew Morton" <firstname.lastname@example.org>, "Thomas Gleixner" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com> Subject: Re: [PATCH] sysctl: add register_sysctl() dummy helper Date: Mon, 6 Nov 2017 18:12:03 +0100 [thread overview] Message-ID: <CAK8P3a25CP-FgezD1S-8kNMCZF3CGraKAvDdb6KaV6HJXATZJw@mail.gmail.com> (raw) In-Reply-To: <20171106165021.GV19485@e103592.cambridge.arm.com> On Mon, Nov 6, 2017 at 5:50 PM, Dave Martin <Dave.Martin@arm.com> wrote: > On Mon, Nov 06, 2017 at 01:36:40PM +0000, Arnd Bergmann wrote: >> The register_sysctl() function has been around for five years with commit >> fea478d4101a ("sysctl: Add register_sysctl for normal sysctl users") >> but now that arm64 started using it, I ran into a compile error: >> >> arch/arm64/kernel/armv8_deprecated.c: In function 'register_insn_emulation_sysctl': >> arch/arm64/kernel/armv8_deprecated.c:257:2: error: implicit declaration of function 'register_sysctl' > > Hmmm, looks like I missed this combination in testing. > > I wonder whether ARMV8_DEPRECATED without SYSCTL is really a good idea > though: in that config, we build a lot of dead code and leak some > memory today. The default emulation is still potentially useful, but > all the support for runtime twiddling of the emulation modes becomes > useless. > > For parallel reasons, the SVE sysctl stuff is protected by #ifdef > CONFIG_SYSCTL, which is why I didn't get a similar splat there. > > > So, although this patch is superficially sensible, it may tend to hide > bugs: code that calls register_sysctl() when CONFIG_SYSCTL=n is > suspicious and probably needs review... no? I think your analysis for this code is correct, we waste a lot of memory if we do it like this. However in the general case where we register a statically allocated 'struct ctl_table', it wouldn't be an issue, because gcc could then eliminate all the dead code. Adding a CONFIG_SYSCTL #ifdef or Kconfig dependency would probably be reasonable for armv8_deprecated.c, for the rest of the kernel, having that wrapper is probably better. I don't really care how this gets fixed, as long as some solution gets merged. Arnd
next prev parent reply other threads:[~2017-11-06 17:12 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-06 13:36 Arnd Bergmann 2017-11-06 16:50 ` Dave Martin 2017-11-06 17:12 ` Arnd Bergmann [this message] 2017-11-06 17:41 ` Dave Martin 2017-11-06 20:36 ` Arnd Bergmann 2017-11-06 16:51 ` Kees Cook 2017-11-07 1:01 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAK8P3a25CP-FgezD1S-8kNMCZF3CGraKAvDdb6KaV6HJXATZJw@mail.gmail.com \ --firstname.lastname@example.org \ --cc=Catalin.Marinas@arm.com \ --cc=Dave.Martin@arm.com \ --cc=Will.Deacon@arm.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH] sysctl: add register_sysctl() dummy helper' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.