Re: [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output

[Philippe Mathieu-Daudé]

Hi Peter,

On 7/27/20 8:09 PM, Peter Xu wrote:
> On Mon, Jul 27, 2020 at 07:45:43PM +0200, Philippe Mathieu-Daudé wrote:
>> When different regions have the same address, we currently
>> sort them by the priority. Also sort them by the region
>> size.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  softmmu/memory.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index af25987518..c28dcaf4d6 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -2960,7 +2960,8 @@ static void mtree_print_mr(const MemoryRegion *mr, 
>> unsigned int level,
>>          QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) {
>>              if (new_ml->mr->addr < ml->mr->addr ||
>>                  (new_ml->mr->addr == ml->mr->addr &&
>> -                 new_ml->mr->priority > ml->mr->priority)) {
>> +                 (MR_SIZE(new_ml->mr->size) > MR_SIZE(ml->mr->size) ||
>> +                  new_ml->mr->priority > ml->mr->priority))) {
>>                  QTAILQ_INSERT_BEFORE(ml, new_ml, mrqueue);
>>                  new_ml = NULL;
>>                  break;
> 
> Note that this change could make the outcome unpredictable... Assuming two
> memory regions:
> 
>   mr1: addr=0, size=0x1000, pri=2
>   mr2: addr=0, size=0x2000, pri=1
> 
> Then assuming submr_print_queue only contains these two mrs.  Then when
> submr_print_queue has mr1 at head, then when we insert mr2 we'll think it
> should be inserted before mr1 (because mr2's size bigger), so the result will 
> be:
> 
>   mr2:...
>   mr1:...
> 
> If submr_print_queue has mr2 at head, then when we insert mr1 we'll think it
> should be inserted before mr2 (because mr1's priority higher).  We'll instead
> get:
> 
>   mr1:...
>   mr2:...
> 
> Phil, could I ask what's the case to be fixed?

What I want is sort regions of same priority by bigger size first,
the smaller size last (as a leaf of the tree, the leaf is the MR
that handles the memory access).

Maybe this patch is not complete. I'll follow Peter Maydell suggestion
to split the compare() function out to make it more readable.
This qtailq is only used for the monitor 'mtree' command, right?
I understand the flatview uses something else.

Regards,

Phil.