Re: [Qemu-devel] [PATCH v3 18/39] target/mips: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

Hi Richard, Aleksandar.

On 5/8/19 4:32 PM, Richard Henderson wrote:
> On 5/8/19 1:15 AM, Aleksandar Markovic wrote:
>>
>> On May 8, 2019 2:19 AM, "Richard Henderson" <address@hidden
>> <mailto:address@hidden>> wrote:
>>>
>>>
>>>
>>
>> This commit message doesnˊt explain the reason for the change, and why is 
>> this
>> an improvement. The underlyng reason for distingishing between  env_cpu and
>> env_archcpu cases is not explained too.
> 
> It's certainly explained in the preceeding patches that introduce those 
> functions.
> 
> Are you suggesting that it is beneficial to copy-and-paste a common block
> explanation into 21 commit messages for each of target/foo/?


*) Richard:

I tried to put myself in Aleksandar shoes. I believe Aleksandar is
worried about his MIPS maintainer duty, wanting to Ack-by this patch.

It is true that out of the context of the series, it is hard to see what
is the problem you try to solve.

You could copy/paste the explanation you used previously,
with s/$arch/mips/:

"Cleanup in the boilerplate that each target must define."

"Combined uses of CPU(mips_env_get_cpu()) were failures to use
the more proper, ENV_GET_CPU macro, now replaced by env_cpu."

Now to clearly understand this patch we still need to look at the
previous two arch-generic patches
- "cpu: Replace ENV_GET_CPU with env_cpu" and
- "cpu: Introduce env_archcpu".

Also, it is tedious to copy/paste the same explanation, but thinking of
forks or stable branch that cherry-pick not all but some commits of a
series, it might be useful.

Another guess is Aleksandar might have looked at the series cover, which
is not well explained as your v2:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07635.html
I think you mistakenly copied the v1 blurb instead of the v2 one.

So at some point I can understand Aleksandar frustation.


*) Aleksandar:

This series fall under the "Overall Guest CPU cores (TCG)" section
maintained by Richard and Paolo. I think you have to see this series as
a whole to understand the benefits of it.

With the same reasoning, I believe you shouldn't worry to not give your
Ack if you don't feel comfortable.

I think Richard sent this v3 to simply address comments raised by the
previous reviewer during v1/v2, where there was some discussions: I took
it as "this is the last round before getting merged" (unless someone
object).

It is hard to make everybody happy on a such big project, with so many
areas, lines of code, people, culture, etc... I believe we all try to
give our best, neither the commiters nor the reviewers are perfect, but
slowly we help this project to improve :)


Best regards,

Phil.