On Wed, 2019-06-26 at 12:35 -0500, Bjorn Helgaas wrote:
> No argument about it being a mess.
>
> I agree that tweaks clutter the history, which is definitely a
> downside. Do you think these actually change the way things work or
> make the code harder to read?
>
> I think there is value in even minor simplifications that make the
> code easier to understand. Small improvements compound over time and
> expose opportunities for more significant improvement.
Oh I absolutely agree. And I love that your patches come with more cset
comment than actual patch lines :-)
The main issue I've had so far trying to untangle things is the sheer
amount of subtle changes and tweaks that went in over the year without
any useful explanation as to why things are done.
For example, do you have any idea why this:
d65245c3297ac63abc51a976d92f45f2195d2854
PCI: don't shrink bridge resources
Was added by Yinghai in 2010 ? :-)
The main issue with this is that previous to this commit,
pbus_size_{io,mem} would essentially ignore the previous state of the
bridge resources, and calculate from scratch (provided the resources
are unclaimed).
After this, it has this subtle dependency.
This is what broke Lorenzo attempts at moving pci_read_bridge_bases()
to the geneneric code a couple of years ago for example. There may be a
good reason to do that on x86, though it's not explained, but it's
definitely not right if the platform requires a full re-assignment.
Now I'll "work around" it by making that function look at the
assignment policy set by the arch/platform, but it's a fix on top of a
quirk on top of a band-aid. However, what else can we do without
understanding the root issue that lead to that patch being created ?
Cheers,
Ben.
