views:

952

answers:

6

We have some software which relied on certain behavior from an another (very commonly used) application that has now changed, rendering our current implementation workable, but less than optimal.

We believe that this change may have affected a number of other applications, particularly in the performance monitoring arena, and we have found a solution that we believe will improve a slew of other potential problems.

Unfortunately, said solution is a kernel change (relatively simple but high-impact if we stuff it up) and we have no experience in submitting kernel patches for review.

Has anyone on SO actually submitted a patch (while I'd appreciate all answers, I suspect the best ones will come from those that have been through the process, even unsuccessfully)? Have you had it accepted (what are the chances that Alan Cox et al hangs about on SO)?

What is the correct process to follow? I have no intention of sending off an email to Linus since I know he has a cadre of protectors that you're supposed to go through before it gets to him. How do I find out who is responsible for a particular section of the kernel.

It may be that I'm being overly optimistic in thinking someone the kernel world's never heard of can contribute, but I'd be interested to find out.

EDIT with more details:

The change is not actually for a performance bug, but an improvement (in my view :-) to the process accounting entries (currently) written when a process terminates.

Websphere App Server (ah, IBM, bless their little hearts) has changed what it does; JVMs used to exit regularly so that their entries were written and we could use that for chargeback. Now it leaves JVMs lying around for months, meaning the data is not available in a timely fashion unless we force WAS down regularly. Somehow I don't think IBM's Software Group are going to fix their software for us :-). In any case, I believe it may be a useful feature to have for other long-lived processes.

Currently type-3 process accounting records are written when a process exits, what we're looking at is a mechanism to write type-N records periodically while a process is still active giving the figures since last write (or process start if this is the first time). Chargeback or performance-monitoring applications could choose to use either the type-3 records (totally unchanged) or the interim type-N records. The current workaround we have is to monitor /proc/PID/stat for specific processes but that's a horrible kludge since it doesn't integrate well with the real process accounting.

It needn't be often (we'd be happy with 24 hours) but there may be a performance impact since the work that's currently done only on process exit() will have to be done occasionally on process context switch. Linus et al might not like that idea since it may be a high-impact area of the code (even checking if there's been 24 hours since the last write may be too slow for them).

Still, thanks for all the answers so far, I'll see how I go. Give me a couple of days and I'll accept the best answer.

+1  A: 

I haven't tried submitting any kernel patches myself, and the docs are lacking in this area.

But this page looks like it can point you in the right direction.

OIS
I especially love the WhatNotToDo section ("you're all idiots!"). That should hopefully avoid any initial problems.
paxdiablo
And the GettingFlamed, which most new submitters probably experience. :)
OIS
+10  A: 

Well, you could try reading Documentation/SubmittingPatches in the linux kernel source tree.

Chris Young
That seems okay, but there's no obvious entry for the area of interest which means I'll be hassling Linus himself, I guess :-).
paxdiablo
No, you should email the linux kernel mailing list, see section 5, paragraph 2.
Chris Young
I have submitted a kernel driver, and had it accepted through the LKML mailing list. this is probably the best way to go about it
Hasturkun
Ah, I couldn't find section 5 (there's only 3 sections), but I see you mean section 1, part 5. Thanks heaps. I'll give it a shot.
paxdiablo
Yes, sorry. Section 1, sub-section 5, paragraph 2. :)
Chris Young
+3  A: 

In a large project, the best way to get a patch into the main tree is to contact the person who is maintaining the particular piece of code. So, look through the Linux MAINTAINERS file to see who is formally the maintainer of the code you've changed and also at the Linux kernel SCM repository to locate the developers who recently worked on that code. To increase your chances of the patch getting accepted:

  • ensure that your patch is easy to understand and follows existing code and documentation standards,
  • explain clearly what your patch achieves,
  • submit your changes in an appropriate format (the output of diff -up for Linux),
  • make sure the patch can be cleanly applied (and works) on the latest software (Linux kernel) version,
  • include test cases that demonstrate both the problem you're addressing and how your patch solves it, and
  • don't include in your code irrelevant (e.g. formatting) changes.

A small fix for a known bug is more likely to get incorporated into a project than large code changes that introduce a new feature of marginal or dubious utility. In some cases it's worth to first file a bug report through the project's issue tracking database, and then submit a patch that addresses the specific issue. To avoid disappointment, if you're thinking of making a large change, it's better to discuss the change and your proposed implementation with the maintainer before writing the code.

