| Message ID | 20241014141345.5680-1-david.hunter.linux@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | streamline_config.pl: fix: process configs set to "y" | expand |
On Mon, 14 Oct 2024 10:13:30 -0400 David Hunter <david.hunter.linux@gmail.com> wrote: > An assumption made in this script is that the config options do not need > to be processed because they will simply be in the new config file. This > assumption is incorrect. > > Process the config entries set to "y" because those config entries might > have dependencies set to "m". If a config entry is set to "m" and is not > loaded directly into the machine, the script will currently turn off > that config entry; however, if that turned off config entry is a > dependency for a "y" option. that means the config entry set to "y" > will also be turned off later when the conf executive file is called. > > Here is a model of the problem (arrows show dependency): > > Original config file > Config_1 (m) <-- Config_2 (y) > > Config_1 is not loaded in this example, so it is turned off. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) > > > It should also be noted that any module in the dependency chain will > also be turned off, even if that module is loaded directly onto the > computer. Here is an example: > > Original config file > Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) > > Config_3 will be loaded in this example. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) <-- Config_3 (m) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) <-- Config_3 (n) > > > I discovered this problem when I ran "make localmodconfig" on a generic > Ubuntu config file. Many hardware devices were not recognized once the > kernel was installed and booted. Another way to reproduced the error I > had is to run "make localmodconfig" twice. The standard error might display > warnings that certain modules should be selected but no config files are > turned on that select that module. > > With the changes in this series patch, all modules are loaded properly > and all of the hardware is loaded when the kernel is installed and > booted. > So I tried this out and compared it to what it use to do, and it was quite a big difference! Attached is the diffconfig. I copied over my special config, ran localmodconfig, saved it, added your patches, copied over the same config, and then did the diffconfig between the two results. There's a lot I know I don't use that was added! For example: BCACHEFS_FS n -> m Why is that needed? I can give you the starting config as well as my lsmod. But right now, it's adding too much that is not needed. -- Steve
On Thu, 17 Oct 2024 18:37:11 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > I can give you the starting config as well as my lsmod. But right now, it's > adding too much that is not needed. Attached is the config I started with and the lsmod used. You could copy the config to .config and then run: make LSMOD=lsmod localmodconfig to test with. -- Steve
From: David.hunter.linux@gmail.com I think that there are 2 distinct reasons that a module should not be turned off when using the 'localmodconfig' command. 1) the module is used directly 2) the module is needed for another module that has been included in the kernel with a "y". I think we both agree on the first reason, but we may be having a disagreement/miscommunication for reason 2. > For example: > > BCACHEFS_FS n -> m > > Why is that needed? With regards to your question, I see that 'CLOSURES' is set to 'y' in your original .config file. 'BCACHEFS_FS' selects 'CLOSURES'. I suspect that if 'BCACHEFS_FS' is set to 'n', then your config would have no way of keeping 'CLOSURES' as 'y'. I understand that the following patch is not suitable for upstream; however, the following patch might help you to understand a little bit more about each config options like 'BCACHEFS_FS'. https://lore.kernel.org/all/20241014141345.5680-5-david.hunter.linux@gmail.com/ If you put in dprintvar("BCACHEFS_FS); you would be able to see what selects 'BCACHEFS_FS' and what is selected by 'BCACHEFS_FS'. I suspect that if you were to use it for each of the config options that you have questions about, you would likely see that each config option set to "m" has a "y" option that it selects. The question then would be is the "y" option actually required for your particular machine. This brings us to the root cause of the issue that we are having: there is no way to know if a config option set to 'y' is actually required. If there was a way to tell if 'CLOSURES' is needed, we can easily determine whether BCACHEFS_FS is actually necessary. Without knowing whether the 'y' options are needed, we then have to make a determination whether it is better to have the (potentially necessary) 'y' options with the extra (not directly used) 'm' options or to drop those 'y' options along with those 'm' options. My argument is that the 'y' options are important, even if we cannot determine whether they are used or not. The problem I had that made me aware of the issues with localmodconfig was that my new computer would consistently do an emergency shutdown whenever I would try to compile the kernel. I eventually realized that the fan was not being recognized, so my workaround was to put the BIOS in control of the fan (instead of the kernel). It was not until I realized that I had a few hardware devices that were not being recognized that I was able to pinpoint the root cause of the problem. For clarity, some of my hardware devices that were nonfunctional were USBs, the microphone, Bluetooth (and as mentioned earlier, the fan). I am curious to know if there are any hardware devices that are not recognized on your computer after you use localmodconfig. One way to check is to use use 'ls' on the psuedofilesystem or to use any of the commands (like lspci) that recognize hardware. I do not know whether any of your hardware requires the 'CLOSURES' config, but because someone's hardware MAY need it, my reasoning is that streamline_config.pl should include 'BCACHEFS_FS' if it is the only thing that selects another config opiton set to 'y'. On my computer, when all of these configs set to 'm' are added (because of reason 2), all of my hardware works (including the Bluetooth, microphone, and USBs. If, on the other hand, you still feel like it would be better to not include these particular config options, I would ask that you allow a command line option like '-s' or '--safe' that users could use to make the config file with the added modules.
On Wed, 27 Nov 2024 18:05:47 -0500 David Hunter <david.hunter.linux@gmail.com> wrote: > From: David.hunter.linux@gmail.com > > I think that there are 2 distinct reasons that a module should not be > turned off when using the 'localmodconfig' command. > > 1) the module is used directly > 2) the module is needed for another module that has been included in the > kernel with a "y". > > I think we both agree on the first reason, but we may be having a > disagreement/miscommunication for reason 2. Note, Linus uses make localmodconfig a lot. He didn't always, because he complained that it would keep too many options enabled. I don't want to suddenly have Linus's make localmodconfig blow up the options again. I can see him complaining about it. Believe me, you don't want Linus to notice this patch for the wrong reasons. This is why I'm very concerned about things like BCACHEFS being enabled after localmodconfig when there's obviously no reason for it. Even if it enables CLOSURES. BCACHEFS is an filesystem that I don't use. It should not be enabled on my machines. > > > For example: > > > > BCACHEFS_FS n -> m > > > > Why is that needed? > > With regards to your question, I see that 'CLOSURES' is set to 'y' in > your original .config file. 'BCACHEFS_FS' selects 'CLOSURES'. I suspect > that if 'BCACHEFS_FS' is set to 'n', then your config would have no way > of keeping 'CLOSURES' as 'y'. Which is OK for this example. > > I understand that the following patch is not suitable for upstream; > however, the following patch might help you to understand a little bit > more about each config options like 'BCACHEFS_FS'. > > https://lore.kernel.org/all/20241014141345.5680-5-david.hunter.linux@gmail.com/ Note, I'll try to remember to look at this after the Thanksgiving break. > > If you put in > > dprintvar("BCACHEFS_FS); > > you would be able to see what selects 'BCACHEFS_FS' and what is selected > by 'BCACHEFS_FS'. I suspect that if you were to use it for each of the > config options that you have questions about, you would likely see that > each config option set to "m" has a "y" option that it selects. The > question then would be is the "y" option actually required for your > particular machine. I would argue that if a 'y' depends on a 'm' then it's not needed unless that 'y' is a dependency for other 'm's that are needed. We should look into seeing if that's the case. > > This brings us to the root cause of the issue that we are having: there > is no way to know if a config option set to 'y' is actually required. If > there was a way to tell if 'CLOSURES' is needed, we can easily determine > whether BCACHEFS_FS is actually necessary. I would say that CLOSURES is considered needed if a 'm' that is needed depends on it. Otherwise, I would say no. > > Without knowing whether the 'y' options are needed, we then have to make > a determination whether it is better to have the (potentially necessary) > 'y' options with the extra (not directly used) 'm' options or to drop > those 'y' options along with those 'm' options. > > My argument is that the 'y' options are important, even if we cannot > determine whether they are used or not. The problem I had that made me > aware of the issues with localmodconfig was that my new computer would > consistently do an emergency shutdown whenever I would try to compile the > kernel. I eventually realized that the fan was not being recognized, so > my workaround was to put the BIOS in control of the fan (instead of the > kernel). > > It was not until I realized that I had a few hardware devices that were > not being recognized that I was able to pinpoint the root cause of the > problem. For clarity, some of my hardware devices that were nonfunctional > were USBs, the microphone, Bluetooth (and as mentioned earlier, the fan). Can you show the dependency change for those devices. Knowing what exactly was disabled and knowing what was needed would be useful. That way we can fix the actual problem without using a big hammer approach of just keeping any module enabled that enables a 'y'. > > I am curious to know if there are any hardware devices that are not > recognized on your computer after you use localmodconfig. One way to > check is to use use 'ls' on the psuedofilesystem or to use any of the > commands (like lspci) that recognize hardware. > > I do not know whether any of your hardware requires the 'CLOSURES' > config, but because someone's hardware MAY need it, my reasoning is that > streamline_config.pl should include 'BCACHEFS_FS' if it is the only thing > that selects another config opiton set to 'y'. BACHEFS is a new file system. If there's a dependency in my config that requires it, that's a bug in the config. > > On my computer, when all of these configs set to 'm' are added (because > of reason 2), all of my hardware works (including the Bluetooth, > microphone, and USBs. > > If, on the other hand, you still feel like it would be better to not > include these particular config options, I would ask that you allow a > command line option like '-s' or '--safe' that users could use to make > the config file with the added modules. Let's first find out what the problem is. Remember, this is code that Linus uses for his builds. Rule #1 I was told when I first became a kernel developer was, "Whatever you do. Don't break Linus's workstation!" -- Steve
An assumption made in this script is that the config options do not need to be processed because they will simply be in the new config file. This assumption is incorrect. Process the config entries set to "y" because those config entries might have dependencies set to "m". If a config entry is set to "m" and is not loaded directly into the machine, the script will currently turn off that config entry; however, if that turned off config entry is a dependency for a "y" option. that means the config entry set to "y" will also be turned off later when the conf executive file is called. Here is a model of the problem (arrows show dependency): Original config file Config_1 (m) <-- Config_2 (y) Config_1 is not loaded in this example, so it is turned off. After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf Config_1 (n) <-- Config_2 (y) After scripts/kconfig/conf Config_1 (n) <-- Config_2 (n) It should also be noted that any module in the dependency chain will also be turned off, even if that module is loaded directly onto the computer. Here is an example: Original config file Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) Config_3 will be loaded in this example. After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf Config_1 (n) <-- Config_2 (y) <-- Config_3 (m) After scripts/kconfig/conf Config_1 (n) <-- Config_2 (n) <-- Config_3 (n) I discovered this problem when I ran "make localmodconfig" on a generic Ubuntu config file. Many hardware devices were not recognized once the kernel was installed and booted. Another way to reproduced the error I had is to run "make localmodconfig" twice. The standard error might display warnings that certain modules should be selected but no config files are turned on that select that module. With the changes in this series patch, all modules are loaded properly and all of the hardware is loaded when the kernel is installed and booted. David Hunter (7): streamline_config.pl: fix missing variable operator in debug print streamline_config.pl: ensure all defaults are tracked streamline_config.pl: remove prompt warnings for configs with defaults streamline_config.pl: include tool to learn about a config option streamline_config.pl: fix: implement choice for kconfigs streamline_config.pl: process config options set to "y" streamline_config.pl: check prompt for bool --- V1: https://lore.kernel.org/all/20240913171205.22126-1-david.hunter.linux@gmail.com/ V2: https://lore.kernel.org/all/20240916142939.754911-1-david.hunter.linux@gmail.com/ - Put in subject. V3: - changed the order of patches - removed a patch that was unneccessary - added a patch for a debugging tool --- scripts/kconfig/streamline_config.pl | 130 ++++++++++++++++++++++++--- 1 file changed, 119 insertions(+), 11 deletions(-)