It is your job to do this. You seem to have taken the attitude that you will give the Chrome team exactly what they asked for instead of trying to understand their requirements and give them what they need.
I'd love to see the name calling. I've read it several times now. I see him saying that this random patch out of the blue, without any discussion, doesn't follow any of the guidelines to submit a kernel patch, doesn't make any sense (in fact, seems to reimplement things already in the kernel), and isn't being accepted without a significant rework.
He never, ever, once, demeans the person who submitted the patch or shows a pattern of abuse to that person -- if you look up the definition of "abusive" you'll see it says (emphasis mine): "extremely offensive AND insulting". While the text may be offensive (to some, to my ex-military eyes, this is being nice), he never once insulted the author.
Arguably it's less direct due to the superlatives and exclamations. For example:
> Christ. That's literally the remap_file_pages() system call
definition. No way in hell does "ON_BEHALF_OF_KERNEL" make any sense
in this context.
This could be
> That's the remap_file_pages() system call definition. "ON_BEHALF_OF_KERNEL" does not make any sense in this context.
If someone I didn't know or was unfamiliar with gave me a review like this I'd frown, but since Torvalds is known for this then I'd shrug it off probably. It's a good, thorough review though.
The first is better, IMHO. No need to be so dry and boring in your second example. Also, in the first example, I know the reviewer is completely against my approach. I need some proper ammo to rebut with, or go back to the drawing board. In the second example, it could easily turn into a multi-day back-and-forth until one of us gives up.
With Linux and git being probably the most widely adopted open source software projects ever, it would be a valid argument that Linus's approach is by historical proof "the best way".
I hate to be the bearer of bad news, but that isn't a logical fallacy. A logical fallacy is "A misconception resulting from flaw in reasoning, or a trick or illusion in thoughts that often succeeds in obfuscating facts/truth."[1] "Selection bias" isn't a recognized logical fallacy, in logical reasoning. "Selection bias" is a statistical bias in sampled data.
They put forth a premise, "With Linux and git being probably the most widely adopted open source software projects ever"
Then an inference, "Linus's approach is historical proof"
And a conclusion, "is the best way [to review code]".
Assuming we can accept the premise as true, you're left to attack the inference (whether or not Linus's approach is historical proof). You may disagree with the conclusion, but calling out a logical argument as a logical fallacy, and using that to dismiss the conclusion, is itself, a logical fallacy called a "fallacy fallacy"[2]
I understand why it could be selection bias. Let's say there are 100 open source projects with openly hostile communication. Only Linux succeeds. You can't point to Linux as proof that hostile communication results in success based on that 1/100 chance.
Similarly you can't just use a single anecdote to prove anything either.
I agree that the argument could be expressed better, but I personally understood the intention.
Yes. They could have taken a moment to explain wtf they were talking about in both comments. It would have saved everyone a few moments of their day.
But sure. We can debate the premise.
If even only Linux and Git succeeded, out of hundreds of projects managed with hostility, that’s two projects; one guy. fWIW, out of hundreds of every other projects NOT managed with hostility (and similarly running on as many computers), most of those, that I’m aware of, aren’t flourishing like Linux is. Usually they suffer from attracting talent and people. Perhaps the hostility of Linus makes the projects more visible (bad news is good news in the PR world), and some people take it as a challenge to succeed there. Thus the project flourishes, vs. fail from lack of talent.
I’d also argue that I’ve (personally) let some truly shitty code into code bases at $dayjob over the years because there was simply no “nice way” to say it was so terrible. Trying to argue on technical merits would just go round-and-round until I gave up. I’m pretty sure I’ve used the same tactics a couple of times to merge some shitty code I don’t care about and actually want someone to rewrite if they ever need to change it.
The point is, had we have been hostile (and managed to not get fired), the code could be hundreds of times better. So, I’d be willing to bet, there isn’t a selection bias here and hostile behavior attracts talent (due to being highly visible; popcorn factor), forcing people to actually rethink really shitty code, and letting people be more candid about how they really feel.
I attack with laughter and sarcasm. Other people have different coping mechanisms. Mine just happens to be socially acceptable atm. It wasn’t always, such as in school where I was accused of not taking things seriously.
His derisive "Congrats!?" would already be in violation of modern American corporate communication guidelines and policies. That sarcastic praise made me feel belittled. The exclamation mark made me feel threatened and unsafe. Failing to coddle me is aggression.
I agree with the final sentiment but I do think you gave a good example of a more direct response. That said, there's something charming about seeing Torvalds's attitude come out. People really care about Linux and it warms my heart to see some passion.
And in spite of the superlatives and exclamations, the author still didn't seem to get the hint that they should maybe go back to the drawing board. See their post which Matt Wilcox is replying to.
I've worked in environments where it was considered a little mean even to use the "request changes" button in GitHub, but it worked because people (1) got the hint when you left comments suggesting changes and (2) would be careful to address all of your suggestions before requesting review again. But I've also worked in an environment where people would try to sneak changes past you and then argue with you when they were caught. I don't think I've ever sworn in a code review comment but these days I can understand where Linus Torvalds is coming from. Things would be much nicer if people got hints and were careful, but we don't live in that world.
My friend tried to use a credit card in Japan, and instead of saying "no" the shopkeeper just bowed and said "excuse me." Meanwhile yesterday in San Francisco I saw a customer arguing with the cashier for five minutes at McDonalds about whether or not they could use a coupon twice.
Funny, I see people getting extra bitter and mean in the last decades or so and call that pathetic. The "fragile" people are the ones who are taking appropriate offense to it.
Anyway there is no possible argument that the meanness of those replies is productive; it would clearly be more useful and direct to omit it and speak clearly about the problems instead of implying the person is an idiot for not understanding their mistakes.
Moreover, why are you "surprised"? Do you live in a some toxic alternate reality where people talk like that regularly? For most people these messages are the only time they'll ever see someone being that mean in a professional context in their lives.
Can you imagine if there were no consequences for submitting shitty code to the Linux kernel team.
You'd have every first year college student trying to get their name on the mailing list with some crappy commit that adds no value and only takes time away from meaningful work.
Then you've got to hand hold and coddle that person, because you have to be nice to everyone. Yeah that's not gonna scale very well.
I can see going easy on independent developers. But this is Google. One of the biggest licensors of Linux and a closed source competitor. Here they are adding a feature to a repo they need to use, and they're doing it half-assed.
This is a feedback loop. Don't submit shit code to the most important repo in the world. Ain't nobody got time for that. If you do it as a fortune 500 company we will publically shame you.
Some of y'all have never been earnestly told to get your shit together and it shows.
Somehow you seem to think that "giving a highly-critical code review" requires "being a dick" and you're just wrong. Actually, the not-dick code review is a lot more impactful than the dickish one. The dickish one just makes people want to quit instead of getting better.
I cannot fathom the internal mindset of a person who says "I'm being a dick, which I know is cruel and bad, but it's okay because of <various political and social factors>". The "knowing it's wrong part" wins! That what it means for something to be wrong! It's still wrong if you rationalize it!
And that's before the fact that it's also worse at its actual goals, like preventing bad code and maintaining community and giving valuable feedback.
> Can you imagine if there were no consequences for submitting shitty code to the Linux kernel team.
Why do there need to be consequences? Can't the maintainers simply ignore shitty code?
> You'd have every first year college student trying to get their name on the mailing list with some crappy commit that adds no value and only takes time away from meaningful work.
Trolls could already do this if they want. In fact, getting a rise out of Linus would be a troll's dream. Instead, the team could (should) simply ignore shitty code. Am I missing something?
> You're in your mid thirties, and still talking like this.
This constitutes a personal insult (calling him immature for his age), so you aren't walking your talk. In this comment you're going much further than Linus's review being discussed, which doesn't contain any personal insults.
Maybe you think you're giving zelon88 a taste of his medicine like this, but if truly believe that people are obliged to communicate in a corporate-friendly manner you wouldn't be dishing out personal abuse like this to make a point. Overt tit-for-tat abusive communication doesn't fly in a modern psuedo-friendly corporate environment; you have to be more clever about it. In other words, you should lead by example.
> Clearly nobody has ever done it to you!
You really think nobody has ever been curt or even rude with zelon88? That beggars belief. Of course he has been on the receiving end of it; everybody has. Don't waste your time with the "you don't know what it's like" argument, nobody is going to believe that. You're wasting your time and undermining yourself with this tit-for-tat approach.
No insult, just bemusement at what i see - perhaps incorrectly, but where's the fun in being reasonable? - as inattentiveness regarding what in my experience have proven effective communication styles when dealing with disparate people working towards a common goal. And, yes, there's an element of playing to the crowd in my comment, that I'd assumed would be so obvious that it doesn't need breaking down as you have done. But perhaps not.
Outside that sort of situation, of course, do what you like - just as I did.
I'll repeat what I said elsewhere in this thread. Linus insulted the code, not the person who wrote it. In engineering, conflating criticism of your work with personal criticism of yourself is a sin.
Yeah, you were wrong elsewhere in the thread also. By treating the person who wrote it like shit, he insulted the person who wrote it. Words like "pointless" and "obvious" and "christ" and "illogical" and "worthless" are not emotionally uncharged words. Using those words is about communicating how he thinks the person, not the code, is a moron who needs to feel bad about their work.
If an engineer can't receive criticism of their work without getting their feelings hurt, then other engineers they work with will be inclined to softball criticism of the work to preserve that engineer's feelings. Letting inter-personal considerations get in the way of engineering is a liability to the process. Engineers need to have thick skin and remember that they are not their work.
> Anyway there is no possible argument that the meanness of those replies is productive; it would clearly be more useful and direct to omit it and speak clearly about the problems instead of implying the person is an idiot for not understanding their mistakes.
Honestly, code review like that would make me absolutely want to work elsewhere. It is absolutely a "good" way to make plenty of people want to have nothing to do with the project, or the reviewer personally.
Even if someone writes code that is bad, that can still be treated as a learning opportunity, with examples of what might be better, or what to avoid. Neither of those call for exclamations, or adopting a tone that makes it seem like the person has an attitude. If being neutral/professional is somehow difficult, then being terse is also an option that's even easier.
> Funny, I see people getting extra bitter and mean in the last decades or so and call that pathetic. The "fragile" people are the ones who are taking appropriate offense to it.
No, sorry. Businesses bend over backwards to coddle people now. Things were much more straightforward a couple decades ago. You could tell people straight up they were doing a shit job. Now you need to dance around it to avoid upsetting people who are wasting people's time.
There is no equivalence being "being mean and cruel" and "telling people they're doing a shit job". You can always tell someone they're doing a shit job in a non-mean and non-cruel way. In fact it is more effective.
The issue isn't directness. The issue is repeatedly taking needless swipes at the author's competency or intent ("worthless", "complete nonsense", "totally illogical"), when Linus' criticisms are really mostly just small (if valid!) gripes about naming conventions and readability.
This is particularly funny because I remember that several years ago, he publicly apologized for being abrasive and promised to get better... but old habits die hard.
He has every right to run the project as he sees fit, but then, people here have every right to be mildly cranky about that too.
Maybe it's because I'm not American but I don't see that language as unwarranted nor particularly severe? (Also not insulting the authors competency; the strong language is mostly on the technical merits of the commit, not the authors own technical merits.)
Like I've read some of Linus' older swear-laden rants back in the day; those could get unreasonably angry at people for what amounted to things like inconsistent commenting and basically had shit in them that amounted to "the author shouldn't ask me for anything ever again". That's not present here.
This is a direct code review that calls out (from what I understand) a number of bad design problems (hence: totally illogical and complete nonsense) and the fact that the author doesn't seem to have followed the kernel commit message rules. (For which, yes, the moniker "worthless" is appropriate for those messages - the kernel has pretty strict commit message rules.)
It's strong language but strong language of this stripe is only bad if it's excessive or attacks the wrong thing. I don't think it's excessive in this case; the patch seems like it's a poorly thought through solution and is a technical mess for a project that has strict rules about user space APIs (aka if a poor solution ends up being used, it'll be maintained for ages).
In general the response from Linus reads more like that of a certain type of uni teacher; "this is bad, go back to the drawing board, here's what you did wrong".
Finally, also keep in mind that this patch was submitted by a Google employee (they're representing their employer) and that as far as I understand it, by the time you get to contribute to the mailing list that has Linus doing code review, you're at least expected to be familiar with how the Kernel works - he's not in a position where he can scare the newbies[0]. I'd not put up with it if it were a colleague but that's also really not the dynamic here. Google is more akin to a third party vendor than anything else.
[0]: Some of the lower tree maintainers are though.
> swipes at the author's competency or intent ("worthless", "complete nonsense", "totally illogical"),
These remarks are directed at specific problems wit the code. Linus did not call the author worthless, nonsensical or illogical. In engineering, conflating criticism of your work with personal criticism of yourself is a sin.
It strikes me as a weird argument. This is your work. You presumably did your best. If another person calls that work "complete nonsense" or "totally illogical", they are implicitly saying that you failed at basic cognitive skills and produced complete garbage.
Which, you know, might be justified every now and then, but it's not justified in this case.
If an engineer can't receive criticism of their work without taking it personally then they're a liability to the project. Taking criticism of your work in stride is an essential part of the job.
No, I read that part, but they're still small gripes. He has no objections to the idea itself, and no reservations about the overall design. He gripes about syntax. Let me be clear: I think the crux of his feedback is solid and is a matter of good engineering. It's good to upkeep these principles for the Linux kernel. But it is very much "fix it up and come back".
I worked with a well know hacker/author at a company and while most people wouldn't step foot near them I saw it as an opportunity to maybe learn more. They were brutally honest in code review, but never personally mean. Yeah, I had some code reviews that looked just like this, but it motivated me to try harder. I learned a lot from this person and eventually gained trust and some friendship on top of additional knowledge. Don't be scared of a code review, be happy that code reviews will give you new insights even if that means the work you have now is wrong or requires refactoring.
Yes. We need more pointing out when people do idiotic things, not less. Otherwise, in a few generations, we'll be wondering why you would water plants with toilet water.
No, but that's not the relationship between Google employees and the kernel either.
Google is functionally a third party vendor submitting their own solution for a technical problem the Chromium team wants fixed by changes to the kernel. That's the lens through which the response should be seen imo.
Google isn't a coworker, a teammate or a friend in this situation. They're a third party entirely.
Certainly. That said, I do think that Linus Torvalds is treating the Google employee respectfully here, given the dynamics and context at play.
Firstly: Torvalds does not owe Google anything beyond basic respect for sending him a patch; they don't pay his wages, nor does the LKML have Google as a business customer (Google does donate to the LF but only 3% of the LFs actual funding goes to kernel development and therefore Torvalds' wages). That basic respect is given - Torvalds does not insult or demean the submitters technical skills, merely the implementation they ended up going with.
Secondly: As far as I can tell, this proposal was brought forward straight up with an implementation, rather than consulting beforehand with the Mailing List on if this is a good idea and what such a contribution would look like in the context of the kernel (given that the implementation isn't a bugfix). Especially for more "mature"/"large" projects, dumping patch proposals that add maintenance overhead as a form of "discussion" as a submission tends to be seen as a hostile move. This is just general FOSS good behavior and from what I can tell this patch went through Googles internal discussion lists rather than the LKML. Google, perhaps to some suprise, does not own the LKML. The rudeness in that regard is mutual and was arguably started by the patch being submitted in that manner to begin with. (Something which is indicated further in the thread by the fact that the author attempted to start a new patch series for their proposal rather than discuss the feedback they got, showcasing they have little care for actually working with the LKML to get the patch merged).
Thirdly: I frankly... just don't think this feedback is that harsh in the light of that dynamic? Sure, some of it uses strong language but I've been on the receiving end of way worse from university teachers and the like. "This is a fundamentally bad implementation that doesn't follow LKML guidelines, but I like the idea" is what a lot of it comes down to. Calling something bad is not a mortal sin; if it sucks it just... sucks. The specifics of how you respond to that come ultimately down to a complex set of interpersonal relationships, but given how this was proposed, the tone of "yeah don't like this implementation because it does thinks I think make zero sense and you broke LKML rules, go back to the drawing board" seems adequate.
I do think that there might be a bit of cultural dissonance at play though. The way the US (and to a lesser extent the UK) tend to handle feedback is just... a lot more "couched" in niceties. That's just... not as much the case in other countries. I can only speak for Europe but if you get feedback here, you get the feedback very directly - this is what the other person thinks of it and if they actively dislike it, you will hear it. In the UK/US this tends to be a lot more muted from what I've seen - actively calling out bad things as bad is discouraged (a frequent example is when someone says "it's alright" and they actually meant "I really dislike this idea" - in Europe you'll often just hear the latter), so when someone actually uses direct language it's seen as too aggressive.
Can you picture it as the passionate founder of the largest open source non-profit in existence telling a highly paid software engineer at the largest closed source tech conglomorate in the world that the self-serving code he's pushing into the repo you've been maintaining for 30 years is trash, because it is trash?
Yet it would be so, so, so easy to do that in a way that isn't so mean and sad to watch.
It's a passionate founder with an abuse problem doing something that doesn't require abuse but being abusive anyway and, because they're too important, no one can easily prevent it.
First, if you fly off the handle every time somebody pushes some bad code, you might want to find some help, because it's going to happen a lot. Everybody writes bad code from time to time, and the more people you work with, the more likely you are to encounter cases. Linus is sure working with a lot of people.
Second, Linus has always been rude. This isn't because his beloved work of 30 years got sullied, it's because he has a temper and can get away with it. He had that temper back in 1991, and he's been at most slightly successful at tempering it since.
He's repeatedly making the choice to be that way. He could choose a different response, but he prefers replying this way, for whatever reasons.
You can argue if that is or isn't OK. But either way, it's a choice to be rude, not an innate emotional response because some dirty peon has sullied the œeuvre of the great artist.
It's funny how people use word "passionate" as a synonym of toxic and abusive.
I had "pleasure" of working with this type of people with inflated egos and too attached to their work, so I can certainly picture it, but not in a way you think.
You've added a bunch of irrelevant qualifications to a, to borrow your language, highly paid jerk in power enjoying being a jerk. This is also much easier to picture and much closer to describing the situation
It’s a bad review. The snarky prose is annoying. He could be much more concise as well. “Won’t merge. Commit messages need far more detail. Don’t do bitmask translations. All of these operations should use the same mask instead of creating redundant ones. Rethink the logic here, not sure if it makes sense. This needs a rewrite, not small tweaks”
…was really all it needed. The wall of text is more offensive than the insulting tone.
Your review doesn't take the time to explain why something is a bad idea though. I didn't think his tone was that bad. I think it depends on how you imagine the conversation in real life. I would prefer to have my review be blunt but instructive
Where is "why" in this repetitive review? Also, what is the values of bluntly repeating in a more aggressive style vs bluntly saying the same thing once in a more professional manner while also explaining the reason (lack of details)
> First off, the simple stuff: the commit messages are worthless. Having
check seal for mmap(2)
as the commit message is not even remotely acceptable, to pick one
random example from the series (7/8).
Try reading the rest of the review and you will see multiple explanations for why things are not a good idea. I don't even think your cherry picked example is that repetitive. It sounds like a normal conversation to me.
I've read the rest of the review, but "cherry picking" another example wouldn't help when you ignore the issues of the first one and have to add qualifiers (yeah, it's not "that repetitive", it's just repetitive)
Ordinarily I'd agree, e.g. within a company or team, I would not accept this kind of tone, it's not how peers communicate
But it's also not Torvald's job to tutor here or be a mentor. He has limited time in the day. And the company doing the proposal here is Google. They probably should have come in with a more-better-finished product.
The consequences to design errors in kernel syscalls are rather intense.
> the company doing the proposal here is Google. They [...] should have come in with a more-better-finished product.
rm "probably"
The PR submitter represents a software giant, love 'em or hate 'em.
It is an understatement to say that the Linux kernel is an important, global-scale software project. A small number of busy people maintain the kernel's design. This is one project where crap should not be submitted.
Whether or not we like the company, a software giant like Google should a) understand the scale of the project and the importance of software design, and b) manifestly understand the rudiments of code quality.
The part that I think many people have a hard time internalizing, is that this is almost certainly following Google internal "best practices" for how the code is organized and delivered. They famously have "code reviewers" that are there to make sure the code reads well.
You can see it in the translations of having 3 flags for the same data. Somewhere, the distinction was made between what the user is asking for, and what the system works with. It is considered a good thing to have that level of abstraction in many large companies. Heck, many small companies go for that.
> Mark Twain who is often connected to this saying did not use it according to the best available research, but one of his tangentially related quotations is given later for your entertainment.
> In 1871 Mark Twain wrote a letter to a friend that included a remark about the length of his note. Twain’s comment did not really match the quotation under investigation but it is related to the general theme:[ref] 1871 June 15, Letter from Mark Twain to James Redpath, Elmira, New York, UCCL 00617 (Union Catalog of Clemens Letters), Mark Twain Project Online. (Accessed marktwainproject.org on 2012 April 24) link[/ref]
quoting Twain:
>> You’ll have to excuse my lengthiness—the reason I dread writing letters is because I am so apt to get to slinging wisdom & forget to let up. Thus much precious time is lost.
Yeah I mean that's maybe fair, and I thought Torvalds had been making attempts to avoid this kind of tone. I still think even if he had been concise and terse he would have been perceived as slightly-rude though.
The proposed summary of the review is too terse and doesn't make any sense. I don't see any redundant prose in the original one.
His concerns are clear, and he gives actionable suggestions for improvement. The tone + his opinion of the patch set is also clear.
Would you rather get a code review that is full of vague euphemisms and tries not to hurt your feelings, and then spend months wasting time on a dead end approach because you thought minor tweaks would work?
I'd much rather get one message that says where the bar for merging is so I could fix my code and get it merged. I absolutely wouldn't care if the latter was a bit sweary.
I've worked at a place where all communication must be in professional tone and polite, and you cannot give valid technical feedback that might hurt someone's feelings.
The verbal communications standards were weaponized by sycophants, who dominated middle management. It was the most passive-aggressive and hostile work environment (as in legal liability) I've ever encountered.
My impression was he was avoiding outright abusing people. Abusing their ideas as stupid was always going to be on the table for him. (Not justifying it but I think people are fooling themselves if they’re imagining a purely softer, gentler version of him all around.)
> Rethink the logic here, not sure if it makes sense.
That invites pointless discussion when most of the points in Linus review are a clear "hell no". Quite sure the last thing someone in Linus position wants is never ending discussions of already rejected ideas, it only wastes time on both sides.
Sorry, I must missing something here, but how is he being a raging asshole? Looking through the linked code review, he's definitely very direct, and I can see how profanity is often uncomfortable to people who are used to formality, but he doesn't seem to attack anyone personally or focus on anything besides the technology?
He is being a raging asshole by behaving like one?
How is it ok to berate a contribution like that? Is it encouraging more people to contribute? Is it creating a positive environment? Let's park that for now, is it helping the technical discussion at all? It's just power tripping on the gatekeeper status.
All of this could be said in a technical, professional manner. Many many great programmers do it everyday, sorry Linus, nobody is a God. People are not anybody's toilet paper.
I would much rather have someone like Linus who at very least I would understand very quickly than a "leader" who is distant and only makes a fuss post-hoc
That's not true, but even if it were, there is still no good reason to do binary reductionism and present the principal components without the ancillary ones
I used to think these Linus smack-downs were pretty funny. I guess I still kinda do, but I'm not proud of it. These were more appropriate when Linus was an underdog - now he's just being an asshole, or at the very least unprofessional. I've contributed Linux kernel code, thankfully several rungs below anything the God emperor would be looking at - I would have been wrecked by a review like this.
The person confidently submitting this without understanding what they're doing is a serious problem. The kernel maintainers only have so much time. Spending it reviewing things that should have never made it to their inbox is going to piss them off.
He did teach, if you read his words and don't take them personally - there were great explanations for why things were bad. Even still, as the man at the top, it's not really his job to encourage and teach.
Nobody emails the CEO of their org with a terrible idea due to complete lack of context, and then gets support from everyone saying "I can't believe he didn't take the time out of his day to nicely explain why you were wrong here!"
Linus is not the CEO of the Linux Foundation. His job is to review kernel patches. This email was not sent directly to Linus. Even if it isn't his job to encourage and teach it isn't his job to be unnecessarily rude in his reviews.
Teaching is way less efficient than filtering, unfortunately.
I used to think we should strive to make everybody grow. But the more I've been confronted with hard challenges and limited resources, the more I've had to filter out.
Now I lean toward "filter first, teach to what remains, repeat".
And yes, being tactless is a very good filter, even give the obvious side effects.
2016 terminology for shutting down people calling out bad behavior. Lazy. Going to call me a Karen next? Seriously, the world is not your sock puppet, boss. You’ll get just as much done being decent.
If reviews of bad code can waste their time to the extent that it's a serious problem, the kernel maintainers failed to set up a sustainable process. If there is a volume of things that shouldn't get to your inbox, you make sure you have filters in place, you don't berate the senders.
Screaming loudly instead of addressing your own failure is of course a choice, but it's not a great one.
> If reviews of bad code can waste their time to the extent that it's a serious problem, the kernel maintainers failed to set up a sustainable process.
This patch is coming from google, are you suggesting that google just be added to some black list they have?
No, I'm not suggesting a blacklist. I'm not sure why you'd think "sustainable process" implies a blacklist.
What I am suggesting is that if you're so stressed out by the time you spend on reviewing things that you yell at people, maybe you should hand of some of the reviews to somebody else. (Especially if you believe, as OP - but not Linus - did that it "should have never made it to their inbox")
Yes, I understand that there's a limited number of kernel reviewers. And if the number is too low to deal with the flood of change requests, that's the definition of an unsustainable process.
I can't speak to the current organizational structure but historically there's Linus and a few others at the center (e.g, Greg KH) and then you have circles of trust extending out. Someone is the key person for subsystem A and they trust someone below them for driver or widget B and you're expected to pass your patches through these channels and by the time they make it to Linus's inbox you should have a branch/patch with commits that follow all the rules and are ready for a final QA inspection, essentially.
Sometimes people and corporations with enough clout manage to slip by the normal processes and this is the result.
Google has a large number of experienced and dedicated kernel engineers who are well versed; it should have gone through thorough internal review with them first.
It must be a cultural thing because I don't find those reviews devastating.
I would be fine being the recipient of this. It's simple, direct, clear, and give you a good sense of how bad things are and the priority of it.
If you fuck up, you fuck up.
I feel utterly disgusted by the opposite however. The "I like it but I don't love it" and other BS.
Feels like hypocrisy and frankly, behavior from people that don't have enough serious problems in their life to assess what is important and what is not.
Which is arguably exactly why Linus should communicate more thoughtfully. While it might be okay for him and the culture around him, this style might NOT work for others. As such, it's better to default to a more constructive approach.
This is again not a case of "some noob trying his best" but a "someone who should've know better". This is like when some new hire asks you something, and you try to slowly explain, and when Bob does it, and you go "Bob, you've ben working on this project for 6 years now, how the fuck do you not know what this thing here does?!"
It's such a good idea, it is already in the kernel. They literally reimplemented already existing shit with a more complex API. You can't make this up.
The use case here is a process saying "I don't trust my future self not to do something stupid".
Mseal alone doesn't seem enough for that. There are plenty of other API's that would also need locking down - and many future API's too. This looks like a big maintenance burden forever into the future.
Perhaps a better model is seccomp BPF. Allow a process to install a seccomp BPF filter on itself. The filter could prevent calling mmap, mprotect, mremap and friends. The filter could also prevent the modification of the installed filter.
That seems far more general and far more powerful and requires less kernel code with only a handful of uses.
malloc can use mmap (aside from s/brk) - it's impossible these days to sandbox any more complex program and not allow mmap (arguments can be limited though to only a subset of flags).
pledge(2)? That’s generally about blacklisting syscalls though a few options allow carveouts (mostly around file system access). Sealing memory mappings is a lot lower level, and more fine-grained.
Well if Linus had worked with any android apis, he wouldn't be the least surprised by completely illogical APIs coming out of Google (or making once completely logical APIs illogical).
My experience is that Google code reviews tend on the whole to get so caught up in stylistic and micro-level technical comments that overall design gets lost in the shuffle. Especially in something really large and old and evolved like Android or Chromium.
In general the code review process can work at Google for most projects because the essential design questions are already set in stone. You use protobuf/gRPC, follow the patterns that other services have mandated. There is usually a "recipe."
And within Google design problems are perhaps sometimes addressed during PRD/design-doc phase, in a big Google Doc. But not always.
But I can see how this could completely fall over when these things go out into the external world.
That and the need to make everything into small microlevel reviews that get nitpicked for days means that large wholesale architecture simplifications and refactors just don't really happen very often. Few projects have people with that kind of authority.
The android codebase is so huge there is nobody who really has a good view at all of it to make architectural decisions.
Also internal politics sometimes impacts technical decisions ("No, you can't add an API to our teams codebase - we're underresourced and don't have time to review your changes till Q4 2025").
Only for people who haven't developed for any other OS :P
There are some APIs that are real stinkers (and quite a few of them obviously targeted at use of a single Google app and noone cared to actually make them good APIs).
Several of them are really well thought out and designed to the point that iOS devs were envious for years.
Gain some, lose some. It's not the best API surface, but it aint GTK or Win32 either.
I haven't worked with GTK, but I can agree that Win32's APIs are also a mess, affected by decades of layers of compatibility. The WinForms API, however, still tends to be quite nice.
I would argue that a lot of public APIs are a mess. We used to joke that instead of firing the bottom 10%, Google just reallocates them to the Android team.
Except you're talking to a former Nokia employee that knows enough about Symbian and J2ME, and had enough of Dalvik falsehoods regarding JVM implementation techniques on constrained devices.
Yes, I remember meeting many Nokia employees at that time (when there was a still a Nokia Symbian vs. iPhoneOS vs. Android race in progress) and they all seemed to be on the wrong planet when it came to developer mindset.
I distinclty remember Nokia trying to sell us on Qt app development... where apps were only actually able to run on like 2 devices out of their 100+ device portfolio. It was hillarious how misguided they were.
That's a really good point but thinking about it that way brings the decision to use Java at all into question. Clearly iOS/iPhones did great with ObjC in the same era.
Agreed, one of the API's that has undergone superfluous changes that are hard to reason about. My favorite one is `service.stopForeground(...)`. It used to take a true/false which meant either remove the notification or not. Such control coupling is a questionable design decision in the first place, but then the API was morphed to now to take a tri-state flag that, from what I can tell, means essentially the same thing as the old boolean flag, but let them avoid fixing a bug in old behavior. For over a decade, it seems that no one has stopped to ask why notifications and service foreground state are coupled so in the API in the first place.
Yeah, that's not a bad thing (Notification implementation in general is one of the parts of Android that's consistently better than iOS in most user research).
I found nothing amiss in Linus’s response. No personal attacks. Just hitting bad code and even suggestions for future fixes which all show a different Linus, a much more accommodating Linus who he and the other maintainers have the tall task of having to maintain quality in the Linux kernel — software that literally runs (most of) the internet.
As an aside, all this second-guessing and backseat driving of Linus Torvalds is getting a little ridiculous. "Hey nice kernel but could you also behave in a way that I approve as well? Otherwise I'm going to go around the Internet complaining about what a big mean jerk you are. Okay? Thanks."
- - - -
Edit to add: I've read the thing now and it seems mild and reasonable to me.
Just did a code review where I wanted to let loose. But lacked the confidence plus practically I knew it would just poison relations to be expressive like this
Looking at Torvalds' responses today, as an older person who has been through some stuff, it's pretty clear he has serious unresolved emotional issues. It's not even that his comments are dickish... it sounds more like a cry for help to me now. Somebody who's been hurt and lashes out as a way to protect themselves, their ego, or the unfair standard they hold themselves or others to. The fact that he's a boy king doesn't help, because he can always take his toys and go home if anybody mentions he should see a therapist.
What do you think Linux would look like today if the kernel team was friendly and outgoing and willing to put on kid gloves and help all the newcomers? What would it look like if everyone's code were "good enough"?
What would enterprise look like if Linus was polite and held everybody's hand who wanted to push code into the kernel? Would Linux be as far along as it is? Would it be the same quality?
I can't tell if this is sarcasm or not. HN is a hellscape of insane nerds on the spectrum with no EQ.
Do you actually think rudeness directly correlates to code quality? Like, the more of a dick you are, the better your code gets? What kind of sadistic computer science class were you subjected to? Did you watch the movie "whiplash" and take it as a truism?
My take away: jeffxu@google.com has to go back to rethink the whole thing.
Good idea, execution, not so, yet.