Patch Issues Lead to RCU Code Freeze

Paul E. McKenney recently submitted a patch for the Read/Copy/Update (RCU) code, which Ingo Molnar pushed up to Linus Torvalds. Both of them then got a stern rebuke from Linus (to the tune of "no new RCU code will be accepted" – see below), which revealed some interesting details about what kind of patches are and are not acceptable in general. It's noteworthy that both Paul and Ingo immediately acknowledged Linus's point and addressed the issues raised.

Of the patch, Linus said:

I refuse to take that nasty <linux/rcu_segcblist.h> header file from hell.

I see absolutely no point in taking a header file of several hundred lines of code.

We have traditionally done too much inline code anyway, but we've learnt our lesson – and even back when we did too much of it, we didn't put random code that nobody uses and by definition cannot be performance-critical in big inline functions in header files.

If it was some one-liner helper function, that would be one thing. But there are functions that don't even fit on the screen, and that have multiple loops and memory barriers in them.

The one function I decided to grep for was used EXACTLY NOWHERE. Yet it was apparently SO INCREDIBLY important that it needed to be inlined in a huge header file despite being huge and complicated.

So no. This is too ugly to live, and certainly too ugly to be pulled.

The RCU code needs to start showing some good taste.

There are valid reasons to inline even large functions, if they have constant arguments that make us expect them to generate a single instruction of code in the end. But that was very much not the case here.

Not pulling. Try again next merge window when the code has been cleaned up and isn't too ugly to live.

Paul took responsibility and conveyed his apologies, saying that he'd patterned the code after code that was already in the kernel, but just hadn't noticed how big his functions had gotten. He said the next version of the patch would get rid of the unused function Linus had noticed and would create non-inlined C code for functions that were non-trivial or weren't performance critical. Linus said that would be fine.

Ingo, a kernel development "higher-up," also took responsibility for failing to notice that the inline functions had gotten too big. He said, "Header file bloat is a creeping problem that has gotten (much) worse over the last 10 years, so the pushback from Linus against adding more bloat to include/linux/ is fully justified." He offered further suggestions for ways Paul could improve the patch.

A week later, Paul submitted a revised version of the patch, which Ingo again pushed up to Linus.

Linus, however, wasn't done chastising. He said:

So I've pulled it now (although it is showing signs of semantic conflicts, so I'll have to look at those), but I've got two requests, one for Ingo, one for Paul.

Ingo: please don't bother sending me stupid crap.

And by that I mean the whole patch WHEN IT IS 300kB IN SIZE!

That's just idiotic. Nobody is ever going to review a 300kB patch that is ~7500 lines. All it does is waste time and make it a pain to even reply to your emails (since I have "include quoted original" on by default in order to be able to quote and reply sanely).

There's a reason "git request-pull" only does the diffstat and the shortlog.

So please fix your scripts. If a patch is so big that it is not worth reviewing, don't include it. I don't know exactly where that limit is, but I would suggest that it is on the order of 1000 lines or so.

This is particularly annoying, because your pull request is one huge pile of shit. It has all that completely useless stuff that nobody is ever going to look at, and it didn't actually mention the *important* parts, namely how the RCU changes apparently mess with the DRM selftest changes.

So stop sending me stupid crap, and please send me the *relevant* stuff instead.

[...]

And for Paul: the RCU subsystem is starting to get ridiculous. Seriously.

That is *particularly* true for srcu. We don't even have all that many users, and I suspect a large subset of those users are just crap to begin with. The biggest reason for srcu seems to be bad callbacks, particularly shit like the mmu notifier code. Things that we probably shouldn't have done in the first place, and where srcu just encouraged people to do bad things.

Seriously, do this

git grep srcu.*lock -- :^Documentation/ :^kernel/rcu/

and notice that we have only a few hundred lines in the kernel that do srcu locking. kvm seems to be the main big user.

This annoys me, because the main reason people use srcu is bad design and lazyness, where they can't be arsed to try to minimize locking and sleeping things. The "sleeping callbacks" in particular tend to be a huge design mistake.

Yet, despite this fairly limited use, rscu seems to be just growing and bloating, and making more and more excuses for bad behavior.

And it was *years* since I asked you to look at getting rid of the absolutely insane proliferations of different RCU models. I don't think anything ever happened. We *still* have TREE_RCU,| PREEMPT_RCU, and TINY_RCU.

And with this pull request we now have CLASSIC_SRCU, TINY_SRCU, TREE_SRCU, and TASKS_RCU.

That's in addition to all the other insane tweaks that nobody uses (eg RCU_FANOUT etc.) and that I made sure got removed from any sane questionnaire.

Paul, this really needs to stop.

I'm now going to stop pulling any more crazy RCU crap. Seriously. If the RCU subsystem doesn't start shrinking, I'm no longer pulling. Send me fixes, but don't send me more of this crazy stuff.

So this is me putting my foot down. I should have done it long ago. I'm done with crazy. Don't waste your time doing yet another RCU mode, because I will not take it. And don't waste your time expanding on the existing ones without looking at which of those things can be removed.

This is not the most stern rebuke I've ever seen from Linus, but it's very stern. It's very rare for him to direct a developer or a project to stop all work except the issues he deems appropriate.

Still, Paul replied, "OK, nothing more from RCU other than fixes, code reduction, and documentation for the time being."

It's worth noting that if Paul or Ingo had disagreed with Linus, they both would have voiced their objections until Linus had made his reasoning clearer.