* [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output @ 2020-07-27 17:45 Philippe Mathieu-Daudé 2020-07-27 18:09 ` Peter Xu 2020-07-27 18:36 ` Peter Maydell 0 siblings, 2 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-07-27 17:45 UTC (permalink / raw) To: qemu-devel Cc: Alexey Kardashevskiy, Paolo Bonzini, Philippe Mathieu-Daudé, Peter Xu, Igor Mammedov 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; -- 2.21.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output 2020-07-27 17:45 [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output Philippe Mathieu-Daudé @ 2020-07-27 18:09 ` Peter Xu 2020-08-05 14:21 ` Philippe Mathieu-Daudé 2020-07-27 18:36 ` Peter Maydell 1 sibling, 1 reply; 5+ messages in thread From: Peter Xu @ 2020-07-27 18:09 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Alexey Kardashevskiy, Paolo Bonzini, qemu-devel, Igor Mammedov 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? -- Peter Xu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output 2020-07-27 18:09 ` Peter Xu @ 2020-08-05 14:21 ` Philippe Mathieu-Daudé 2020-08-05 14:45 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-08-05 14:21 UTC (permalink / raw) To: Peter Xu; +Cc: Alexey Kardashevskiy, Paolo Bonzini, qemu-devel, Igor Mammedov 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output 2020-08-05 14:21 ` Philippe Mathieu-Daudé @ 2020-08-05 14:45 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-08-05 14:45 UTC (permalink / raw) To: Peter Xu; +Cc: Alexey Kardashevskiy, Paolo Bonzini, qemu-devel, Igor Mammedov On 8/5/20 4:21 PM, Philippe Mathieu-Daudé wrote: > 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). As example as easier to understand, this is the change I expect: memory-region: io 0000000000000000-000000000000ffff (prio 0, i/o): io + 0000000000000000-0000000000000007 (prio 0, i/o): dma-chan 0000000000000000-0000000000000003 (prio 0, i/o): acpi-evt 0000000000000004-0000000000000005 (prio 0, i/o): acpi-cnt 0000000000000008-000000000000000b (prio 0, i/o): acpi-tmr - 0000000000000000-0000000000000007 (prio 0, i/o): dma-chan 0000000000000008-000000000000000f (prio 0, i/o): dma-cont 0000000000000020-0000000000000021 (prio 0, i/o): pic 0000000000000040-0000000000000043 (prio 0, i/o): pit For me it doesn't seem natural to look after 0x8 if there is another region mapped at 0x0. Now it is easier to see 'dma-chan' is masking the acpi events. Note I'd expect 'acpi-tmr' to be after 'dma-cont' to also realize it is masked. FYI the resulting flatview: (qemu) info mtree -f FlatView #2 AS "I/O", root: io Root memory region: io 0000000000000000-0000000000000007 (prio 0, i/o): dma-chan 0000000000000008-000000000000000f (prio 0, i/o): dma-cont > > 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. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output 2020-07-27 17:45 [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output Philippe Mathieu-Daudé 2020-07-27 18:09 ` Peter Xu @ 2020-07-27 18:36 ` Peter Maydell 1 sibling, 0 replies; 5+ messages in thread From: Peter Maydell @ 2020-07-27 18:36 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Alexey Kardashevskiy, Paolo Bonzini, QEMU Developers, Peter Xu, Igor Mammedov On Mon, 27 Jul 2020 at 18:46, Philippe Mathieu-Daudé <f4bug@amsat.org> 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; I think this is the point where you want to factor out the comparison-of-two-MRs function. Which then makes it easier to implement the required logic, which as Peter Xu says should be "address compare first; if those are equal look at the priority; if those are also equal look at the size", ie the usual comparison-with-multiple-fields. thanks -- PMM ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-05 14:47 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-27 17:45 [PATCH-for-5.2] memory: Display bigger regions first in 'info mtree' output Philippe Mathieu-Daudé 2020-07-27 18:09 ` Peter Xu 2020-08-05 14:21 ` Philippe Mathieu-Daudé 2020-08-05 14:45 ` Philippe Mathieu-Daudé 2020-07-27 18:36 ` Peter Maydell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.