On Sat, May 16, 2020 at 6:39 AM Peter Stuge peter@stuge.se wrote:
Holy moly..
Furquan Shaikh wrote:
- Reland new allocator guarded by a Kconfig:
I looked at this and the original change.
It's nice that the new algorithm is well commented!
But I could not find any detailed explanation of the neccessity for touching much less *rewriting* this quite literally central piece of code in coreboot.
All I could find was "prepare for 64 bit resources". That is beyond weak.
Even worse, I also could not find any explanation of how the new algorithm is different from the old, neither in commit messages nor in comments.
Patrick and Nico already provided some background in their responses. I had made an attempt to capture the weaknesses of and the differences from the old resource allocator as part of the commit message: https://review.coreboot.org/c/coreboot/+/39486. I understand that commit messages can never be too long. There have been several attempts in the past years to add changes here and there to the resource allocator or work around it for different use cases. So, I tried to keep the commit message informative enough to understand the intent. If you think there is more information that could have been helpful to understand this better, I would be very happy to incorporate that in the latest CL before the changes land back in the tree.
I have to say that I am really amazed and apalled by this change.
I am so relieved that I don't invest heavily in coreboot anymore because I would be furious if I did, not to mention how I would feel if I had been contributing significantly to the existing, working algorithm.
Google, please consider what that means. Regardless of intentions, if you treat everyone else who contributes to the coreboot community with so little respect then you as a primary contributor set an atmosphere where others will not be respectful either. As a result, the project races to the bottom.
I apologize if this demonstrated less or lack of respect. That has never been the intent. In my opinion, one of the things that makes coreboot strong is the respect individuals in the community have towards each other and the work that is being done. And I have always valued that. Personally, I have learnt a lot in this community and this has been an attempt to improve coreboot not just for the things I care about but also making it easier and better for everyone. It is no news that modern architecture is changing and coreboot needs to evolve. This change aims to take everyone forward and not just focus on personal gains.
This is obviously the classic conflict between one strong contributor working toward their special interest (enable particular future work) and the larger coreboot community having a fundamentally different interest (do not break a working core algorithm).
I would say that there were a number of broken assumptions and issues hiding behind the old changes which got exposed by this change. A lot of these issues were just going unnoticed for years now. I don't claim that the new changes are perfect in every sense, but like any other work, I am working with the community to make things better overall.
In such situations, the strong contributor must always carry responsibility, and the responsible action in this case is to ensure that the new code is opt-in, not opt-out.
We should not take for granted that our own stuff is the most important, but rather the other way around.
I want to emphasize that I do not blame Furquan here, but Google the organization. This situation suggests to me that something isn't right in Google's coreboot team. The classic problem would be that there are some impossible (time) constraints, which just ends up being really toxic. :\
I think the situation is kind of opposite. If time constraints were the only thing that we cared about, then like many other changes, this would have been a very isolated change to make that one use case work and add more legacy to carry around.
Kind regards
//Peter _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org