[Outreachy] Tips for the kernel newbies

Next round of Outreachy has started from September 12, 2016. I usually get lot of mails and messages through social networking sites regarding Outreachy and generally for the contribution to Linux Kernel. I worked as an Outreachy intern in Round 10 and last summer I also worked as a GSoC mentor under the Linux Foundation. GSoC project I mentored was also related to Linux Kernel. So, I guess it is probably right time to give few tips to prospective applicants of the next Outreachy round. Some of these tips also applies to every newbie who wants to contribute to Linux Kernel.

General tips:

  • Start early so that you have enough time to understand how community works in general and to decide which projects you are interested in.
  • Do not hesitate to ask questions. But keep in mind that mentors also expect you to do some sort of search before asking a question. So, it is important to understand  what questions to ask. The paper How To Ask Questions The Smart Way by Eric  Raymond and Rick Moen is an excellent guide.
  • Take mentors’/reviewers’/maintainers’ comments positively. And work on them to avoid repeating same mistakes again. Also, learn from others’ mistakes. I am still used to read comments of developers on others’ patches so that I don’t repeat the same mistake in my patch. I followed the same rule during my application round as well. So, read comments of mentors on other applicants’ patches and try to follow them as well.
  • If something is not clear in the comment then please ask. It is better to ask at that time than repeating the same mistake again. This will save your time.
  • Be nice and respect others’ point of views. Definitely we like to discuss on technical parts but arguing politely will help you to create bonding and it is necessary for the healthy community as well.
  • The culture in a particular open source community may vary from that in other communities. Kernel community is famous for their frank opinions. Generally the comments on your patch is for your code or the change you are proposing, and not for the person as an individual. So,never take those comments personally. Take them positively and try to improve the quality of code for the next contribution.
  • Engage in community. We like when applicants helps each other. Never look at this as a competition. In the end, we all are going to learn something. This is true for both mentors and applicants.

Kernel contribution or patch related tips:

  • First kernel patch tutorial is your friend. This includes everything from installing things to sending your patches properly. Read it very carefully.
  • Usually in Outreachy we advise applicants to start from checkpatch.pl issues just to give  them an idea about how community works and to help them get started with contributing to the Linux Kernel. But not every checkpatch.pl warning/error is right. So please try to understand why that warning is there instead of fixing them blindly. There are false positives as well.
  • Do not top post while responding to mails. Look at the details of how to respond to mails inline.
  • Do not copy commit log and subject. You can check the other patches for the reference but copy pasting will never help.
  • For the subject line always run ‘git log’ to check what kind of subject lines are used for the particular file.
  • Quantity of patches matters but quality of patches is important as well. Move from checkpatch.pl issues once you get the idea of how things are working. This will help you to learn more.
  • Usually in the Linux kernel projects, we have small tasks for the applicants by mentors. Work on them and ask questions to mentors if something is not clear.
  • Be consistent in sending patches. Being in touch with the mentors help them to understand your perspective for the project.
  • Two important things to do before sending every patch:
    1. Always compile-test your patch before sending it to the maintainers/mailing lists.
    2. Always run checkpatch.pl on your patch.

    Half of the time this will show you if your change is correct or not.

Application related tips:

  • Never lie about your commitments for the internship period. Be clear and discuss this with mentors of the project you are applying.
  • Mention about what kind of work you have done and how this will help you achieve your goal in the project. The point of mentioning this tip here is, you can have idea about what kind of patches you should send during the application period.

Last but not least, always remember that we all were newbie at one point of time. So, never feel embarrassed or bad when something is not working. You are amazing and we are thankful to you as you are helping us to improve our community or code or even ourselves as well.

Welcome to the kernel community and all the best everyone!

Devm functions and their CORRECT usage

Hi,

Lets start this blog with some good news. Guess what, I won the Linux Foundation Training Scholarship under Kernel Guru category. I am so excited for taking Linux Foundation training class. They announced names of scholarship winners in LinuxCon NA which was in settle last week and it came out that I am a youngest scholar who won this scholarship. 🙂

So, lets come to the topic of this blog post. Over the last couple of weeks, I worked on devm functions and their incorrect usage. I couldn’t make it to update my blog properly regarding the same. But in this blog, I have tried to write good summary of my work on devm functions and by adding links wherever possible for someone who is more interested in the discussions and patches. First of all, lets start with introducing devm functions.

Devm Functions: What, Why and Where

