qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH  v2 0/6] linux-user mmap debug cleanup
@ 2019-12-05 12:25 Alex Bennée
  2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée

Hi,

This is a quick re-spin of the mmap tracing. I don't bother with
pretty formatting the protection/type flags as they were incomplete
and not hard to extract from just dumping the hex values. This makes
most of the patches much simpler. I've also dropped the home-made
pattern_glob routine.

The following patches need review:
   01 - linux user convert target_mprotect debug to trace
   02 - linux user convert target_mmap debug to tracepoin
   04 - linux user log page table changes under d page
   06 - trace replace hand crafted pattern_glob with g_pa

Alex Bennée (6):
  linux-user: convert target_mprotect debug to tracepoint
  linux-user: convert target_mmap debug to tracepoint
  linux-user: add target_mmap_complete tracepoint
  linux-user: log page table changes under -d page
  linux-user: convert target_munmap debug to a tracepoint
  trace: replace hand-crafted pattern_glob with g_pattern_match_simple

 include/exec/log.h      |  5 +++-
 bsd-user/main.c         |  2 +-
 linux-user/main.c       |  2 +-
 linux-user/mmap.c       | 56 ++++++++---------------------------------
 trace/control.c         | 35 +-------------------------
 linux-user/trace-events |  6 +++++
 6 files changed, 23 insertions(+), 83 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-05 15:43   ` Richard Henderson
  2019-12-11 14:46   ` Laurent Vivier
  2019-12-05 12:25 ` [PATCH v2 2/6] linux-user: convert target_mmap " Alex Bennée
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier

It is a pain to re-compile when you need to debug and tracepoints are
a fairly low impact way to instrument QEMU.

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

---
v2
  - don't both with ascii conversion
---
 linux-user/mmap.c       | 10 ++--------
 linux-user/trace-events |  3 +++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 46a6e3a761a..26a83e74069 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -17,7 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
-
+#include "trace.h"
 #include "qemu.h"
 
 //#define DEBUG_MMAP
@@ -66,13 +66,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
     abi_ulong end, host_start, host_end, addr;
     int prot1, ret;
 
-#ifdef DEBUG_MMAP
-    printf("mprotect: start=0x" TARGET_ABI_FMT_lx
-           "len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
-           prot & PROT_READ ? 'r' : '-',
-           prot & PROT_WRITE ? 'w' : '-',
-           prot & PROT_EXEC ? 'x' : '-');
-#endif
+    trace_target_mprotect(start, len, prot);
 
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -TARGET_EINVAL;
diff --git a/linux-user/trace-events b/linux-user/trace-events
index 6df234bbb67..8419243de4e 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -11,3 +11,6 @@ user_handle_signal(void *env, int target_sig) "env=%p signal %d"
 user_host_signal(void *env, int host_sig, int target_sig) "env=%p signal %d (target %d("
 user_queue_signal(void *env, int target_sig) "env=%p signal %d"
 user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_addr) "env=%p frame psw.addr 0x%"PRIx64 " current psw.addr 0x%"PRIx64
+
+# mmap.c
+target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH  v2 2/6] linux-user: convert target_mmap debug to tracepoint
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
  2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-05 15:44   ` Richard Henderson
  2019-12-11 14:48   ` Laurent Vivier
  2019-12-05 12:25 ` [PATCH v2 3/6] linux-user: add target_mmap_complete tracepoint Alex Bennée
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier

It is a pain to re-compile when you need to debug and tracepoints are
a fairly low impact way to instrument QEMU.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/mmap.c       | 27 +--------------------------
 linux-user/trace-events |  1 +
 2 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 26a83e74069..f4f10deaeac 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -363,32 +363,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
 
     mmap_lock();
