From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933054AbdKFUgj (ORCPT ); Mon, 6 Nov 2017 15:36:39 -0500 Received: from mail-qt0-f195.google.com ([209.85.216.195]:48287 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932796AbdKFUgi (ORCPT ); Mon, 6 Nov 2017 15:36:38 -0500 X-Google-Smtp-Source: ABhQp+TcJEzrIVbQx9nn8hBL91f8RGun8dgDTgEGtuN8V00Aed99OLEflQn7lmJxMdbd77tGmM4MvkUSnaR3Hdhe1hE= MIME-Version: 1.0 In-Reply-To: <20171106174128.GW19485@e103592.cambridge.arm.com> References: <20171106133700.558647-1-arnd@arndb.de> <20171106165021.GV19485@e103592.cambridge.arm.com> <20171106174128.GW19485@e103592.cambridge.arm.com> From: Arnd Bergmann Date: Mon, 6 Nov 2017 21:36:36 +0100 X-Google-Sender-Auth: 5cJQLkVs8rHlDhctlBDCIhYrQqI Message-ID: Subject: Re: [PATCH] sysctl: add register_sysctl() dummy helper To: Dave Martin Cc: Catalin Marinas , Will Deacon , "Luis R. Rodriguez" , Kees Cook , =?UTF-8?B?QWxleCBCZW5uw6ll?= , "Eric W. Biederman" , Andrew Morton , Thomas Gleixner , "linux-kernel@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 6, 2017 at 6:41 PM, Dave Martin wrote: > On Mon, Nov 06, 2017 at 05:12:03PM +0000, Arnd Bergmann wrote: >> On Mon, Nov 6, 2017 at 5:50 PM, Dave Martin 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 > > > The armv8_deprecated code looks suspicious in any case, so I'll propose > a fix for that separately -- adding a Kconfig dependency seems simplest. Thanks, your other patch looks good too (replied with an Ack there). Andrew, can you pick up my patch into -mm then? It seems we still want it anyway, but there is no longer a dependency with the arm64 tree, so no reason to pick merge it there. Arnd