I'd argue the differences are not just style changes and even "just style" changes are not trivial. The kernel style guide specifically mentions to avoid using conditional compilation where possible, and as a result, Michael Ellerman's patch is far cleaner.
> I'd argue the differences are not just style changes and even "just style" changes are not trivial.
Op did all the research and multiple implementations of a fix. Ellerman refactored the fix to be simpler, but the effect is the same.
> The kernel style guide specifically mentions to avoid using conditional compilation where possible, and as a result, Michael Ellerman's patch is far cleaner.
It more seems like he knew about IS_ENABLED and Miculas didn't, because he uses `IS_ENABLED(CONFIG_PPC32)` instead of Miculas' `#ifdef CONFIG_PPC32`. Besides, Ellerman's changes are all inside `#ifdef CONFIG_PPC_FPU_REGS` blocks anyway so I'm not sure this was a major consideration. Using `IS_ENABLED` gets you 90% of the way to the "cleaner" patch, and in a ~30 line patch I don't know if it's worth golfing further.
Oh he also got a code review giving him exactly these tips [0]. IDK, getting real "I don't want to coach this rookie, I'll just do this myself, thanks for the tip" vibes from Ellerman here. Maybe that's valid, but it seems like not a wonderful way to keep people interested in kernel dev.
I'm just disagreeing that the patches are the same. I agree that Miculas did do the hard work by finding the cause of the issue in the first place - but he's correctly credited as the reporter.
As for the clean patch - part of the issue is that Miculas was slightly overengineering some of it with the additional macros and added noise with extra conditions on the fpidx declaration.
Ellerman's "I don't want to coach this rookie" vibe is somewhat understandable given this is the security mailing list - it's just not the time and the place to be going back-and-forth.
I generally agree and gave you an upvote. The macros up top were hard for me to grok too, mostly I was reading it thinking, "why isn't this just a cast" and it turns out is could be! And I get this is the security mailing list, but it's 32-bit PPC, and they took days to really get to it. FWIW I think Ariel was being overly cautious; it's hard to see how this could be a significant security issue, and it feels counterproductive to punish him for being cautious. I can kind of see both sides, but I just can't get over doing all the work Ariel did and only getting a "Reported by" credit. His feelings are justified IMO.
I'm sure other people did the "hard part" too for the Row hammer and Heartbleed bugs; which were even harder. All they got was credit in identifying the bugs; kernel-contributor-status is not a prize for doing hard work finding bugs.
* OP didn't identify the bug, it had been reported years prior. OP identified the root cause and suggested a working fix.
* What then is the hard part, if this is the "hard part"
* What is kernel contributor status a "prize" for then, if not for contributing to the kernel?
* Why all the focus on OP wanting recognition for the work, and 0 on the dev who merged the patch. Why not have all patches be merged by a committe/ someone who isn't the author.
The maintainer (not random dev) didn't "merge" the patch - they rewrote an entirely new patch. Ate you arguing OP should be credited for that work? Sure, the maintainer could have guided OP towards the same patch over multiple exchanges/days, but it is their prerogative - they have to maintain the code going forward and are not obliged to accept patches that are in rough shape (in their judgement). OP did contribute to the kernel - but not by having their code included as they had hoped.
Michael Ellerman is between a rock and a hard place here: he could have done better and that's something that may have to be added to the guidelines for the patch contributors, at the same time maintainers have a massive workload and this was a tiny, broken patch submitted in a non-standard way by a new contributor. He could have made the origins of the few lines of code clearer but maintainers do this all the time, they usually are more focused on the quality of the code, and getting a fix in (even if it isn't yours) and may not realize that the contribution is insignificant compared to the fact that someone is very much focused on the credit.
Incentives clearly aren't aligned and I think if the OP would have taken this up privately they could have worked it out. Instead you get this public attack on people that we already have far too few of and one that misrepresents the interaction. That's where I draw the line: you keep your dirty laundry inside until you've exhausted every avenue for redress if you really care that much. Note that Ellerman explicitly offered to work with the OP if he wanted to be credited for a patch to the kernel on another bug, which is one way to differentiate between drive-by contributors and long term relationships.
All in all I think everybody could have done better here but Michael Ellerman's wrongs are far less clear to me than what the OP did and I'm fairly sure if there had been a ready for inclusion patch or better guidelines about how to deal with various degrees of crediting contributors (and probably for a more substantial fix) that there would have been no problem either.
Personally I don't see the problem at all: the LKML thread is archived for eternity, the contribution of the OP is clear, if he wants to say he's contributed code to the kernel I don't think anybody would object to that and Ellerman did his job as a maintainer, even if he could have handled it with some more grace I'm more than willing to forgive him. If I had been in the OPs place I would have probably jumped at the opportunity suggested by the invitation, and I definitely would not have 'paraphrased' the interaction or use the word 'robbed' without first reaching out to Michael Ellerman because those two things alone undo any goodwill created by the submission of the patch.