-#ifdef DEBUG_MMAP
-    {
-        printf("mmap: start=0x" TARGET_ABI_FMT_lx
-               " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
-               start, len,
-               prot & PROT_READ ? 'r' : '-',
-               prot & PROT_WRITE ? 'w' : '-',
-               prot & PROT_EXEC ? 'x' : '-');
-        if (flags & MAP_FIXED)
-            printf("MAP_FIXED ");
-        if (flags & MAP_ANONYMOUS)
-            printf("MAP_ANON ");
-        switch(flags & MAP_TYPE) {
-        case MAP_PRIVATE:
-            printf("MAP_PRIVATE ");
-            break;
-        case MAP_SHARED:
-            printf("MAP_SHARED ");
-            break;
-        default:
-            printf("[MAP_TYPE=0x%x] ", flags & MAP_TYPE);
-            break;
-        }
-        printf("fd=%d offset=" TARGET_ABI_FMT_lx "\n", fd, offset);
-    }
-#endif
+    trace_target_mmap(start, len, prot, flags, fd, offset);
 
     if (!len) {
         errno = EINVAL;
diff --git a/linux-user/trace-events b/linux-user/trace-events
index 8419243de4e..8d8d4c3c68c 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -14,3 +14,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add
 
 # mmap.c
 target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x"
+target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH  v2 3/6] linux-user: add target_mmap_complete tracepoint
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
  2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
  2019-12-05 12:25 ` [PATCH v2 2/6] linux-user: convert target_mmap " Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-11 14:48   ` Laurent Vivier
  2019-12-05 12:25 ` [PATCH v2 4/6] linux-user: log page table changes under -d page Alex Bennée
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Riku Voipio, Alex Bennée, Laurent Vivier

For full details we also want to see where the mmaps end up.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/mmap.c       | 2 +-
 linux-user/trace-events | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index f4f10deaeac..0b1b43ac3c0 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -538,8 +538,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
  the_end1:
     page_set_flags(start, start + len, prot | PAGE_VALID);
  the_end:
+    trace_target_mmap_complete(start);
 #ifdef DEBUG_MMAP
-    printf("ret=0x" TARGET_ABI_FMT_lx "\n", start);
     page_dump(stdout);
     printf("\n");
 #endif
diff --git a/linux-user/trace-events b/linux-user/trace-events
index 8d8d4c3c68c..6d6aeef7b52 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -15,3 +15,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add
 # mmap.c
 target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x"
 target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64
+target_mmap_complete(uint64_t retaddr) "retaddr=0x%"PRIx64
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH  v2 4/6] linux-user: log page table changes under -d page
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
                   ` (2 preceding siblings ...)
  2019-12-05 12:25 ` [PATCH v2 3/6] linux-user: add target_mmap_complete tracepoint Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-05 15:45   ` Richard Henderson
  2019-12-11 14:53   ` Laurent Vivier
  2019-12-05 12:25 ` [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint Alex Bennée
  2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
  5 siblings, 2 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier

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.

We push the locking into log_page_dump and pass a reason for the
banner text.

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

---
  v2
    - reworded banner text
    - moved locking into helper
    - converted stray calls of page_dump
---
 include/exec/log.h | 5 ++++-
 bsd-user/main.c    | 2 +-
 linux-user/main.c  | 2 +-
 linux-user/mmap.c  | 8 ++++----
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/exec/log.h b/include/exec/log.h
index e2cfd436e61..012af09f9b3 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -50,9 +50,12 @@ static inline void log_disas(void *code, unsigned long size)
 
 #if defined(CONFIG_USER_ONLY)
 /* page_dump() output to the log file: */
-static inline void log_page_dump(void)
+static inline void log_page_dump(const char *operation)
 {
+    qemu_log_lock();
+    qemu_log("page layout changed following %s\n", operation);
     page_dump(qemu_logfile);
+    qemu_log_unlock();
 }
 #endif
 #endif
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 470a8bf79ed..7f4e3cd6271 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -963,7 +963,7 @@ int main(int argc, char **argv)
 
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
         qemu_log("guest_base  0x%lx\n", guest_base);
-        log_page_dump();
+        log_page_dump("binary load");
 
         qemu_log("start_brk   0x" TARGET_ABI_FMT_lx "\n", info->start_brk);
         qemu_log("end_code    0x" TARGET_ABI_FMT_lx "\n", info->end_code);
diff --git a/linux-user/main.c b/linux-user/main.c
index 6ff7851e86f..8718d03ee21 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -826,7 +826,7 @@ int main(int argc, char **argv, char **envp)
 
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
         qemu_log("guest_base  0x%lx\n", guest_base);
-        log_page_dump();
+        log_page_dump("binary load");
 
         qemu_log("start_brk   0x" TARGET_ABI_FMT_lx "\n", info->start_brk);
         qemu_log("end_code    0x" TARGET_ABI_FMT_lx "\n", info->end_code);
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 0b1b43ac3c0..3d90fa459ca 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
@@ -539,10 +540,9 @@ 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)) {
+        log_page_dump(__func__);
+    }
     tb_invalidate_phys_range(start, start + len);
     mmap_unlock();
     return start;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
                   ` (3 preceding siblings ...)
  2019-12-05 12:25 ` [PATCH v2 4/6] linux-user: log page table changes under -d page Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-11 14:54   ` Laurent Vivier
  2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
  5 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Riku Voipio, Alex Bennée, Laurent Vivier

Convert the final bit of DEBUG_MMAP to a tracepoint and remove the
last remanents of the #ifdef hackery.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/mmap.c       | 9 ++-------
 linux-user/trace-events | 1 +
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 3d90fa459ca..8685f02e7e9 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -21,8 +21,6 @@
 #include "exec/log.h"
 #include "qemu.h"
 
-//#define DEBUG_MMAP
-
 static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
 static __thread int mmap_lock_count;
 
@@ -597,11 +595,8 @@ int target_munmap(abi_ulong start, abi_ulong len)
     abi_ulong end, real_start, real_end, addr;
     int prot, ret;
 
-#ifdef DEBUG_MMAP
-    printf("munmap: start=0x" TARGET_ABI_FMT_lx " len=0x"
-           TARGET_ABI_FMT_lx "\n",
-           start, len);
-#endif
+    trace_target_munmap(start, len);
+
     if (start & ~TARGET_PAGE_MASK)
         return -TARGET_EINVAL;
     len = TARGET_PAGE_ALIGN(len);
diff --git a/linux-user/trace-events b/linux-user/trace-events
index 6d6aeef7b52..f6de1b8befc 100644
--- a/linux-user/trace-events
+++ b/linux-user/trace-events
@@ -16,3 +16,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add
 target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x"
 target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64
 target_mmap_complete(uint64_t retaddr) "retaddr=0x%"PRIx64
+target_munmap(uint64_t start, uint64_t len) "start=0x%"PRIx64" len=0x%"PRIx64
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
  2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
                   ` (4 preceding siblings ...)
  2019-12-05 12:25 ` [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint Alex Bennée
@ 2019-12-05 12:25 ` Alex Bennée
  2019-12-05 15:51   ` Richard Henderson
                     ` (2 more replies)
  5 siblings, 3 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-05 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée, Stefan Hajnoczi

