regarding const variables/structures
Hi All... One curious question.. Linux Foundation tweeted this - Meet Bhumika Goyal, age 22, from India. She has had more 340 patches accepted into the Linux kernel, which helped her land one of our two Linux Kernel Guru LiFT scholarships: https://twitter.com/linuxfoundation/status/940340927897489408?lang=en and her commits are - https://github.com/torvalds/linux/commits?author=bhumikagoyal One of my friend told me that, most/many of patches are just converting variables to "*constant*" For example - platform/chrome: chromeos_laptop: make chromeos_laptop const - Declare chromeos_laptop structures as *const *as they are only used during a copy operation. As their value is never modified during runtime, they can be made const. the question is that, how these many const variable issues are left/missed by the previous developers?!?! per my little knowledge, this task looks like a simple one.. many other Kernel Developers should have thought and made these changes long before, but i am not able to understand why this wasnt done until recently?!?!? -- Best Regards, Sekar
On Wed, Sep 12, 2018 at 11:50:35AM +0530, inventsekar wrote:
the question is that, how these many const variable issues are left/missed by the previous developers?!?!
Because we never realized the benifit of doing so when we created the code to start with. And then others copied previous code standards and away we all went. This developer took the time, wrote the correct tooling, handled creating the changes and integrating them properly and getting them merged into hundreds of different kernel subsystems. A non-trivial task to say the least. And it has benifited all users of the kernel greatly due to the increased security it provides. A very nice job.
per my little knowledge, this task looks like a simple one.. many other Kernel Developers should have thought and made these changes long before, but i am not able to understand why this wasnt done until recently?!?!?
Again, because we never really thought about it. Or if we did, we only made changes to the 1-2 drivers/subsystems that we controlled. No one spent the time to look at the whole kernel like Bhumika did, and put in the hard work to make these changes. Each individual patch might look "simple", but realize the effort involved to get all of that work done is not. Hope this helps, greg k-h
On Wed, Sep 12, 2018 at 11:50:35AM +0530, inventsekar wrote:
Hi All... One curious question..
Linux Foundation tweeted this - Meet Bhumika Goyal, age 22, from India. She has had more 340 patches accepted into the Linux kernel, which helped her land one of our two Linux Kernel Guru LiFT scholarships: https://twitter.com/linuxfoundation/status/940340927897489408?lang=en
and her commits are - https://github.com/torvalds/linux/commits?author=bhumikagoyal
One of my friend told me that, most/many of patches are just converting variables to "*constant*"
For example - platform/chrome: chromeos_laptop: make chromeos_laptop const -
Declare chromeos_laptop structures as *const *as they are only used during a copy operation. As their value is never modified during runtime, they can be made const.
the question is that, how these many const variable issues are left/missed by the previous developers?!?! per my little knowledge, this task looks like a simple one.. many other Kernel Developers should have thought and made these changes long before, but i am not able to understand why this wasnt done until recently?!?!?
well (almost) all issues are simple once they are known, and in 16M LoC or what ever the kernel currently is, it is not a big surprise to find a few hundred issues of almost any bug-class. If you really want to know how bad the problem is, or rather how much it says about the overall quality, you would need to answer at least two questions. 1) how many cases of const are missing 2) how many cases of const are in use correctly just counting how many are missing does not tell you much about how good or bad kernel developers are. If 99% of the cases are handled correctly I would say thats good - but given the size of the kernel that would probably mean that there still are quite a few that are not handled correctly. Further you would probably need to check how old the code was that that was fixed up. A brute force grep in the kernel shows that there are 130493 " const " in ther (as of 4.18.2 in -stable) so if the developers got 340 wrong - I would say thats not that scary after all the consequence of not having the const in there are quite benign. I think that it is often times not a mater of not seeing issues but it is a matter of priority and available time - and what this case really does show is that there are opportunities for people moving into kernel development to get to work on simple problems to start with where they can get well acquainted with the procedural issues first - I think that is a good thing and may actually be important to allow new developers to join in. thx! hofrat
Thx Hofrat and Greg.. Kindly suggest me one more thing...(for newbies like me)... I hope she didn't complete that job (Converting all structs/variables to const) (As she did that in last Dec, some new code may be written in the old style, ...Or, she may not have converted 100% structs, variables) So, please suggest some subsystems or some small puedes of code, where i can "dwell" sometimes and submit my first patch. Best regards, Sekar On Wed 12 Sep, 2018, 12:38 PM Nicholas Mc Guire, <der.herr@hofr.at> wrote:
On Wed, Sep 12, 2018 at 11:50:35AM +0530, inventsekar wrote:
Hi All... One curious question..
Linux Foundation tweeted this - Meet Bhumika Goyal, age 22, from India. She has had more 340 patches accepted into the Linux kernel, which helped her land one of our two Linux Kernel Guru LiFT scholarships: https://twitter.com/linuxfoundation/status/940340927897489408?lang=en
and her commits are - https://github.com/torvalds/linux/commits?author=bhumikagoyal
One of my friend told me that, most/many of patches are just converting variables to "*constant*"
For example - platform/chrome: chromeos_laptop: make chromeos_laptop const -
Declare chromeos_laptop structures as *const *as they are only used during a copy operation. As their value is never modified during runtime, they can be made const.
the question is that, how these many const variable issues are left/missed by the previous developers?!?! per my little knowledge, this task looks like a simple one.. many other Kernel Developers should have thought and made these changes long before, but i am not able to understand why this wasnt done until recently?!?!?
well (almost) all issues are simple once they are known, and in 16M LoC or what ever the kernel currently is, it is not a big surprise to find a few hundred issues of almost any bug-class. If you really want to know how bad the problem is, or rather how much it says about the overall quality, you would need to answer at least two questions. 1) how many cases of const are missing 2) how many cases of const are in use correctly just counting how many are missing does not tell you much about how good or bad kernel developers are. If 99% of the cases are handled correctly I would say thats good - but given the size of the kernel that would probably mean that there still are quite a few that are not handled correctly. Further you would probably need to check how old the code was that that was fixed up.
A brute force grep in the kernel shows that there are 130493 " const " in ther (as of 4.18.2 in -stable) so if the developers got 340 wrong - I would say thats not that scary after all the consequence of not having the const in there are quite benign.
I think that it is often times not a mater of not seeing issues but it is a matter of priority and available time - and what this case really does show is that there are opportunities for people moving into kernel development to get to work on simple problems to start with where they can get well acquainted with the procedural issues first - I think that is a good thing and may actually be important to allow new developers to join in.
thx! hofrat
On Wed, 12 Sep 2018 22:40:50 +0530, inventsekar said:
So, please suggest some subsystems or some small puedes of code, where i can "dwell" sometimes and submit my first patch.
0) subsystems? Anything under drivers/staging is fair game and certain to provide hours of amusement... 1) Install sparse 2) Get yourself a linux-next tree (so you don't submit fixes for already fixed stuff) $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git $ cd linux $ git remote add linux-next git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git $ git fetch linux-next $ git fetch --tags linux-next If you want to update it later, do *not* use 'git pull'. Due to the way linux-next is rebased every day, doing that will result in a Lovecraftian horror.... $ git remote update $ git checkout next-20180911 (or whatever) 3) (optional) Add the following patch to the top level Makefile: diff --git a/Makefile b/Makefile index 9e71826f67d7..f28b2ab9c369 100644 --- a/Makefile +++ b/Makefile @@ -434,6 +434,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -Werror-implicit-function-declaration \ -Wno-format-security \ -std=gnu89 +KBUILD_CFLAGS += -Wimplicit-fallthrough=2 -Wvla KBUILD_CPPFLAGS := -D__KERNEL__ KBUILD_AFLAGS_KERNEL := KBUILD_CFLAGS_KERNEL := 4) Figure out why I said (3) (There's an actual reason for that) 5) Build your kernel, running sparse on the code before compiling with extra warnings: $ make C=2 W=1 |& tee build.output 6) Start looking at the output. Note however that sparse and gcc sometimes throw warnings for perfectly good code, so do *not* submit random "shut up warning" patches. Verify the warning is in fact correct, *then* submit the patch. If you get ambitious, find out what Coverty and Trinity are, and other similar tools. If you use Trinity, remember to always mount a scratch monkey. :)
Thanks a ton, Valdis. On Wed 12 Sep, 2018, 11:23 PM , <valdis.kletnieks@vt.edu> wrote:
On Wed, 12 Sep 2018 22:40:50 +0530, inventsekar said:
So, please suggest some subsystems or some small puedes of code, where i can "dwell" sometimes and submit my first patch.
0) subsystems? Anything under drivers/staging is fair game and certain to provide hours of amusement...
1) Install sparse 2) Get yourself a linux-next tree (so you don't submit fixes for already fixed stuff)
$ git clone git:// git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git $ cd linux $ git remote add linux-next git:// git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git $ git fetch linux-next $ git fetch --tags linux-next
If you want to update it later, do *not* use 'git pull'. Due to the way linux-next is rebased every day, doing that will result in a Lovecraftian horror....
$ git remote update $ git checkout next-20180911 (or whatever)
3) (optional) Add the following patch to the top level Makefile:
diff --git a/Makefile b/Makefile index 9e71826f67d7..f28b2ab9c369 100644 --- a/Makefile +++ b/Makefile @@ -434,6 +434,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -Werror-implicit-function-declaration \ -Wno-format-security \ -std=gnu89 +KBUILD_CFLAGS += -Wimplicit-fallthrough=2 -Wvla KBUILD_CPPFLAGS := -D__KERNEL__ KBUILD_AFLAGS_KERNEL := KBUILD_CFLAGS_KERNEL :=
4) Figure out why I said (3) (There's an actual reason for that)
5) Build your kernel, running sparse on the code before compiling with extra warnings:
$ make C=2 W=1 |& tee build.output
6) Start looking at the output. Note however that sparse and gcc sometimes throw warnings for perfectly good code, so do *not* submit random "shut up warning" patches. Verify the warning is in fact correct, *then* submit the patch.
If you get ambitious, find out what Coverty and Trinity are, and other similar tools. If you use Trinity, remember to always mount a scratch monkey. :)
On Wed, Sep 12, 2018 at 11:53:57PM +0530, inventsekar wrote:
Thanks a ton, Valdis.
Another tip for you, read up on what 'top posting' is and don't do it :) Shamelessly stealing Greg's 'top posting' link: https://daringfireball.net/2007/07/on_top Good luck, Tobin.
I didnt get you... I thanked Valdis for his detailed mail.. On Thu 13 Sep, 2018, 3:20 PM Tobin C. Harding, <me@tobin.cc> wrote:
On Wed, Sep 12, 2018 at 11:53:57PM +0530, inventsekar wrote:
Thanks a ton, Valdis.
Another tip for you, read up on what 'top posting' is and don't do it :)
Shamelessly stealing Greg's 'top posting' link:
https://daringfireball.net/2007/07/on_top
Good luck, Tobin.
Hi, ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Saturday, September 15, 2018 1:19 AM, inventsekar <inventsekar@gmail.com> wrote: | I didnt get you... \-- You should always use trimmed, interleaved, bottom-posting. See, how I have quoted your message, and replied below it (similar to how you answer below a question in an exam). You might also want to refer: https://en.wikipedia.org/wiki/Posting_style#Placement_of_replies http://www.shakthimaan.com/downloads/glv/presentations/mailing-list-etiquett... SK -- Shakthi Kannan http://www.shakthimaan.com
On Mon, Sep 17, 2018 at 12:59 PM Shakthi Kannan <author@shakthimaan.com> wrote:
You should always use trimmed, interleaved, bottom-posting. See, how I have quoted your message, and replied below it (similar to how you answer below a question in an exam). Sure, i got it, SK. thanks.
You might also want to refer: https://en.wikipedia.org/wiki/Posting_style#Placement_of_replies
http://www.shakthimaan.com/downloads/glv/presentations/mailing-list-etiquett...
Good readings... Actually i thought this before and i assumed that you guys are mailing using linux's command prompt mail tool. so, this confusions. Good learning on the mail etiquettes. thanks again. Best Regards, Sekar
On Sat, 15 Sep 2018 01:19:24 +0530, inventsekar said:
I didnt get you... I thanked Valdis for his detailed mail..
A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing on usenet and in e-mail?
>>> A brute force grep in the kernel shows that there are 130493 " const " in there Hi Hofrat, 1. may i know the command to do this above grep please.. 2. (and the opposite) may i know the command to grep other type of variables/structures thanks again.. your points and Greg's points are very clear. i was having this doubt from last Dec(from that tweet) and it got cleared. Happy Ganesh Chaturthi to All. ( Lord Ganesh is widely revered as the remover of obstacles,[9] <https://en.wikipedia.org/wiki/Ganesha#cite_note-9> the patron of arts and sciences and the deva <https://en.wikipedia.org/wiki/Deva_(Hinduism)> of intellect and wisdom) On Wed, Sep 12, 2018 at 12:38 PM Nicholas Mc Guire <der.herr@hofr.at> wrote: > On Wed, Sep 12, 2018 at 11:50:35AM +0530, inventsekar wrote: > > Hi All... > > One curious question.. > > > > Linux Foundation tweeted this - > > Meet Bhumika Goyal, age 22, from India. She has had more 340 patches > > accepted into the Linux kernel, which helped her land one of our two > Linux > > Kernel Guru LiFT scholarships: > > https://twitter.com/linuxfoundation/status/940340927897489408?lang=en > > > > and her commits are - > > https://github.com/torvalds/linux/commits?author=bhumikagoyal > > > > One of my friend told me that, most/many of patches are just converting > > variables to "*constant*" > > > > For example - platform/chrome: chromeos_laptop: make chromeos_laptop > const > > - > > > > Declare chromeos_laptop structures as *const *as they are only used > during > > a copy operation. As their value is never modified during runtime, they > > can be made const. > > > > > > the question is that, how these many const variable issues are > left/missed > > by the previous developers?!?! per my little knowledge, this task looks > > like a simple one.. many other Kernel Developers should have thought and > > made these changes long before, but i am not able to understand why this > > wasnt done until recently?!?!? > > > well (almost) all issues are simple once they are known, and in 16M LoC or > what ever the kernel currently is, it is not a big surprise to find a few > hundred issues of almost any bug-class. If you really want to know how bad > the problem is, or rather how much it says about the overall quality, you > would need to answer at least two questions. > 1) how many cases of const are missing > 2) how many cases of const are in use correctly > just counting how many are missing does not tell you much about how good or > bad kernel developers are. If 99% of the cases are handled correctly I > would > say thats good - but given the size of the kernel that would probably mean > that there still are quite a few that are not handled correctly. Further > you > would probably need to check how old the code was that that was fixed up. > > A brute force grep in the kernel shows that there are 130493 " const " in > ther (as of 4.18.2 in -stable) so if the developers got 340 wrong - I would > say thats not that scary after all the consequence of not having the const > in > there are quite benign. > > I think that it is often times not a mater of not seeing issues but it is a > matter of priority and available time - and what this case really does show > is that there are opportunities for people moving into kernel development > to get to work on simple problems to start with where they can get well > acquainted with the procedural issues first - I think that is a good > thing and may actually be important to allow new developers to join in. > > > thx! > hofrat > -- Best Regards, Sekar
On Thu, Sep 13, 2018 at 09:12:32AM +0530, inventsekar wrote: > >>> A brute force grep in the kernel shows that there are 130493 " const " > in there > Hi Hofrat, > 1. may i know the command to do this above grep please.. $ grep -cre " const " * | more will give you the " const " count per file - and then put a shell loop around it $ SUM=0 ; for N in `grep -cre " const " * | cut -f 2 -d ":" ` ; do let SUM=$SUM+$N ; done ; echo $SUM ...as noted "brute force" > 2. (and the opposite) may i know the command to grep other type of > variables/structures well if you want to know how to do that then you do need to look at basics if regular expressions are not clear then you might want to look at those first. And grep really is not the right tool to search for specific structures and their use use something like cscope. Try to focus on doing work you understand including the tools and processes around it - if you just are looking for a fast way of getting X patches into the kernel you are wasting your time. What the work from Bhumika Goyal shows is that its not about the complexity of the change but about the systematic approach based on using understanding a problem class, translating it to an abstract representation amenable to tools (coccinelle in this case) and documenting her understanding in the commit messages to each patch. thx! hofrat
Hi, for the past three years I have been trying hard to do Linux kernel development, but, with No success. I don't know what is the exact reason...maybe, I didn't do much practical, all the times I was only reading reading reading... so you can understand my frustration, that I wanted to submit my first patch asap...and then slowly I can do concentrate on other areas of kernel development... ok thank you. On Thu 13 Sep, 2018, 11:37 AM Nicholas Mc Guire, <der.herr@hofr.at> wrote: > On Thu, Sep 13, 2018 at 09:12:32AM +0530, inventsekar wrote: > > >>> A brute force grep in the kernel shows that there are 130493 " const > " > > in there > > Hi Hofrat, > > 1. may i know the command to do this above grep please.. > > $ grep -cre " const " * | more > > will give you the " const " count per file - and then put a shell loop > around it > > $ SUM=0 ; for N in `grep -cre " const " * | cut -f 2 -d ":" ` ; do let > SUM=$SUM+$N ; done ; echo $SUM > > ...as noted "brute force" > > > 2. (and the opposite) may i know the command to grep other type of > > variables/structures > > well if you want to know how to do that then you do need to look at basics > if regular expressions are not clear then you might want to look at > those first. And grep really is not the right tool to search for specific > structures and their use use something like cscope. > > Try to focus on doing work you understand including the tools and processes > around it - if you just are looking for a fast way of getting X patches > into the kernel you are wasting your time. What the work from Bhumika Goyal > shows is that its not about the complexity of the change but about the > systematic approach based on using understanding a problem class, > translating > it to an abstract representation amenable to tools (coccinelle in this > case) > and documenting her understanding in the commit messages to each patch. > > thx! > hofrat >
Don't be discouraged! So, if you only read the code, it may appear to you that "in the small" you are understanding the code. But, the possible states and paths are a large combinatorial, and lots of things can happen at runtime that you will not be able to predict, even after only reading the code once. Therefore, the best thing to do is to set up some testing framework - that would be highly useful. There isn't much testing capability for the linux kernel itself, other than to have a dedicated machine (be it hardware or qemu or other emulator). The only way to know if something works is to try it, but when it comes to the linux kernel, it cannot be run except with the special conditions that exist at the kernel level, which itself is a problem. I'm not sure if such as I am suggesting is really so feasible or realistic (it certainly isn't possible for some portions of the kernel), but if you could cut off any particular module that resides within the innards of the kernel, and make it retargetably compilable so that you can produce userland modules with it, that would make for a highly ambitious project that would allow kernel developers to more quickly iterate and test their work as they are changing things. You would have to provide, for the set of interfaces that such an inner kernel module were to consume, a second implementation, so that the same header can be reused and the service guarantees maintained. There are lots of things to choose from within the kernel land. For example, there are many data structures that the kernel development environment provides. You could pick one and create a series of sanity checks, and some harness to run each of them safely to establish for yourself testably what the properties are that such a structure or service provides. Then, you could expose to others your tests which they could consume and use to both help debug their code and to maintain it, and if you did the faking of the implementation right so that you can compile it for different targets, users would be able to have a subset of their code produce both kernel and userland targets, where the userland targets have some best effort one to one implementation (if not the real thing excised from the kernel) to establish their correct operation. It sounds like it isn't much if you think that the only thing to test is lists. However, there's nothing stopping you from at least trying to do this for whole portions of the kernel. It may comprise a lot of work, but if you have a userland dummy that mocks out the behavior of something like a scheduler or userland page alignment algorithm so that it can be decoupled and safely run in some simulation capability, you will have made a lot of developers much more productive. On Fri, Sep 14, 2018 at 12:53 PM inventsekar <inventsekar@gmail.com> wrote: > Hi, for the past three years I have been trying hard to do Linux kernel > development, but, with No success. > > I don't know what is the exact reason...maybe, I didn't do much > practical, all the times I was only reading reading reading... so you can > understand my frustration, that I wanted to submit my first patch > asap...and then slowly I can do concentrate on other areas of kernel > development... ok thank you. > > On Thu 13 Sep, 2018, 11:37 AM Nicholas Mc Guire, <der.herr@hofr.at> wrote: > >> On Thu, Sep 13, 2018 at 09:12:32AM +0530, inventsekar wrote: >> > >>> A brute force grep in the kernel shows that there are 130493 " >> const " >> > in there >> > Hi Hofrat, >> > 1. may i know the command to do this above grep please.. >> >> $ grep -cre " const " * | more >> >> will give you the " const " count per file - and then put a shell loop >> around it >> >> $ SUM=0 ; for N in `grep -cre " const " * | cut -f 2 -d ":" ` ; do let >> SUM=$SUM+$N ; done ; echo $SUM >> >> ...as noted "brute force" >> >> > 2. (and the opposite) may i know the command to grep other type of >> > variables/structures >> >> well if you want to know how to do that then you do need to look at basics >> if regular expressions are not clear then you might want to look at >> those first. And grep really is not the right tool to search for specific >> structures and their use use something like cscope. >> >> Try to focus on doing work you understand including the tools and >> processes >> around it - if you just are looking for a fast way of getting X patches >> into the kernel you are wasting your time. What the work from Bhumika >> Goyal >> shows is that its not about the complexity of the change but about the >> systematic approach based on using understanding a problem class, >> translating >> it to an abstract representation amenable to tools (coccinelle in this >> case) >> and documenting her understanding in the commit messages to each patch. >> >> thx! >> hofrat >> > _______________________________________________ > Kernelnewbies mailing list > Kernelnewbies@kernelnewbies.org > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies >
participants (7)
-
Greg KH -
inventsekar -
Kenneth Adam Miller -
Nicholas Mc Guire -
Shakthi Kannan -
Tobin C. Harding -
valdis.kletnieks＠vt.edu