There are some common memory resources used by drivers which are allocated using functions and stored as a linked list of arbitrarily sized memory areas called devres. Some of this memory is allocated in the probe function. But all of this memory should be freed on a failure patch or before the driver is detached. Else, the driver will leak resources and wastes main memory.

There are couple of reasons behind introducing devm functions. First is of course about resource leaking. It can be possible that if anything fails in the middle of probe function, it frees anything allocated. Also, remove function is just a duplicate code of probe’s error handling. And most important thing is it is hard to spot problems in failure handling with non-managed resource allocation. So, we need managed versions of such functions = devm functions. Devm functions basically allocates memory in a order resources are allocated and deallocates those memory automatically in a reverse order. Here, is a link of slides which can be useful to understand devm functions and their usages.

Problems I encountered or came across while looking in to devm functions

1. Basically sometimes what happens is developers want to use devm functions because they think that it is better to use managed resource functions but they don’t know how to use it and end up with messing things. For example, here is one of my patch. In this file, we already have devm_snd_soc_register_card and as I said devm functions automatically handles when to free memory. So, actually we don’t need to use function for unregistering the card. I have sent some patches for similar cases and I have provided links of accepted patches at the end of the blog.

2. While working on devm functions, I encountered that many files are using devm_free_irq in remove functions with the devm_request_irq in probe function. One can understand that we need devm_free_irq in remove function when we don’t have devm counterparts for each allocated resource function and it needs to occur before something else to be released. That can be a case where we need to call devm_free_irq explicitly. But not all cases are like that. Here is a link of one of such discussion on a case where use of devm_free_irq is not at all necessary. There are some other such interesting cases for devm_free_irq and but one need to look closely for each case. Also, it can be possible that there are some other such cases for other devm functions. One another example is this.

3. Sometimes it is possible that memory lives within a single function and frees after some line of code. Then we don’t really need devm functions because it just wastes extra memory used for devres structures and few extra cycles for maintaining them. Here, is such example.

Here, are those Coccinelle semantic patches which I used to detect such cases. File devm_entry_points is used to detect problematic usage of devm functions and devm_opportunity is used for finding files which are still using non-managed functions whose devm counterparts already exists.  I worked on each case encountered by devm_entry_points but not going to discuss it here. In case you have any questions please ask me here and I’ll be happy to answer them.

I sent many patches using both of these files and for some which came in to my way while working on devm functions. Here, is a links of some of my accepted patches for the reference and there are many more in their way to be accepted. All of my accepted patches can be found here.  I am going to send some more as still there are many opportunities where devm functions can be used.

I know understanding devm functions is little bit complicated at first but once you will understand them properly, you can enjoy the beauty of them. Please ping me for any queries regarding devm functions or my patches.

Links:

  1. power_supply: bq24735: Convert to using managed resources
  2. ASoC: tegra: Use devm_clk_get
  3. ASoC: tegra: Convert to managed resources
  4. ASoC: davinci-vcif: Use devm_snd_soc_register_component
  5. crypto: sahara – Use dmam_alloc_coherent
  6. ASoC: rockchip: i2s: Adjust devm usage
  7. ASoC: samsung: Remove redundant arndale_audio_remove
  8. ata: pata_arasam_cf: Use devm_clk_get

Update – 1: I have continued working on devm functions even after the internship. So, now there are many patches which are accepted in the mainline kernel. And still there are many opportunities where one can go for working on. So, here is a link of my all patches. One can go there and check the things for better understanding.

Macro builtin_platform_driver

This post is for introducing use of macro builtin_platform_driver. I just came across it while solving cases of module init/exit boilarplate code. Paul Gortmaker introduced this macro. One can see many patches with his name in Linux Kernel git tree. So, lets discuss the reason behind introducing it and where we can use it. I am not going in to that much detail, instead I am providing links so that one can go there and understand what is happening here.

Why we need macro like builtin_platform_driver?

Basically there are increasing number of non-modular drivers which are using module_driver type register functions. And there are several downsides of using this. One can see that commit from Paul Gortmaker here which has very infromative commit log.

Where we can use this macro?

One can go for checking in Kconfig file while handling any driver. And see if the driver is configured with a Kconfig option that is declared as a bool. If that is the case then one can go for using this macro for that file. But yes one need to make sure that if the driver depends on other drivers then all of them are declared as bool. Usually we have tristate for module drivers.