We already use g_pattern_match elsewhere so remove the duplication.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 trace/control.c | 35 +----------------------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/trace/control.c b/trace/control.c
index d9cafc161bb..0fb81241607 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -98,38 +98,6 @@ TraceEvent *trace_event_name(const char *name)
     return NULL;
 }
 
-static bool pattern_glob(const char *pat, const char *ev)
-{
-    while (*pat != '\0' && *ev != '\0') {
-        if (*pat == *ev) {
-            pat++;
-            ev++;
-        }
-        else if (*pat == '*') {
-            if (pattern_glob(pat, ev+1)) {
-                return true;
-            } else if (pattern_glob(pat+1, ev)) {
-                return true;
-            } else {
-                return false;
-            }
-        } else {
-            return false;
-        }
-    }
-
-    while (*pat == '*') {
-        pat++;
-    }
-
-    if (*pat == '\0' && *ev == '\0') {
-        return true;
-    } else {
-        return false;
-    }
-}
-
-
 void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 {
     iter->event = 0;
@@ -148,8 +116,7 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter)
             iter->group++;
         }
         if (!iter->pattern ||
-            pattern_glob(iter->pattern,
-                         trace_event_get_name(ev))) {
+            g_pattern_match_simple(iter->pattern, trace_event_get_name(ev))) {
             return ev;
         }
     }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint
  2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
