diff mbox series

[v1,4/5] linux-user: log page table changes under -d page

Message ID 20191128194603.24818-5-alex.bennee@linaro.org
State New
Headers show
Series linux-user mmap debug cleanup | expand

Commit Message

Alex Bennée Nov. 28, 2019, 7:46 p.m. UTC 
The CPU_LOG_PAGE flag is woefully underused and could stand to do
extra duty tracking page changes. If the user doesn't want to see the
details as things change they still have the tracepoints available.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 linux-user/mmap.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Dec. 2, 2019, 1:26 a.m. UTC | #1 
On 11/28/19 7:46 PM, Alex Bennée wrote:
> +    if (qemu_loglevel_mask(CPU_LOG_PAGE)) {

> +        qemu_log_lock();

> +        qemu_log("new page @ 0x"TARGET_ABI_FMT_lx" updates page map:\n", start);

> +        log_page_dump();

> +        qemu_log_unlock();

> +    }


Hmm.  The language used here asserts a change, when we don't actually know that
for a fact.  It *probably* adds a new page, but it could be overwriting an old
page.  Can you find a better wording here?


r~
Richard Henderson Dec. 2, 2019, 6:20 p.m. UTC | #2 
On 12/1/19 1:26 AM, Richard Henderson wrote:
> On 11/28/19 7:46 PM, Alex Bennée wrote:

>> +    if (qemu_loglevel_mask(CPU_LOG_PAGE)) {

>> +        qemu_log_lock();

>> +        qemu_log("new page @ 0x"TARGET_ABI_FMT_lx" updates page map:\n", start);

>> +        log_page_dump();

>> +        qemu_log_unlock();

>> +    }

> 

> Hmm.  The language used here asserts a change, when we don't actually know that

> for a fact.  It *probably* adds a new page, but it could be overwriting an old

> page.  Can you find a better wording here?


Also, if you're going to do this, you might as well instrument munmap and
mremap as well.  Otherwise we're missing transitions.


r~
diff mbox series

Patch

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index a2c7037f1b6..c2755fcba1f 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -18,6 +18,7 @@ 
  */
 #include "qemu/osdep.h"
 #include "trace.h"
+#include "exec/log.h"
 #include "qemu.h"
 
 //#define DEBUG_MMAP
@@ -578,10 +579,12 @@  abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
     trace_target_mmap_complete(start);
-#ifdef DEBUG_MMAP
-    page_dump(stdout);
-    printf("\n");
-#endif
+    if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
+        qemu_log_lock();
+        qemu_log("new page @ 0x"TARGET_ABI_FMT_lx" updates page map:\n", start);
+        log_page_dump();
+        qemu_log_unlock();
+    }
     tb_invalidate_phys_range(start, start + len);
     mmap_unlock();
     return start;