back to article Huawei dev flamed for 'useless' Linux kernel code contributions

Last week, Linux kernel contributor Qu Wenruo scolded another code donor, Zhen Lei, for wasting kernel maintainers' time with unnecessary patches. In a post to Zhen Lei and the rest of the Linux kernel mailing list, Wenruo said he recently found a patch removing a debug out-of-memory error message from a selftest used by btrfs …

  1. Anonymous Coward
    Anonymous Coward

    Sounds like a new circle of hell from the phbs at Huawei.

    Work 996, and oh, btw, your kpis need opensource contribs in all that spare time

    1. Anonymous Coward
      Anonymous Coward

      Come now..

      You just know they're working on ways to get more time out of you when you sleep.

  2. Bob H

    Huh?

    It's utterly irrelevant to the Linux maintainers as to if the submitter is doing it to maintain internal KPIs or not. It's kernel maintenance that probably no one else wanted to do and serves to make the kernel better.

    If the author happens to be doing scutt work or busy work and it produces something useful for the world? Great! Is it even Huawei's loss?

    Don't get me wrong, Huawei has issues but in this context it seems more like the maintainer has an axe to grind. Want to talk about people being forced to work too hard? Let's also look at Samsung and others.

    1. Anonymous Coward
      Linux

      It's utterly irrelevant to the Linux maintainers as to if the submitter is doing it to maintain internal KPIs or not. It's kernel maintenance that probably no one else wanted to do and serves to make the kernel better.

      Fixes aren't free, because there is a cost to other people in verifying and validating them. More fixes mean each one gets less attention, so pointless fixes reduce the quality of the code.

      And motivation is important because it affects everything that someone does. For instance, if I care passionately about getting something to work, then I will do a better job than if my presence on the project is just a tick box in my career progression.

      1. Roland6 Silver badge

        >Fixes aren't free...

        However, incorrectly commented code does carry an increased maintenance cost.

        I appreciate that most peopal can read a mispelt word and their brain will substitute the correct word, however a typo in the non-comment parts can have a much bigger impact.

        The cost problem arises because the current change tools don't differentiate between code and non-code changes made to the same module. Perhaps this could be mitigated by comparing the compiled binary before and after change output; a comment change would result in no change whereas a code change would result in a change.

        However, going back to my first point, the comment revision does need to be reviewed by someone who has some real understanding of a modules functionality, otherwise the code base is open nonsense comment revision attack, ho hum...

        1. oiseau
          Facepalm

          ... incorrectly commented code does carry an increased maintenance cost.

          Indeed.

          Not only incorrectly commented code: how much unfixed code has been piling up for years ie: won't fix code because it did not affect enough people or because the kernel version was soon to be EOL'd?

          Sure, some may have been snipped out in the next kernel version but most will probably not have and that's just dirt/grime being piled up and festering in some dark corner.

          Dirt/grime which may (or may not) eventually raise it's ugly head and break something which may (or may not) take a lot of time and manpower to fix.

          So yes, it does have a cost whichever way you look at it.

          ----

          The only way forward is lean and clean code.

          ---

          A concept that would seem to have been lost.

          O.

        2. Nathar Leichoz

          > "The cost problem arises because the current change tools don't differentiate between code and non-code changes made to the same module."

          Ironically, if the system separates between submitting typos and real bug fixes, Huawei devs may stop submitting patches because it won't give them a boost in KPI score.

          1. Anonymous Coward
            Anonymous Coward

            Considering the CCP tentacles inside every major Chinese company, I'd say that would be a win.

    2. Dan 55 Silver badge

      It does seem to me that the Linux Kernel code review process is creaking at the seams after the Uni of Minnesota stunt. Don't they triage commits?

    3. elsergiovolador Silver badge

      This could be about building trust with smaller updates and then trying to sneak in a backdoor.

    4. SurdTeamer

      It's not scutt work. Scutt work implies there's something actually to do. This is just shuffling around and removing debug comments. I'm wondering if you've read the article.

      1. teknopaul

        removing debug comments is almost always worth doing

        1. bazza Silver badge

          And, one would think, it would be a matter for minimal review, in terms of time.

  3. Anonymous Coward
    Anonymous Coward

    It sometimes seems too anal and pedantic to submit bug fixes for typos, but if you do so with FreeBSD you get thanked.

    If Linux people don't like it, maybe have a separate list for trivial fixes, or let the "elite" comitters simply ignore low priority fixes.

    A fix is a fix, and unless it's something they don't want fixed, they should be grateful, and shouldn't care who sends it.

    1. nijam Silver badge

      > A fix is a fix, and unless it's something ...

      ... that doesn't require a fix.

      And as for comments, the very best you can hope for is that they don't contradict the code.

    2. Roland6 Silver badge

      >It sometimes seems too anal and pedantic to submit bug fixes for typos, but if you do so with FreeBSD you get thanked.

      I suspect a reason why this contributor from Huawei is finding all these typo's is because they are not a native English speaker.

      Hence they are running the comments through a translation system, where typo's can totally change the meaning of a sentence. Thus this person should in fact be applauded as they are helping to make the source more accessible to a potentially large pool of non-native English speaking maintainers...

  4. Bartholomew
    Facepalm

    Monty Python's Life of Brian (album)

    If a company is using commits as a KPI for an employees salary there is one thing that you can be damn sure of and that is that at least 98% of the commits will be related to correct wording of strings of text and nothing else.

    Monty python comes to mind (who am I kidding, they are always there):

    Link man In the meantime...

    Studioman No, er...

    Link man During? During?

    Studioman Yes. Try during.

    Link man During the meanwhile...

    Studioman No, that's not right. That's not good English. Can you try "during the meantime"?

    Link man During the meantime...

    Studioman No. That's not right either.

    Link man Meantime, meanwhile, mean during the...

    Studioman While.

    Link man During the while...

    Studio man No, that won't work at all.

    Link man Well, it sounded quite good to me.

    Studio man No, I don't think it's very good English, I'm sorry.

    Link man Wouldn't it?

    Studio man No.

    Link man Oh. During... What did I say?

    Studio man During the meanwhilst.

    Link man During the meanwhilst... No, I don't like that at all.

    Studio man No.

    Link man During... Well, what sh.. what should I try?

    Studio man We'll just cut this.

    Link man What?

    Studio man We'll just cut this link.

    Link man Well, you don't need it at all?

    Studio man I don't think so.

    Link man Well, that's all right then...

    1. elregidente

      Re: Monty Python's Life of Brian (album)

      Many years ago, i worked for a lovely little Dutch company which was purchased by a big American company.

      Big American company - and I kid you not - had a metric of developer performance which was - wait for it - *number of lines of code committed*. You had to reach a given minimum to meet the KPI.

      They paid about 25m EUR or so for the company, and I think a few years later, having fired half the staff (I'm not kidding) and run it into the ground, they sold it for about half that price.

      There are upper limits on how *well* something can be done, but there's not much in the way of *lower* limits :-)

      1. elregidente

        Re: Monty Python's Life of Brian (album)

        BTW, in the end, that Big American company was purchased by Oracle, at which point I then knew Oracle were just as bad or worse, since no one in their right mind would have made that purchase.

      2. elsergiovolador Silver badge

        Re: Monty Python's Life of Brian (album)

        That could be American OP to make sure Europe doesn't grow any unicorns.

  5. Ace2 Silver badge

    That’s presumptive

    Lots and lots of this sort of thing gets submitted from all over. To call it out from this one person from this one company is maybe….

    It makes me think of the 27 pointless malloc() variants, where people have decided that it’s better to have a tangled mess of an api than to just do a simple (element size * element count) calculation right there in the code. That got put in (and revised) (and revised again) without any aspersions being cast on anyone’s company or culture.

  6. gerryg
    Angel

    My own comment commit

    S/less/fewer -g

    1. Anonymous Coward
      Anonymous Coward

      Re: My own comment commit

      I can't bfewer that...

      1. elsergiovolador Silver badge

        Re: My own comment commit

        That is pricefewer!

        1. Ace2 Silver badge

          Re: My own comment commit

          Can you help me find a driver for my uint64_t-port ethernet card?

    2. katrinab Silver badge
      Headmaster

      Re: My own comment commit

      “less” is correct for uncountable/plural nouns

      “fewer” is correct for countable nouns.

      Eg, less fat, fewer calories.

      1. Anonymous Coward Silver badge
        Headmaster

        Re: My own comment commit

        But making sure that you're only correcting whole words and not strings of letters contained within other words is importanter.

  7. hayzoos

    More to the Story?

    My gut instinct tells me there has to be something more to the story. The gist of the comments seem to agree that blowing up over housekeeping submits is not right. The clue is the KPI reference and mention of a 996 work culture. One possibility I can think of is that submits from the huawei domain upon analysis have become mostly small numerous comment corrections or the like unnecessarily broken up into smaller submits but spread out over time so as to be less noticeable but produce high counts for KPI. Inefficiency at it's finest driven by phbs. Each submit may require a minimum time for administrativa be it one typo fix or all.

    1. cornetman Silver badge

      Re: More to the Story?

      That sounds likely TBH.

  8. steelpillow Silver badge
    WTF?

    Who's the student here?

    So it's okay for students and other kernel noobs to submit trivial patches - the maintainers understand that this is bringing on new talent. And Huawei is a major corporate contributor who has been building their own distro. Maybe, just maybe, they are expanding their Linux skills base, are unable to source experienced talent locally and are bringing on their own in-house trainees - such a this Zhen Lei.

    Genuine question, asking for a knowledgeable response: do we actually know that this is not the case?

    1. chuBb.

      Re: Who's the student here?

      I don't think that's the problem, more the fact that this particular contributor has repeatedly submitted discrete typo fixes rather than bundling them into one commit, I.e. They fix 100 typos and commit 100 fixes, each commithas to follow a rigorous QA process even for something trivial, like a debug comment which you would only see if debugging That module and never during a normal build. If they had submitted 1 commit of a 100 fixes then that would create 99% less work for the maintainers.

      If there is a noticeable trend of this time wasting coming from a given company then it's right to call them out as a company, inferring kpis and 996 working though is a step to far in my eyes as I prefer to attribute ignorance rather than malice, end of the day Huawei should be doing a better job of bundling trivial commits like these into sensible bundles

      1. steelpillow Silver badge
        Pint

        Re: Who's the student here?

        Thanks. makes sense all round (except of course the nerd in question). Have one on me.

      2. lizjohnson

        Re: Who's the student here?

        Isn't this an auditing vs efficiency issue?

        For example if they submit 100 fixes as a single patch that touches 100 different components then whom ever is analysing that patch would have to check multiple components and some which they might not be familiar with. I guess in terms of efficiency it might be easier to check a single patch with multiple fixes (esp trivial), but in terms of an audit trail that patch just touched multiple components and that seems bad to me.

        I have no idea what is actually going on with this story and it seems that there is a lot of hearsay going on.

        1. Graham Cobb Silver badge

          Re: Who's the student here?

          Feel free to join the list, or read the archives.

          The btrfs maintainer prefers to receive tidyup changes in large chunks. Yes, they require careful review to make sure they really are not changing code, and that changes to comments, text strings, etc are correct, but it is easier to do that for 100 changes in one go, once a year, than 100 separate patches spread at 2 each week.

          One rule you quickly learn if you want to commit to Linux is to follow the rules set by the relevant kernel subsystem maintainer.

        2. chuBb.

          Re: Who's the student here?

          Kinda the heresay is the speculation on multiple motivation for choddy trivial commits.

          The problem is resource/effort wastage of the maintainers time, I doubt If a single commit targeting multiple modules would get that far it would be bounced back to dev and told to split it into correctly targeted commits against each module. But for the nature of these commits (typos, debug log messages, stuff not included in a standard build unless you enable a specific compiler debug flag) it's really a matter of etiquette and being a team player and/or learning how to use git "properly*" more than just git commit -am "I fixed a typo"

          *if anyone has a categoric definition of proper git usage I'd love to see it lol

  9. Anonymous Coward
    Anonymous Coward

    The patch was fine

    The patch was fine and the commit message, frankly, was better than average. The maintainer, David Sterba, approved the patch.

    There are firm rules for this to say what is right and what is wrong. Zhen Lei should not have complained about a patch when it is obviously correct. If you start debating stuff like "Yes it's a typo, but does spelling really matter" or whatever then you end up wasting a lot time having stupid debates. Faster to just fix it.

    Some people feel like newbies are gaming the system to get their patch count up but I feel like it's fine. I was in the top three contributors one year. It's a fun game and I fixed a lot of bugs. Plus it let me get hired to do kernel work for a living.

    Huawei has created a static checker infrastructure which makes it easy for them to generate these sorts of patches. Quite a few are bug fixes. And some of there static checker work is innovative.

    1. This post has been deleted by its author

    2. oiseau
      Facepalm

      Re: The patch was fine

      ... stuff like "Yes it's a typo, but does spelling really matter"

      +1

      Of course, it does matter.

      But not only because it is faster/more efficient to just fix it and not waste time ridiculously debating a non-debatable issue.

      It also matters because it sets a standard.

      And it is not being anal, it is just being professional in what you are doing.

      Make it matter, then other more important things will not get overlooked.

      And there'll be less bugs and festering rubbish in any code.

      O.

  10. Anonymous Coward
    Anonymous Coward

    Open Source?

    My mistake. I thought Linux was/is open source? Sounds more like The United Nations. Let’s not confuse one with the other!

    1. nijam Silver badge

      Re: Open Source?

      s/United/untidy/g

  11. HAL-9000
    Devil

    That explains it

    Is it no wonder the Linux kernel is ballooning exponentially, how many millions of lines of code have been added for the next release... Friday goal based KPI commits must be the explanation. Now where was that web site to download Open Euler OS, it'll be a cert for quality patches.

  12. DrXym

    Fortunately I've only suffered that BS once in my career

    Lines of code as a metric is bullshit. I could write a function in 4 lines or in 40. It doesn't mean I'm being 10x as productive by padding the code out assuming both functions do what they're meant to. If anything it just makes the product more prone to bugs and less maintainable in the long term.

    Early in my career when I worked in a software consultancy a manager who looked like Penfold in Danger Mouse would come around to ask how many lines of code we had written. I just made up a number because I didn't know or care. I tried to explain that it was a meaningless metric for the reasons stated above but it was like talking to a brick wall. God knows what he did with that number because it was the only thing he appeared to do in the place.

    1. nintendoeats

      Re: Fortunately I've only suffered that BS once in my career

      I think everybody pretty much understands that this is stupid now. I have been working on some very ugly code lately, and generally I am committing files that are several hundred lines shorter. If I didn't refactor out all the cruft, it would have been much harder for me to actually fix/extend the software.

      My boss has not complained about this, presumably because she's not a suit.

  13. Anonymous Coward
    Anonymous Coward

    CoC

    So - a perfectly valid (albeit very minor) patch gets submitted, some a-hole loses his nut on the mailing list and the Code of Conduct doesn’t work?

    If you don’t want a cleanup patch then just reject it. Worst case it’ll just come back as part of a bigger change.

  14. nickj2019

    Dev should NOT have been flamed

    New contributors are actually encouraged to start with very simple fixes (like fixing typos in comments) to get used to the patch-submission process.

    As a completely separate matter, any company that pressures employees to work 9/9/6 should be called out regularly and legislators should be pressured to impose sanctions. I'm not pointing a finger at Huawei, actually I don't think it's true that they make developers work 9/9/6. But there are companies that do and they shouldn't be allowed to get away with it.

  15. trevorde Silver badge

    Rewarding the wrong metric

    Worked on a product where the code was written by an outsourced agency. So the story goes, their devs had a KPI to write 200 lines of code per day. Thus, their motivation was not to write good, clean, extensible code but to hit their target number of lines of code. Needless to say, it was a complete and unmitigated disaster. The code had every anti-pattern you can think of and even a few new ones. You should never get mercenaries to defend your castle.

  16. Glenn Amspaugh
    Coat

    Code Fix Submission

    "…and is no that hard to spot.”"

    …and is not that hard to spot."

    My quarterly performance review is in the bag!

POST COMMENT House rules

Not a member of The Register? Create a new account here.

  • Enter your comment

  • Add an icon

Anonymous cowards cannot choose their icon

Other stories you might like