@ 2019-12-05 15:43   ` Richard Henderson
  2019-12-11 14:46   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2019-12-05 15:43 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, Laurent Vivier

On 12/5/19 4:25 AM, Alex Bennée wrote:
> It is a pain to re-compile when you need to debug and tracepoints are
> a fairly low impact way to instrument QEMU.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - don't both with ascii conversion
> ---
>  linux-user/mmap.c       | 10 ++--------
>  linux-user/trace-events |  3 +++
>  2 files changed, 5 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/6] linux-user: convert target_mmap debug to tracepoint
  2019-12-05 12:25 ` [PATCH v2 2/6] linux-user: convert target_mmap " Alex Bennée
@ 2019-12-05 15:44   ` Richard Henderson
  2019-12-11 14:48   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2019-12-05 15:44 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, Laurent Vivier

On 12/5/19 4:25 AM, Alex Bennée wrote:
> It is a pain to re-compile when you need to debug and tracepoints are
> a fairly low impact way to instrument QEMU.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/mmap.c       | 27 +--------------------------
>  linux-user/trace-events |  1 +
>  2 files changed, 2 insertions(+), 26 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/6] linux-user: log page table changes under -d page
  2019-12-05 12:25 ` [PATCH v2 4/6] linux-user: log page table changes under -d page Alex Bennée
@ 2019-12-05 15:45   ` Richard Henderson
  2019-12-11 14:53   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2019-12-05 15:45 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, Laurent Vivier

On 12/5/19 4:25 AM, Alex Bennée wrote:
> 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.
> 
> We push the locking into log_page_dump and pass a reason for the
> banner text.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
>   v2
>     - reworded banner text
>     - moved locking into helper
>     - converted stray calls of page_dump
> ---
>  include/exec/log.h | 5 ++++-
>  bsd-user/main.c    | 2 +-
>  linux-user/main.c  | 2 +-
>  linux-user/mmap.c  | 8 ++++----
>  4 files changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
  2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
@ 2019-12-05 15:51   ` Richard Henderson
  2019-12-06 11:03   ` Stefan Hajnoczi
  2019-12-09 15:58   ` Stefan Hajnoczi
  2 siblings, 0 replies; 22+ messages in thread
From: Richard Henderson @ 2019-12-05 15:51 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Stefan Hajnoczi

On 12/5/19 4:25 AM, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/control.c | 35 +----------------------------------
>  1 file changed, 1 insertion(+), 34 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH  v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
  2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
  2019-12-05 15:51   ` Richard Henderson
@ 2019-12-06 11:03   ` Stefan Hajnoczi
  2019-12-06 11:59     ` Alex Bennée
  2019-12-09 15:58   ` Stefan Hajnoczi
  2 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2019-12-06 11:03 UTC (permalink / raw)
  To: Alex Bennée; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/control.c | 35 +----------------------------------
>  1 file changed, 1 insertion(+), 34 deletions(-)

Is g_pattern_match() a superset of pattern_glob()?  Existing patterns
should continue to work.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH  v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
  2019-12-06 11:03   ` Stefan Hajnoczi
@ 2019-12-06 11:59     ` Alex Bennée
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-06 11:59 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel


Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:
>> We already use g_pattern_match elsewhere so remove the duplication.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  trace/control.c | 35 +----------------------------------
>>  1 file changed, 1 insertion(+), 34 deletions(-)
>
> Is g_pattern_match() a superset of pattern_glob()?  Existing patterns
> should continue to work.

Yes - it supports more than pattern_glob and a bit less than the system
glob():

  The g_pattern_match* functions match a string against a pattern
  containing '*' and '?' wildcards with similar semantics as the standard
  glob() function: '*' matches an arbitrary, possibly empty, string, '?'
  matches an arbitrary character.

  Note that in contrast to glob(), the '/' character can be matched by the
  wildcards, there are no '[...]' character ranges and '*' and '?' can not
  be escaped to include them literally in a pattern.

If you give me some example existing pattern forms we can add them to
test-logging. I manually tested both single and double * patterns while
working on the rest of the series.

>
> Stefan


--
Alex Bennée


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH  v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
  2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
  2019-12-05 15:51   ` Richard Henderson
  2019-12-06 11:03   ` Stefan Hajnoczi
