Making the code cleaner: Recommendations for preparing changes to the Linux kernel
Continuing the theme of improving the Linux kernel code , I want to give a few recommendations based on both life experience and existing documentation.
The first attempts may not succeed, below are some recommendations to reduce the likelihood of such an outcome. I will briefly mention the main areas of problems during the preparation and submission of changes:
Additionally, I will briefly discuss the existing verification mechanisms.
Among all the many subsystems of the kernel, select the following:
The first is an architectural code. It often contains outdated architectures or code that requires careful study and understanding. Changing something there only makes sense if you really have a piece of hardware on which to test, and the real problem you are struggling with.
The second, SCSI, is perhaps the most conservative subsystem in the core, although there it is possible to push changes through, but this is the hardest way.
')
The third is the main part of file system support. One of the very specific and sensitive subsystems in the kernel. Maintainer Al Viro doesn’t get into your pocket for a word, and if you try to do something without thinking, then don’t get offended - you’ll get an answer not pleasant.
The fourth category is outdated subsystems, so only global edits are accepted there when changing the internal API.
The fifth subsystem for you! No wonder I mentioned staging earlier. In this subsystem, drivers go through an incubation period, that is, on the one hand, the need for a driver is caused by the market (there are devices, support is needed), but on the other hand, the quality of the code is not enough to be included in one of the existing subsystems. Great place to sample the pen. Greg KH is a very loyal maintainer, just be sure to check the changes before sending, otherwise upset him.
There are very good materials in the documentation on the style of the code and the design of the changes, namely CodingStyle and SubmittingPatches . I strongly recommend to get acquainted with them before starting any action.
Before making any changes, maybe someone has already done the same thing? It does not make sense to duplicate the work. For example, the user blueboar2 made a change , but it turned out that there is an earlier , almost a year ago. And if you look closely, then you can find much older .
Do not be discouraged if your change has no reaction.
Firstly, it should be taken into account that the maintener may be busy or absent.
Secondly, the usual public exposure time is a week or more. In this way, the maintainer gives other community members the opportunity to comment on the proposed change.
Thirdly, there is about once a quarter a couple of weeks of silence. This is the so-called merge window, in other words, the window, when the maintainers of the subsystems send the changes that have accumulated during the previous cycle to the main one, that is, to Linus. The window starts exactly from the moment of the release of the next stable version, in the near future v4.0. It is also necessary to take into account that many maintainers no longer accept the new code after -rc5 (v4.0-rc5 is expected on Monday, March 23), since they themselves must deal with their trees.
Below I will describe several methods and tools for checking for change.
First make sure your change is at least compiled. For example, if your edit was for drivers / staging / unisys / virtpci / virtpci.c, then through drivers / staging / unisys / virtpci / Makefile, you can easily understand which configuration option is responsible for enabling the driver:
That is, we need to find how to enable this option. Since we know that the edit is in staging, in the unisys subdirectory, in Kconfig of which the UNISYSPAR menu is defined, so you need to enable several options at once
You can also go along the path
In some cases, if the build occurs on a different architecture, you can either do it in a virtual machine (KVM / Qemu), or (in rare cases) try to add a dependency on COMPILE_TEST to Kconfig, like, for example, for such a driver:
And add to the kernel configuration:
You should absolutely not send such patches without making sure that they are collected on all architectures and do not make a huge number of warnings, or the angry Linus will come to your inbox.
How to test the assembly? Here we go to the following excellent environment variables during assembly, namely
Before building it will be useful to run:
It will search for duplicate inclusions of the same * .h files.
Now you are holding the assembled module. You have made changes in the form of a *. Patch file using
Once again, before sending, make sure that you arrange everything as required. Now boldly run
Finally, for the simplest changes there is a special address: trivial@kernel.org. Read another article like howto: Submitting (Trivial) Linux Kernel Patches .
The first attempts may not succeed, below are some recommendations to reduce the likelihood of such an outcome. I will briefly mention the main areas of problems during the preparation and submission of changes:
- Outdated, conservative and special kernel subsystems
- Style code and design changes
- Novelty change
- Process nuances
Additionally, I will briefly discuss the existing verification mechanisms.
Kernel subsystems
Among all the many subsystems of the kernel, select the following:
- arch / <architecture>
- drivers / scsi
- fs / (main part)
- drivers / isdn, drivers / ide and the like
- drivers / staging
The first is an architectural code. It often contains outdated architectures or code that requires careful study and understanding. Changing something there only makes sense if you really have a piece of hardware on which to test, and the real problem you are struggling with.
The second, SCSI, is perhaps the most conservative subsystem in the core, although there it is possible to push changes through, but this is the hardest way.
')
The third is the main part of file system support. One of the very specific and sensitive subsystems in the kernel. Maintainer Al Viro doesn’t get into your pocket for a word, and if you try to do something without thinking, then don’t get offended - you’ll get an answer not pleasant.
The fourth category is outdated subsystems, so only global edits are accepted there when changing the internal API.
The fifth subsystem for you! No wonder I mentioned staging earlier. In this subsystem, drivers go through an incubation period, that is, on the one hand, the need for a driver is caused by the market (there are devices, support is needed), but on the other hand, the quality of the code is not enough to be included in one of the existing subsystems. Great place to sample the pen. Greg KH is a very loyal maintainer, just be sure to check the changes before sending, otherwise upset him.
Style and design
There are very good materials in the documentation on the style of the code and the design of the changes, namely CodingStyle and SubmittingPatches . I strongly recommend to get acquainted with them before starting any action.
Novelty change
Before making any changes, maybe someone has already done the same thing? It does not make sense to duplicate the work. For example, the user blueboar2 made a change , but it turned out that there is an earlier , almost a year ago. And if you look closely, then you can find much older .
Process nuances
Do not be discouraged if your change has no reaction.
Firstly, it should be taken into account that the maintener may be busy or absent.
Secondly, the usual public exposure time is a week or more. In this way, the maintainer gives other community members the opportunity to comment on the proposed change.
Thirdly, there is about once a quarter a couple of weeks of silence. This is the so-called merge window, in other words, the window, when the maintainers of the subsystems send the changes that have accumulated during the previous cycle to the main one, that is, to Linus. The window starts exactly from the moment of the release of the next stable version, in the near future v4.0. It is also necessary to take into account that many maintainers no longer accept the new code after -rc5 (v4.0-rc5 is expected on Monday, March 23), since they themselves must deal with their trees.
Check mechanisms
Below I will describe several methods and tools for checking for change.
First make sure your change is at least compiled. For example, if your edit was for drivers / staging / unisys / virtpci / virtpci.c, then through drivers / staging / unisys / virtpci / Makefile, you can easily understand which configuration option is responsible for enabling the driver:
obj-$(CONFIG_UNISYS_VIRTPCI) += virtpci.o That is, we need to find how to enable this option. Since we know that the edit is in staging, in the unisys subdirectory, in Kconfig of which the UNISYSPAR menu is defined, so you need to enable several options at once
CONFIG_STAGING = y
CONFIG_UNISYSPAR = y
CONFIG_UNISYS_VIRTPCI = m
You can also go along the path
make menuconfig , guessing by descriptions what you want to include.In some cases, if the build occurs on a different architecture, you can either do it in a virtual machine (KVM / Qemu), or (in rare cases) try to add a dependency on COMPILE_TEST to Kconfig, like, for example, for such a driver:
--- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -269,7 +269,7 @@ config ATA_PIIX config SATA_DWC tristate "DesignWare Cores SATA support" - depends on 460EX + depends on 460EX || COMPILE_TEST help This option enables support for the on-chip SATA controller of the AppliedMicro processor 460EX. And add to the kernel configuration:
CONFIG_COMPILE_TEST = y
You should absolutely not send such patches without making sure that they are collected on all architectures and do not make a huge number of warnings, or the angry Linus will come to your inbox.
How to test the assembly? Here we go to the following excellent environment variables during assembly, namely
W=1 C=1 . The first of them raises the level of compiler warnings, while the second, with the sparse package installed, runs a static code analyzer. Therefore, combining with -j8 , we get: $ make C=1 W=1 -j8 Before building it will be useful to run:
$ make includecheck It will search for duplicate inclusions of the same * .h files.
Now you are holding the assembled module. You have made changes in the form of a *. Patch file using
git format-patch . It would be nice to check the style of the code. Run checkpatch.pl : $ scripts/checkpatch.pl 00* total: 0 errors, 0 warnings, 43 lines checked 0001-dmaengine-hsu-remove-redundant-pieces-of-code.patch has no obvious style problems and is ready for submission. Once again, before sending, make sure that you arrange everything as required. Now boldly run
git send-email .Finally, for the simplest changes there is a special address: trivial@kernel.org. Read another article like howto: Submitting (Trivial) Linux Kernel Patches .
Source: https://habr.com/ru/post/253421/
All Articles