How I came across this macro?
When I was working on module init/exit boilarplate issue, I just saw some patches from Paul Gortmaker. Also, while handling one case Paul Bolle suggested me to use this macro. Although I knew that this macro exists, I was not sure about using for that particular file. And I ended up using it for file drivers/hwtracing/coresight/coresight-replicator.c. My patch can be found here.

Can Coccinelle help to handle such cases?
No, Coccinelle can’t help to detect such cases because for each file one need to check Kconfig options in Kconfig file. So one need to do it by hand. But yes Coccinelle can help you with transformation part. One can write semantic patch accordingly. I have added some semantic patches on which I worked in my github account. One can use them and change them according to their need.

Ether device API functions and some other hacks

Hi again

I worked on ether device API functions in a last month before module init/exit cases. Mainly I worked on 3 functions and got understanding of some others. I am going to talk about some other functions along with ether device API functions in this blog. So, this is going to be somehow managed blog.

1. eth_zero_addr:

This is a ether device API function which replaces memset to assign zero address. Linux kernel community prefers to using eth_zero_addr function instead of memset to assign the zero address to the given array. The function definition goes like this:

static inline void eth_zero_addr(u8 *addr)
{
memset(addr, 0x00, ETH_ALEN);
}

There were like 10-12 files having such cases. I sent patches for all of them. Most of them are now added in a kernel tree. Here, is a semantic patch which I used to do this change. It is preety simple. Only thing is sometimes value ‘6’ is used as ETH_ALEN. In that case one need to check that it is really a network code and in a memset 6 means ETH_ALEN.

@eth_zero_addr@
expression e;
@@

-memset(e,0x00,\(6\|ETH_ALEN\));
+eth_zero_addr(e);

2. eth_broadcast_addr:
This is also ether device API function and replaces memset. Only difference is this function is used to assign the broadcast address to the given address array. There were only 2-3 such cases. I handled all of them too. Semantic patch I used is as follows:

@eth_broadcast_addr@
identifier e;
@@

-memset(e,\(0xff\|0xFF\|255\),\(6\|ETH_ALEN\));
+eth_broadcast_addr(e);

3. eth_hw_addr_random:

This is an intersting function. It generates random Ethernet address [MAC] to be used by a net device and set addr_assign_type so that state can be read by sysfs and be used by userspace. Now, definition of this function goes like this:

static inline void eth_hw_addr_random(struct net_device *dev)
{
dev->addr_assign_type = NET_ADDR_RANDOM;
eth_random_addr(dev->dev_addr);
}

Basically I found this case from the deprecated function list. There were like 63 uses of random_eth_addr in linux 3.0 and now very less uses are there. Primary reason behind this was here random_eth_addr is changed to eth_random_addrfor the consistency purpose. So, there were many commits proposing that change. But when I looked in to detail then found a large bunch of commits from Danny Kukawka in 2012, replacing random_eth_addr with eth_hw_addr_random. Although in each case one need to check many things at a time because sometimes original call of random_eth_addr/eth_random_addr do not make assignment to NET_ADDR_RANDOM. And in some cases we have seen that merging the call and the assignment is not correct. So, from my side I can just compile test the change. And in each case I need to write about this possibility under —. For the experiment I just sent a patch explaining the situation and it turned out positive. After tesing it maintainer applied the patch. That patch can be found here. I am going to send some more patches in a future.

4. Deprecated macro DEFINE_PCI_DEVICE_TABLE

This macro is actually deprecated and this comment is made in a header file too. I just accidentally came across this while reading some other code. And thought to handle it. Basically point is we can use struct pci_device_id instead of this macro. When I searched for the remaining use of this macro in a linux-next, it turned out that most of them were handled by Julia’s one of other intern in a Paris. Only 3 uses were remaining. I sent patches for them. One is applied in a local branch and others are on their way. This was a simple case too. Semantic patch I used is as follows:

@@
identifier a;
declarer name DEFINE_PCI_DEVICE_TABLE;
initializer i;
@@
– DEFINE_PCI_DEVICE_TABLE(a)
+ const struct pci_device_id a[]
= i;

5. Unnecessary function snd_pcm_lib_preallocate_free_for_all()

I picked this from deprecated functions list of Julia. Here, there is no point of using this function snd_pcm_lib_preallocate_free_for_all() because ALSA core takes care that all preallocated memory is freed when the PCM itself is freed. Only one case was left. I just sent a patch for it yesterday and to my surprise it was applied in a very next minute by Mark. 🙂 One can look at the patch here.