@ 2019-12-09 15:58   ` Stefan Hajnoczi
  2 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2019-12-09 15:58 UTC (permalink / raw)
  To: Alex Bennée; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/control.c | 35 +----------------------------------
>  1 file changed, 1 insertion(+), 34 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint
  2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
  2019-12-05 15:43   ` Richard Henderson
@ 2019-12-11 14:46   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Laurent Vivier @ 2019-12-11 14:46 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio

Le 05/12/2019 à 13:25, Alex Bennée a écrit :
> It is a pain to re-compile when you need to debug and tracepoints are
> a fairly low impact way to instrument QEMU.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - don't both with ascii conversion
> ---
>  linux-user/mmap.c       | 10 ++--------
>  linux-user/trace-events |  3 +++
>  2 files changed, 5 insertions(+), 8 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/6] linux-user: convert target_mmap debug to tracepoint
  2019-12-05 12:25 ` [PATCH v2 2/6] linux-user: convert target_mmap " Alex Bennée
  2019-12-05 15:44   ` Richard Henderson
@ 2019-12-11 14:48   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Laurent Vivier @ 2019-12-11 14:48 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio

Le 05/12/2019 à 13:25, Alex Bennée a écrit :
> It is a pain to re-compile when you need to debug and tracepoints are
> a fairly low impact way to instrument QEMU.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/mmap.c       | 27 +--------------------------
>  linux-user/trace-events |  1 +
>  2 files changed, 2 insertions(+), 26 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/6] linux-user: add target_mmap_complete tracepoint
  2019-12-05 12:25 ` [PATCH v2 3/6] linux-user: add target_mmap_complete tracepoint Alex Bennée
@ 2019-12-11 14:48   ` Laurent Vivier
  0 siblings, 0 replies; 22+ messages in thread
From: Laurent Vivier @ 2019-12-11 14:48 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, Richard Henderson

Le 05/12/2019 à 13:25, Alex Bennée a écrit :
> For full details we also want to see where the mmaps end up.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/mmap.c       | 2 +-
>  linux-user/trace-events | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/6] linux-user: log page table changes under -d page
  2019-12-05 12:25 ` [PATCH v2 4/6] linux-user: log page table changes under -d page Alex Bennée
  2019-12-05 15:45   ` Richard Henderson
