March 18, 2015 at 16:20Let's make the code cleaner: Recommendations for preparing changes to the Linux kernel
Continuing the topic of improving the Linux kernel code , I want to give some 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. Briefly note the main areas of problems during the preparation and sending of changes:
In addition, I will briefly talk about the existing verification mechanisms.
Among the whole set of kernel subsystems, we distinguish the following:
The first is the architectural code. Often contains legacy architectures or code that requires careful study and understanding. Changing something there makes sense only if you really have a piece of iron on which you can test, and a real problem that you are struggling with.
The second, SCSI, is perhaps the most conservative subsystem in the kernel, although it is possible to push changes there, but this is the toughest way.
The third is the bulk of file system support. One of the very specific and sensitive subsystems in the core. The Al Viro maintainer will not get into your pocket for a word, and if you try to do something without thinking, then do not be offended - you will not get a pleasant answer.
The fourth category is obsolete subsystems, therefore only global edits are accepted there when changing the internal API.
The fifth subsystem for you! Not for nothing that 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, the quality of the code is not enough to be included in one of the existing subsystems. Great place to sample your 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 changes, namely CodingStyle and SubmittingPatches . I strongly recommend that you familiarize yourself with them before starting any action.
Before making changes, look, maybe someone has already done the same? It makes no sense to duplicate the work. So, for example, the user blueboar2 made a change , but it turned out that there was an earlier , almost a year ago. And if you look closely, you can find much older .
Cheer up if there is no reaction to your change.
First, keep in mind that the miner may be busy or absent.
Secondly, the usual exposure time of changes in public is a week or more. In this way, the maintainer allows other members of the community to speak out about the proposed change.
Thirdly, there is a couple of weeks of silence about once a quarter. This is the so-called merge window, in other words, the window when the subsystem maintainers send the changes accumulated over the previous cycle to the main one, that is, Linus. The window starts exactly from the moment the next stable version is released, in the near future v4.0. It is also necessary to consider that many maintainers stop accepting new code after -rc5 (v4.0-rc5 is expected on Monday, March 23), since they themselves need to deal with their trees.
Below I will describe several methods and tools for checking 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 the Kconfig of which the UNISYSPAR menu is defined, therefore you need to enable several options at once
You can also go along the way
In some cases, if the assembly takes place on a different architecture, you can either do it in a virtual machine (KVM / Qemu), or (in rare cases) try to add a dependency to COMPILE_TEST in Kconfig, as, for example, for such a driver:
And add to the kernel configuration:
It is absolutely not worth sending such patches without making sure that they are collected on all architectures and do not bring a huge number of warnings, or the angry Linus will come to your mailbox.
How to test the assembly? Here we move on to the next great 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 have the assembled module in your hands. You registered the changes as a * .patch file with
Once again, before sending, make sure that you have everything as designed . 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. Briefly note the main areas of problems during the preparation and sending of changes:
- Obsolete, conservative, and special kernel subsystems
- Code style and design changes
- Novelty of change
- The nuances of the process
In addition, I will briefly talk about the existing verification mechanisms.
Kernel subsystems
Among the whole set of kernel subsystems, we distinguish the following:
- arch / <architecture>
- drivers / scsi
- fs / (main part)
- drivers / isdn, drivers / ide and the like
- drivers / staging
The first is the architectural code. Often contains legacy architectures or code that requires careful study and understanding. Changing something there makes sense only if you really have a piece of iron on which you can test, and a real problem that you are struggling with.
The second, SCSI, is perhaps the most conservative subsystem in the kernel, although it is possible to push changes there, but this is the toughest way.
The third is the bulk of file system support. One of the very specific and sensitive subsystems in the core. The Al Viro maintainer will not get into your pocket for a word, and if you try to do something without thinking, then do not be offended - you will not get a pleasant answer.
The fourth category is obsolete subsystems, therefore only global edits are accepted there when changing the internal API.
The fifth subsystem for you! Not for nothing that 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, the quality of the code is not enough to be included in one of the existing subsystems. Great place to sample your 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 changes, namely CodingStyle and SubmittingPatches . I strongly recommend that you familiarize yourself with them before starting any action.
Novelty of change
Before making changes, look, maybe someone has already done the same? It makes no sense to duplicate the work. So, for example, the user blueboar2 made a change , but it turned out that there was an earlier , almost a year ago. And if you look closely, you can find much older .
The nuances of the process
Cheer up if there is no reaction to your change.
First, keep in mind that the miner may be busy or absent.
Secondly, the usual exposure time of changes in public is a week or more. In this way, the maintainer allows other members of the community to speak out about the proposed change.
Thirdly, there is a couple of weeks of silence about once a quarter. This is the so-called merge window, in other words, the window when the subsystem maintainers send the changes accumulated over the previous cycle to the main one, that is, Linus. The window starts exactly from the moment the next stable version is released, in the near future v4.0. It is also necessary to consider that many maintainers stop accepting new code after -rc5 (v4.0-rc5 is expected on Monday, March 23), since they themselves need to deal with their trees.
Verification mechanisms
Below I will describe several methods and tools for checking 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 the Kconfig of which the UNISYSPAR menu is defined, therefore you need to enable several options at once
CONFIG_STAGING = y
CONFIG_UNISYSPAR = y
CONFIG_UNISYS_VIRTPCI = m
You can also go along the way
make menuconfig, guessing from the descriptions what you want to include. In some cases, if the assembly takes place on a different architecture, you can either do it in a virtual machine (KVM / Qemu), or (in rare cases) try to add a dependency to COMPILE_TEST in Kconfig, as, 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
It is absolutely not worth sending such patches without making sure that they are collected on all architectures and do not bring a huge number of warnings, or the angry Linus will come to your mailbox.
How to test the assembly? Here we move on to the next great environment variables during assembly, namely
W=1 C=1. The first of them raises the compiler warning level, while the second, when the package sparseis installed, launches a static code analyzer. Therefore, combining with -j8, we obtain:$ 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 have the assembled module in your hands. You registered the changes as a * .patch file with
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 have everything as designed . 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 .