Diomidis Spinellis
+1  A: 

Read the Documentation/SubmittingPatches, find the appropriate MAINTAINER, and most importantly, discover where all the discussion is really happening. It might not be on the kernel mailing list itself, but perhaps on some subsystem ML.

Then subscribe to this ML, and submit your patch as RFC.

I don't know if your patch is invasive, but try to split it in a queue of logical change, each with his own explanation.

shodanex
+10  A: 

Before anything else: concentrating about the performance bug report, and getting it right (with repeatable benchmarks) will at least help you to get people to bother with the problem. Also submit the patch after testing it, but beware that your great patch might use the wrong approach, and that they might write a better one. Or that simply it may be great, but might need fixes to get accepted, that even happens with uber-guys. And don't think to email somebody privately, but refer to LKML or to the appropriate subsystem ML.

I'd suggest you to read through all other answers, and all applicable material, before contacting kernel developers; and read the bibliography of SubmittingPatches as well. They might be harsh if you do it wrong. The kernelnewbies IRC chat is a good place for you to start, because they are for sure welcoming, even if sometimes the environment can be too newbie-like (not sure, I've not been there so much).

It may be that I'm being overly optimistic in thinking someone the kernel world's never heard of can contribute, but I'd be interested to find out.

It's not overly optimistic; not in itself at least. Abstracting from you (since I don't know your skills), what is more unlikely is that your patch will be accepted without modifications, or that it's written according to the right skills. But actually, if your patch is addressed to a smaller community, it may be much easier.

From somebody with some experience (i.e. me), before considering the patch submission, describe the problem and why it affects other applications. Considerations like "this improves our performance", especially if you qualify (vaguely) as a vendor, won't have appeal on kernel developers.

Especially, omit such statements:

rendering our current implementation workable, but less than optimal. this will buy you a "fix your code" recommendation immediately by most readers.

If performance of an existing application (not written by you) is impacted, that's different. For instance, once Linus promptly paid attention to fixing in the kernel performance for screwed up code, because that code was part of make, even if he was proud of the code he had written and of the fact that he didn't need to do that exact fix. I.e., you need an application which everybody cares about, or a solution without disadvantages. So, stuff like:

behavior from an another (very commonly used) application is good, as long as your usage of that application is not deemed unreasonable.

Finally, if you refer to source code, they'll likely ask to see the interested section - think to license issues with your code, if they exist, and solve any of them beforehand if you want to answer them quickly.

Btw, this is a partial account of my experience there: https://www.ohloh.net/accounts/Blaisorblade

If you want, you can contact me to help you directly with a proposed mail, and CC me on the discussion. I'm quite busy, but I might find some more time :-).

Blaisorblade
See update to question.
paxdiablo
+1  A: 

On the EDIT, the answer might be interesting as an example case. I guess your requirement is totally reasonable, but you're right that even a test on context switch might be too expensive. But since the kernel has a timer implementation, I don't see why it can't be used to avoid that. So, indeed, suggesting a request for enhancement is the safest bet. I'm surprised that suggesting to send a bug report instead of a patch was such a good fit. You can also modify the patch yourself to use timers yourself if you would like to submit it, but still be ready for discussion :-) You can even add "we have a local fix but it adds some tests on the context-switch fast path, that's why the patch is attached for reference but should not be applied". Turning down your own code, if it's known to be bad, will avoid harsh reviews of the patch.

The alternative is to run some benchmarks and prove there is no impact, but if timers are viable that code will be rejected anyway, or to try the timer solution yourself (something better may exist). Find the benchmarks they use for the kernel scheduler; look at the "recent" threads about the CFS Ingo's (or Kolivas'?) patch and take their benchmarks.

About support, kernel developers won't care about "Websphere App Server" by itself, if it does unreasonable things, not even IBM-funded ones. But with my limited knowledge of the situations, shutting down a JVM periodically does not make sense, it seems just a way to paper over some memory leak/instability, so the current behaviour must be supported.

Blaisorblade
No way would I suggest it's a bug to the devs, it's clearly an enhancement - I accepted your earlier answer 'cos of the other info in it. Anyhoo, it's up on LKML now for the world to see and discuss. One msg came back and it appears DB2 also acts similarly to WAS, hopeflly not just IBM will benefit.
paxdiablo
You are sorry, I overlooked this very important detail.
Blaisorblade