On Sat, May 16, 2020 at 6:39 AM Peter Stuge <pe...@stuge.se> wrote:
>
> Holy moly..
>
> Furquan Shaikh wrote:
> > - Reland new allocator guarded by a Kconfig:
> > https://review.coreboot.org/c/coreboot/+/41443
>
> 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-le...@coreboot.org
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org

Reply via email to