@ 2019-12-11 14:53   ` Laurent Vivier
  1 sibling, 0 replies; 22+ messages in thread
From: Laurent Vivier @ 2019-12-11 14:53 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio

Le 05/12/2019 à 13:25, Alex Bennée a écrit :
> 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.
> 
> We push the locking into log_page_dump and pass a reason for the
> banner text.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
>   v2
>     - reworded banner text
>     - moved locking into helper
>     - converted stray calls of page_dump
> ---
>  include/exec/log.h | 5 ++++-
>  bsd-user/main.c    | 2 +-
>  linux-user/main.c  | 2 +-
>  linux-user/mmap.c  | 8 ++++----
>  4 files changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint
  2019-12-05 12:25 ` [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint Alex Bennée
@ 2019-12-11 14:54   ` Laurent Vivier
  2019-12-16 12:00     ` Alex Bennée
  0 siblings, 1 reply; 22+ messages in thread
From: Laurent Vivier @ 2019-12-11 14:54 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel; +Cc: Riku Voipio, Richard Henderson

Le 05/12/2019 à 13:25, Alex Bennée a écrit :
> Convert the final bit of DEBUG_MMAP to a tracepoint and remove the
> last remanents of the #ifdef hackery.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/mmap.c       | 9 ++-------
>  linux-user/trace-events | 1 +
>  2 files changed, 3 insertions(+), 7 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint
  2019-12-11 14:54   ` Laurent Vivier
@ 2019-12-16 12:00     ` Alex Bennée
  2019-12-16 12:05       ` Laurent Vivier
  0 siblings, 1 reply; 22+ messages in thread
From: Alex Bennée @ 2019-12-16 12:00 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, Richard Henderson, qemu-devel


Laurent Vivier <laurent@vivier.eu> writes:

> Le 05/12/2019 à 13:25, Alex Bennée a écrit :
>> Convert the final bit of DEBUG_MMAP to a tracepoint and remove the
>> last remanents of the #ifdef hackery.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  linux-user/mmap.c       | 9 ++-------
>>  linux-user/trace-events | 1 +
>>  2 files changed, 3 insertions(+), 7 deletions(-)
>> 
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Are you going to take this series via your tree or would you like me to
put the PR together?

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint
  2019-12-16 12:00     ` Alex Bennée
@ 2019-12-16 12:05       ` Laurent Vivier
  2019-12-18 20:03         ` Alex Bennée
  0 siblings, 1 reply; 22+ messages in thread
From: Laurent Vivier @ 2019-12-16 12:05 UTC (permalink / raw)
  To: Alex Bennée; +Cc: Riku Voipio, Richard Henderson, qemu-devel

Le 16/12/2019 à 13:00, Alex Bennée a écrit :
> 
> Laurent Vivier <laurent@vivier.eu> writes:
> 
>> Le 05/12/2019 à 13:25, Alex Bennée a écrit :
>>> Convert the final bit of DEBUG_MMAP to a tracepoint and remove the
>>> last remanents of the #ifdef hackery.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  linux-user/mmap.c       | 9 ++-------
>>>  linux-user/trace-events | 1 +
>>>  2 files changed, 3 insertions(+), 7 deletions(-)
>>>
>>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> Are you going to take this series via your tree or would you like me to
> put the PR together?
> 

As you prefer.

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint
  2019-12-16 12:05       ` Laurent Vivier
@ 2019-12-18 20:03         ` Alex Bennée
  0 siblings, 0 replies; 22+ messages in thread
From: Alex Bennée @ 2019-12-18 20:03 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, Richard Henderson, qemu-devel


Laurent Vivier <laurent@vivier.eu> writes:

> Le 16/12/2019 à 13:00, Alex Bennée a écrit :
>> 
>> Laurent Vivier <laurent@vivier.eu> writes:
>> 
>>> Le 05/12/2019 à 13:25, Alex Bennée a écrit :
>>>> Convert the final bit of DEBUG_MMAP to a tracepoint and remove the
>>>> last remanents of the #ifdef hackery.
>>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>  linux-user/mmap.c       | 9 ++-------
>>>>  linux-user/trace-events | 1 +
>>>>  2 files changed, 3 insertions(+), 7 deletions(-)
>>>>
>>>
>>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> 
>> Are you going to take this series via your tree or would you like me to
>> put the PR together?
>> 
>
> As you prefer.

Very well queued to pullreq/testing-logging-misc, thanks.

>
> Thanks,
> Laurent


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-12-18 20:05 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05 12:25 [PATCH v2 0/6] linux-user mmap debug cleanup Alex Bennée
2019-12-05 12:25 ` [PATCH v2 1/6] linux-user: convert target_mprotect debug to tracepoint Alex Bennée
2019-12-05 15:43   ` Richard Henderson
2019-12-11 14:46   ` Laurent Vivier
2019-12-05 12:25 ` [PATCH v2 2/6] linux-user: convert target_mmap " Alex Bennée
2019-12-05 15:44   ` Richard Henderson
2019-12-11 14:48   ` Laurent Vivier
2019-12-05 12:25 ` [PATCH v2 3/6] linux-user: add target_mmap_complete tracepoint Alex Bennée
2019-12-11 14:48   ` Laurent Vivier
2019-12-05 12:25 ` [PATCH v2 4/6] linux-user: log page table changes under -d page Alex Bennée
2019-12-05 15:45   ` Richard Henderson
2019-12-11 14:53   ` Laurent Vivier
2019-12-05 12:25 ` [PATCH v2 5/6] linux-user: convert target_munmap debug to a tracepoint Alex Bennée
2019-12-11 14:54   ` Laurent Vivier
2019-12-16 12:00     ` Alex Bennée
2019-12-16 12:05       ` Laurent Vivier
2019-12-18 20:03         ` Alex Bennée
2019-12-05 12:25 ` [PATCH v2 6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple Alex Bennée
2019-12-05 15:51   ` Richard Henderson
2019-12-06 11:03   ` Stefan Hajnoczi
2019-12-06 11:59     ` Alex Bennée
2019-12-09 15:58   ` Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).