By the way, I came across non-modular version of module init/exit macros. Recently many such patches are added in a kernel tree. It is interesting to understand the reasons behind introducing and using such macro. I guess one separate post would be fine for that. I will post that blog in a 2-3 days. Also, I am working on fixing some usage of devm functions from last 2 weeks. I’ll write a blog about it in a next week too.

Till then signing off. Have a happy weekend.

[Part II] Macros module init/exit, Boilerplate code and Linux Kernel

So, as I said in my last blog, I will explain about transformation semantic patch which can be used after matching module init/exit part. Before I go in to the detail, I would like to clear some points. Someone asked me about the use of * in the semantic patch of my last blog post. So, I thought I should add it in this blog too. Basically we use * for something of interest. For example in our case we just want to check if actually there are some cases present which do nothing except register/unregister in module init/exit. One can’t use * with +/-. Following is an output of the semantic patch explained in last blog post.

diff -u -p ./lib/ts_fsm.c /tmp/nothing/lib/ts_fsm.c
--- ./lib/ts_fsm.c
+++ /tmp/nothing/lib/ts_fsm.c
@@ -327,12 +327,10 @@ static struct ts_ops fsm_ops = {
static int __init init_fsm(void)
{
- return textsearch_register(&fsm_ops);
}
static void __exit exit_fsm(void)
{
- textsearch_unregister(&fsm_ops);
}
MODULE_LICENSE("GPL");

Now lets continue from where we left in last blog post. So, after matching cases, we need to remove functions of module init/exit along with declarations of module-init/module_exit. Also, we need to add helper macros like module_platform_driver. We can do that in individual cases by matching register/unregister functions. Here, is an example of such semantic patch for cases where we can use helper macro module_platform_driver.

@r@
declarer name module_init;
identifier f;
@@
module_init(f);

@s@
declarer name module_exit;
identifier e;
@@
module_exit(e);

@a@
identifier r.f;
identifier x;
@@
static f(…) {return platform_driver_register(&x); }

@b depends on a@
identifier s.e,a.x;
@@
static e(…) { platform_driver_unregister(&x); }

@t depends on r && a@
identifier r.f;
@@
-module_init(f);

@v depends on s && a && b@
declarer name module_platform_driver;
identifier s.e, a.x;
@@
-module_exit(e);
+module_platform_driver(x);

@c depends on b@
identifier r.f, a.x;
@@
-static f(…) { return platform_driver_register(&x); }

@d depends on c@
identifier s.e, a.x;
@@

-static e(…) { platform_driver_unregister(&x); }

In first four rules of the semantic patch we are doing matching and then depending on these 4 rules we are doing transformation. This semantic patch can be used for any such cases. Only thing which one need to change is names of register/unregister functions and helper macro. Interesting, right! Best part is those algorithms which are used by Julia in developing a tool. Because Coccinelle is very fast. And I think that’s the beauty of Coccinelle 🙂 In last 2 weeks, I worked on some ether device API functions too. But may be one separate blog will be good to explain them. So, this will be the subject of my next blog. Till then stay connected. Stay happy. Ttyl.

[Part I] Macros module init/exit, Boilerplate code and Linux Kernel

Hii

Lets talk about boiler plate code today. I worked on cases of boiler plate code in Linux kernel during last few days. I am going to divide this particular topic in 2 blog posts. In this post, I will talk about boiler plate code, cases of them and how can we find them in the kernel using Coccinelle. In the second post, I will talk about how Coccinelle can help to handle such cases.

Boiler plate code:

Boilerplate code is any seemingly repetitive code that shows up again and again in order to get some result that seems like it ought to be much simpler. Basically boilerplate code or boilerplate is the sections of code that have to be included in many places with little or no alteration.

Bolier plate code and init/exit macros:

In kernel macro module_init can either be called during do_initcalls (if builtin) or at module insertion time (if a module). Macro module_exit is used to wrap the driver clean-up code with cleanup_module when used with rmmod and the driver is a module. If the driver is statically compiled into the kernel, module_exit has no effect. There can only be one module_init and one module_exit per module. In 70% of cases drivers don’t do anything special in module init/exit. So, such bolier plate code can be eliminated using some helper macros like module_platform_driver, module_pci_driver, module_pcmcia_driver etc. Here, is an example of such code form kernel:


static int __init snirm710_init(void)
{
return platform_driver_register(&snirm710_driver);
}
static void __exit snirm710_exit(void)
{
platform_driver_unregister(&snirm710_driver);
}
module_init(snirm710_init);
module_exit(snirm710_exit);

[From file drivers/scsi/sni_53c710.c]

Basically these helper macros are defined for drivers whose init and exit paths does only register and unregister. Sometimes we have unnecessary print statements and code in module init/exit too. In such cases we can use these helper macros too. Currently there are some such general macros and driver specific helper macros presented in the kernel. And many more such opportunities are there.

Macros module init/exit and Coccinelle

Generally we can use following Coccinelle semantic patch to match the module init/exit associated functions and statements.

@r@
declarer name module_init;
identifier f;
@@
module_init(f);

@s@
declarer name module_exit;
identifier f;
@@
module_exit(f);

@a@
identifier r.f;
statement S;
@@
f(…) { S }

@depends on a@
identifier s.f;
statement S;
@@
f(…) {
*S
}

@b@
identifier s.f;
statement S;
@@
f(…) { S }

@depends on b@
identifier r.f;
statement S;
@@
f(…) {
*S
}

So, after getting output of this semantic patch I analyzed all cases. And grouped them together accordingly. I grouped them in with categories like old macro and new macro means cases where we can use already defined helper macro and where we need to define new macros respectively. I am thinking to put that file on my github along with some coccinelle scripts so that others can use it. I am going to handle most of them. I have sent some patches and will sent for all those cases where old macro can be used. For the new macro cases, I will send patches introducing some of those macros which can handle maximum cases and such cases are already present there.

Note that above script can help with matching things only. For the transformation one need to be more specific in the script. I will talk about those scripts and some intersting stuff regarding the same in my next post.

Till then stay tuned!

Say yes or no to mod_timer!

Its been almost 20 days that I have not written any blog on my progress. Yes, I know I should and I am feeling sorry for that. 😦 Anyways I am back. 🙂 I worked on many new things and learnt a lot during last few days. I will not be able to sum up everything in a single blog. That’s why here I am going to post 3-4 blog posts back to back again. So, lets talk about mod_timer today.

I came to know about mod_timer function and its usage during Outreachy application. Actually one of Coccinelle challenge problem was for setup_timer function [View my blog on it to get more information]. While looking at the definition of setup_timer function, I got to know about mod_timer function. During application period, I sent few patches for that and they were accepted too. So, I decided to look at them again with the goal of writing one common semantic patch. But I ended up with skipping them completely. There are particular reason behind this. I was not sure about writing this post but then I thought outreachy is all about learning. And as I learnt something from this, I should share it.

Why mod_timer was introduced?

First of all, let me start with why mod_timer function was introduced. So, kernel documentation says that mod_timer is a more efficient way to update the expire field of an active timer (if the timer is inactive it will be activated). And if there are multiple unserialized concurrent users of the same timer, then mod_timer is the only safe way to modify the timeout. According to definition, mod_timer(timer, expires) is equivalent to:

del_timer(timer);
timer->expires = expires;
add_timer(timer);

What is the problem with using mod_timer for these kind of patterns?

It looks pretty obvious use, right?? But no, it is not. And reason is, it can be possible that instances of this pattern also modify the timer’s data and function fields which if the timer is still running and expiring while being fiddled with, might result in
a race condition. Which means that after the change there will be an old function but new data field being used in combination or something like that. So, it’s not a good idea to remove del_timer in the such cases and use mod_timer. One can look at the ROSE directory stack where such cases are present and we can’t use mod_timer there.

Is there any other pattern where mod_timer can be used?

Yes. I have seen some patches in sound directory from Takashi Iwai about introducing use of mod_timer. He introduced use of mod_timer for following kind of pattern:

timer->expires = expires;
add_timer(timer);

I looked in to those cases where he refactored such code with mod_timer. And came across this particular case. Here, in the later one he uses setup_timer and mod_timer. There is no del_timer, because the timer is not yet running. It looked little strange because the code is not modifying an existing timer, but rather starting up a new one. So, Julia told me to ask Takashi for the same. I mailed him and also asked for suggestion about doing this in general in the kernel. He replied that “The behavior of mod_timer() for the first invocation in this case is defined to behave just like add_timer(), so it’s correct to use it. But yes it isn’t always intuitive as a replacement of add_timer()”.

So thing is for this kind of pattern mod_timer is a matter of taste and it depends on maintainers. And I guess when it comes to adding any semantic patch or doing something in a kernel, one should always think about general cases. So, Julia advised me that it could be more useful to work on other functions/macros which really need some work. And I ended up with closing chapter of mod_timer here. 🙂