From: Dave Martin <Dave.Martin@arm.com> To: Arnd Bergmann <firstname.lastname@example.org> Cc: "Catalin Marinas" <Catalin.Marinas@arm.com>, "Will Deacon" <Will.Deacon@arm.com>, "Luis R. Rodriguez" <email@example.com>, "Kees Cook" <firstname.lastname@example.org>, "Alex Bennée" <email@example.com>, "Eric W. Biederman" <firstname.lastname@example.org>, "Andrew Morton" <email@example.com>, "Thomas Gleixner" <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 Date: Mon, 6 Nov 2017 17:41:29 +0000 [thread overview] Message-ID: <20171106174128.GW19485@e103592.cambridge.arm.com> (raw) In-Reply-To: <CAK8P3a25CP-FgezD1S-8kNMCZF3CGraKAvDdb6KaV6HJXATZJw@mail.gmail.com> On Mon, Nov 06, 2017 at 05:12:03PM +0000, Arnd Bergmann wrote: > 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. Well, since your patch makes things more consistent, I'm happy to Reviewed-by: Dave Martin <Dave.Martin@arm.com> The armv8_deprecated code looks suspicious in any case, so I'll propose a fix for that separately -- adding a Kconfig dependency seems simplest. Cheers ---Dave
next prev parent reply other threads:[~2017-11-06 17:41 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 2017-11-06 17:41 ` Dave Martin [this message] 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=20171106174128.GW19485@e103592.cambridge.arm.com \ --email@example.com \ --cc=Catalin.Marinas@arm.com \ --cc=Will.Deacon@arm.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 \ --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.