| Message ID | 20180129234900.11121-1-anders.roxell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 30, 2018 at 12:49:00AM +0100, Anders Roxell wrote: > While testing multi_v7_defconfig with LOCKDEP enabled, the kernel > fails to load simple modules, as reported by kselftest: > > [ 34.107620] test_printf: section 4 reloc 2 sym 'memset': relocation > 28 out of range (0xbf046044 -> 0xc109f720) > selftests: printf.sh [FAIL] > > The problem that is seen when LOCKDEP is enabled without > ARM_MODULE_PLTS, is that LOCKDEP eats so much memory that the top of the > kernel gets out of reach from the bottom of the module area. This really doesn't follow IMHO - enabling various features can cause this, and we're not going to end up stuffing the Kconfig full of these select statements each time we find a combination of Kconfig symbols that cause it. lockdep isn't that special - I can (and do) build kernels with lockdep enabled, with modules, and I do not run into this problem. So it's not as simple as you make out in this commit description. It's likely that you have either a fairly full kernel configuration (it must be to place memset() more than 16MB) or you are not placing the kernel at 0xc0008000 due to memory reservations in the low memory. > Suggested-by: Arnd Bergmann <arnd@arndb.de> I guess this was discussed privately with Arnd, since there's no record of the discussion on the lists - which is even more reason why the commit message needs to describe better why you need this change. > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- > arch/arm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 51c8df561077..a339157497c6 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -20,6 +20,7 @@ config ARM > select ARCH_USE_BUILTIN_BSWAP > select ARCH_USE_CMPXCHG_LOCKREF > select ARCH_WANT_IPC_PARSE_VERSION > + select ARM_MODULE_PLTS if LOCKDEP > select BUILDTIME_EXTABLE_SORT if MMU > select CLONE_BACKWARDS > select CPU_PM if (SUSPEND || CPU_IDLE) > -- > 2.11.0 >
Hi Anders,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Anders-Roxell/arch-arm-Kconfig-enable-ARM_MODULE_PLTS-when-LOCKDEP-y/20180131-115917
config: arm-moxart_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All warnings (new ones prefixed by >>):
warning: (ARM) selects ARM_MODULE_PLTS which has unmet direct dependencies (MODULES)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Jan 30, 2018 at 12:57 AM, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Tue, Jan 30, 2018 at 12:49:00AM +0100, Anders Roxell wrote: >> While testing multi_v7_defconfig with LOCKDEP enabled, the kernel >> fails to load simple modules, as reported by kselftest: >> >> [ 34.107620] test_printf: section 4 reloc 2 sym 'memset': relocation >> 28 out of range (0xbf046044 -> 0xc109f720) >> selftests: printf.sh [FAIL] >> >> The problem that is seen when LOCKDEP is enabled without >> ARM_MODULE_PLTS, is that LOCKDEP eats so much memory that the top of the >> kernel gets out of reach from the bottom of the module area. > > This really doesn't follow IMHO - enabling various features can cause > this, and we're not going to end up stuffing the Kconfig full of these > select statements each time we find a combination of Kconfig symbols > that cause it. > > lockdep isn't that special - I can (and do) build kernels with lockdep > enabled, with modules, and I do not run into this problem. So it's > not as simple as you make out in this commit description. > > It's likely that you have either a fairly full kernel configuration (it > must be to place memset() more than 16MB) or you are not placing the > kernel at 0xc0008000 due to memory reservations in the low memory. > >> Suggested-by: Arnd Bergmann <arnd@arndb.de> > > I guess this was discussed privately with Arnd, since there's no record > of the discussion on the lists - which is even more reason why the > commit message needs to describe better why you need this change. I don't remember the original discussion exactly, but it's clear that the addition of LOCKDEP to the build is what caused the size to grow dramatically and prevent module loading. There are two alternative ways to do this without forcing ARM_MODULE_PLTS on all the time (which also triggered the 0day bot warning): a) we could make ARM_MODULE_PLTS default to 'y' when LOCKDEP is anbled, making it a more reasonable default while also letting users turn it off when the lockdep-enabled kernel is still small enough b) The Kconfig fragment that Anders uses which turns on LOCKDEP could simply enable ARM_MODULE_PLTS as well. That would be the easiest solution for him. I assume he already does it, but it's likely that others run into the same issue with kselftest testing. Arnd
On Wed, Jan 31, 2018 at 12:25:33PM +0100, Arnd Bergmann wrote: > On Tue, Jan 30, 2018 at 12:57 AM, Russell King - ARM Linux > <linux@armlinux.org.uk> wrote: > > On Tue, Jan 30, 2018 at 12:49:00AM +0100, Anders Roxell wrote: > >> While testing multi_v7_defconfig with LOCKDEP enabled, the kernel > >> fails to load simple modules, as reported by kselftest: > >> > >> [ 34.107620] test_printf: section 4 reloc 2 sym 'memset': relocation > >> 28 out of range (0xbf046044 -> 0xc109f720) > >> selftests: printf.sh [FAIL] > >> > >> The problem that is seen when LOCKDEP is enabled without > >> ARM_MODULE_PLTS, is that LOCKDEP eats so much memory that the top of the > >> kernel gets out of reach from the bottom of the module area. > > > > This really doesn't follow IMHO - enabling various features can cause > > this, and we're not going to end up stuffing the Kconfig full of these > > select statements each time we find a combination of Kconfig symbols > > that cause it. > > > > lockdep isn't that special - I can (and do) build kernels with lockdep > > enabled, with modules, and I do not run into this problem. So it's > > not as simple as you make out in this commit description. > > > > It's likely that you have either a fairly full kernel configuration (it > > must be to place memset() more than 16MB) or you are not placing the > > kernel at 0xc0008000 due to memory reservations in the low memory. > > > >> Suggested-by: Arnd Bergmann <arnd@arndb.de> > > > > I guess this was discussed privately with Arnd, since there's no record > > of the discussion on the lists - which is even more reason why the > > commit message needs to describe better why you need this change. > > I don't remember the original discussion exactly, but it's clear that > the addition of LOCKDEP to the build is what caused the size to > grow dramatically and prevent module loading. As I say above, it does not follow "lockdep is enabled" therefore "we need module plts" so I don't accept this argument. Yes, if you have a lot of features built in, then its entirely possible that enabling lockdep will cause the kernel to overflow the non-plt boundary. It will also be true that enabling a few more other features will also cause the kernel to overflow the non-plt boundary as well. Should we add those other features to a Kconfig expression too? At what point do we stop adding these? > There are two alternative ways to do this without forcing > ARM_MODULE_PLTS on all the time (which also triggered > the 0day bot warning): Yes, 0day is pointing out yet again what a silly idea it is to select symbols that (a) have dependencies and (b) are user visible, something that I've long disagreed about. > a) we could make ARM_MODULE_PLTS default to 'y' when > LOCKDEP is anbled, making it a more reasonable default while > also letting users turn it off when the lockdep-enabled kernel > is still small enough As I've said, I don't believe "LOCKDEP" therefore need "MODULE_PLTS" follows. It's just a symptom of an already large kernel. I suspect without lockdep, Ander's kernel is already approaching the problematical 16MB mark. Another option that causes the kernel to grow by a few megabytes is the kernel protection options (ronx etc). I suspect if Anders built a kernel that had lockdep enabled and ronx etc disabled, that there would be no need for module plts. Should we turn on module plts if lockdep or ronx is enabled? I don't think there's a _sane_ solution to this other than defaulting ARM_MODULE_PLTS to 'y' without any of these dependencies, and if people want to turn it off, they still can if they're sure they won't run into this situation.
Hi Anders,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Anders-Roxell/arch-arm-Kconfig-enable-ARM_MODULE_PLTS-when-LOCKDEP-y/20180131-115917
config: arm-moxart_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
arch/arm/kernel/module-plts.c: In function 'in_init':
>> arch/arm/kernel/module-plts.c:36:23: error: dereferencing pointer to incomplete type 'const struct module'
return loc - (u32)mod->init_layout.base < mod->init_layout.size;
^~
arch/arm/kernel/module-plts.c: In function 'get_module_plt':
>> arch/arm/kernel/module-plts.c:41:56: error: dereferencing pointer to incomplete type 'struct module'
struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
^~
vim +36 arch/arm/kernel/module-plts.c
7d485f64 Ard Biesheuvel 2014-11-24 33
b7ede5a1 Ard Biesheuvel 2017-02-22 34 static bool in_init(const struct module *mod, unsigned long loc)
b7ede5a1 Ard Biesheuvel 2017-02-22 35 {
b7ede5a1 Ard Biesheuvel 2017-02-22 @36 return loc - (u32)mod->init_layout.base < mod->init_layout.size;
b7ede5a1 Ard Biesheuvel 2017-02-22 37 }
b7ede5a1 Ard Biesheuvel 2017-02-22 38
7d485f64 Ard Biesheuvel 2014-11-24 39 u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val)
7d485f64 Ard Biesheuvel 2014-11-24 40 {
b7ede5a1 Ard Biesheuvel 2017-02-22 @41 struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
b7ede5a1 Ard Biesheuvel 2017-02-22 42 &mod->arch.init;
b7ede5a1 Ard Biesheuvel 2017-02-22 43
b7ede5a1 Ard Biesheuvel 2017-02-22 44 struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr;
66e94ba3 Ard Biesheuvel 2016-08-18 45 int idx = 0;
35fa91ee Ard Biesheuvel 2016-08-16 46
66e94ba3 Ard Biesheuvel 2016-08-18 47 /*
66e94ba3 Ard Biesheuvel 2016-08-18 48 * Look for an existing entry pointing to 'val'. Given that the
66e94ba3 Ard Biesheuvel 2016-08-18 49 * relocations are sorted, this will be the last entry we allocated.
66e94ba3 Ard Biesheuvel 2016-08-18 50 * (if one exists).
66e94ba3 Ard Biesheuvel 2016-08-18 51 */
b7ede5a1 Ard Biesheuvel 2017-02-22 52 if (pltsec->plt_count > 0) {
b7ede5a1 Ard Biesheuvel 2017-02-22 53 plt += (pltsec->plt_count - 1) / PLT_ENT_COUNT;
b7ede5a1 Ard Biesheuvel 2017-02-22 54 idx = (pltsec->plt_count - 1) % PLT_ENT_COUNT;
7d485f64 Ard Biesheuvel 2014-11-24 55
66e94ba3 Ard Biesheuvel 2016-08-18 56 if (plt->lit[idx] == val)
66e94ba3 Ard Biesheuvel 2016-08-18 57 return (u32)&plt->ldr[idx];
66e94ba3 Ard Biesheuvel 2016-08-18 58
66e94ba3 Ard Biesheuvel 2016-08-18 59 idx = (idx + 1) % PLT_ENT_COUNT;
66e94ba3 Ard Biesheuvel 2016-08-18 60 if (!idx)
66e94ba3 Ard Biesheuvel 2016-08-18 61 plt++;
66e94ba3 Ard Biesheuvel 2016-08-18 62 }
66e94ba3 Ard Biesheuvel 2016-08-18 63
b7ede5a1 Ard Biesheuvel 2017-02-22 64 pltsec->plt_count++;
b7ede5a1 Ard Biesheuvel 2017-02-22 65 BUG_ON(pltsec->plt_count * PLT_ENT_SIZE > pltsec->plt->sh_size);
7d485f64 Ard Biesheuvel 2014-11-24 66
66e94ba3 Ard Biesheuvel 2016-08-18 67 if (!idx)
7d485f64 Ard Biesheuvel 2014-11-24 68 /* Populate a new set of entries */
7d485f64 Ard Biesheuvel 2014-11-24 69 *plt = (struct plt_entries){
7d485f64 Ard Biesheuvel 2014-11-24 70 { [0 ... PLT_ENT_COUNT - 1] = PLT_ENT_LDR, },
7d485f64 Ard Biesheuvel 2014-11-24 71 { val, }
7d485f64 Ard Biesheuvel 2014-11-24 72 };
66e94ba3 Ard Biesheuvel 2016-08-18 73 else
66e94ba3 Ard Biesheuvel 2016-08-18 74 plt->lit[idx] = val;
66e94ba3 Ard Biesheuvel 2016-08-18 75
66e94ba3 Ard Biesheuvel 2016-08-18 76 return (u32)&plt->ldr[idx];
7d485f64 Ard Biesheuvel 2014-11-24 77 }
7d485f64 Ard Biesheuvel 2014-11-24 78
:::::: The code at line 36 was first introduced by commit
:::::: b7ede5a1f5905ac394cc8e61712a13e3c5cb7b8f ARM: 8662/1: module: split core and init PLT sections
:::::: TO: Ard Biesheuvel <ard.biesheuvel@linaro.org>
:::::: CC: Russell King <rmk+kernel@armlinux.org.uk>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 31 January 2018 at 12:46, Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > On Wed, Jan 31, 2018 at 12:25:33PM +0100, Arnd Bergmann wrote: >> On Tue, Jan 30, 2018 at 12:57 AM, Russell King - ARM Linux >> <linux@armlinux.org.uk> wrote: >> > On Tue, Jan 30, 2018 at 12:49:00AM +0100, Anders Roxell wrote: >> >> While testing multi_v7_defconfig with LOCKDEP enabled, the kernel >> >> fails to load simple modules, as reported by kselftest: >> >> >> >> [ 34.107620] test_printf: section 4 reloc 2 sym 'memset': relocation >> >> 28 out of range (0xbf046044 -> 0xc109f720) >> >> selftests: printf.sh [FAIL] >> >> >> >> The problem that is seen when LOCKDEP is enabled without >> >> ARM_MODULE_PLTS, is that LOCKDEP eats so much memory that the top of the >> >> kernel gets out of reach from the bottom of the module area. >> > >> > This really doesn't follow IMHO - enabling various features can cause >> > this, and we're not going to end up stuffing the Kconfig full of these >> > select statements each time we find a combination of Kconfig symbols >> > that cause it. >> > >> > lockdep isn't that special - I can (and do) build kernels with lockdep >> > enabled, with modules, and I do not run into this problem. So it's >> > not as simple as you make out in this commit description. >> > >> > It's likely that you have either a fairly full kernel configuration (it >> > must be to place memset() more than 16MB) or you are not placing the >> > kernel at 0xc0008000 due to memory reservations in the low memory. >> > >> >> Suggested-by: Arnd Bergmann <arnd@arndb.de> >> > >> > I guess this was discussed privately with Arnd, since there's no record >> > of the discussion on the lists - which is even more reason why the >> > commit message needs to describe better why you need this change. >> >> I don't remember the original discussion exactly, but it's clear that >> the addition of LOCKDEP to the build is what caused the size to >> grow dramatically and prevent module loading. > > As I say above, it does not follow "lockdep is enabled" therefore "we > need module plts" so I don't accept this argument. > > Yes, if you have a lot of features built in, then its entirely possible > that enabling lockdep will cause the kernel to overflow the non-plt > boundary. It will also be true that enabling a few more other features > will also cause the kernel to overflow the non-plt boundary as well. > Should we add those other features to a Kconfig expression too? At > what point do we stop adding these? > >> There are two alternative ways to do this without forcing >> ARM_MODULE_PLTS on all the time (which also triggered >> the 0day bot warning): > > Yes, 0day is pointing out yet again what a silly idea it is to select > symbols that (a) have dependencies and (b) are user visible, something > that I've long disagreed about. > >> a) we could make ARM_MODULE_PLTS default to 'y' when >> LOCKDEP is anbled, making it a more reasonable default while >> also letting users turn it off when the lockdep-enabled kernel >> is still small enough > > As I've said, I don't believe "LOCKDEP" therefore need "MODULE_PLTS" > follows. It's just a symptom of an already large kernel. I suspect > without lockdep, Ander's kernel is already approaching the > problematical 16MB mark. > > Another option that causes the kernel to grow by a few megabytes is > the kernel protection options (ronx etc). I suspect if Anders built > a kernel that had lockdep enabled and ronx etc disabled, that there > would be no need for module plts. Should we turn on module plts if > lockdep or ronx is enabled? > > I don't think there's a _sane_ solution to this other than defaulting > ARM_MODULE_PLTS to 'y' without any of these dependencies, and if people > want to turn it off, they still can if they're sure they won't run into > this situation. I'll send out a v2 with this change shortly. Thank you all for your comments. Cheers, Anders
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 51c8df561077..a339157497c6 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -20,6 +20,7 @@ config ARM select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_CMPXCHG_LOCKREF select ARCH_WANT_IPC_PARSE_VERSION + select ARM_MODULE_PLTS if LOCKDEP select BUILDTIME_EXTABLE_SORT if MMU select CLONE_BACKWARDS select CPU_PM if (SUSPEND || CPU_IDLE)
While testing multi_v7_defconfig with LOCKDEP enabled, the kernel fails to load simple modules, as reported by kselftest: [ 34.107620] test_printf: section 4 reloc 2 sym 'memset': relocation 28 out of range (0xbf046044 -> 0xc109f720) selftests: printf.sh [FAIL] The problem that is seen when LOCKDEP is enabled without ARM_MODULE_PLTS, is that LOCKDEP eats so much memory that the top of the kernel gets out of reach from the bottom of the module area. Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+)