perf/x86/intel/pt: Fix sampling synchronization

pt_event_snapshot_aux() uses pt->handle_nmi to determine if tracing
needs to be stopped, however tracing can still be going because
pt->handle_nmi is set to zero before tracing is stopped in pt_event_stop,
whereas pt_event_snapshot_aux() requires that tracing must be stopped in
order to copy a sample of trace from the buffer.

Instead call pt_config_stop() always, which anyway checks config for
RTIT_CTL_TRACEEN and does nothing if it is already clear.

Note pt_event_snapshot_aux() can continue to use pt->handle_nmi to
determine if the trace needs to be restarted afterwards.

Fixes: 25e8920b301c ("perf/x86/intel/pt: Add sampling support")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20240715160712.127117-2-adrian.hunter@intel.com

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index b4aa8daa47738f39c463bc6dad1fb0c647c65a0c..2959970dd10eb1c3f26ca3c95b4cfc083fc8a214 100644 (file)
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1606,6 +1606,7 @@ static void pt_event_stop(struct perf_event *event, int mode)
         * see comment in intel_pt_interrupt().
         */
        WRITE_ONCE(pt->handle_nmi, 0);
+       barrier();
 
        pt_config_stop(event);
 
@@ -1657,11 +1658,10 @@ static long pt_event_snapshot_aux(struct perf_event *event,
                return 0;
 
        /*
-        * Here, handle_nmi tells us if the tracing is on
+        * There is no PT interrupt in this mode, so stop the trace and it will
+        * remain stopped while the buffer is copied.
         */
-       if (READ_ONCE(pt->handle_nmi))
-               pt_config_stop(event);
-
+       pt_config_stop(event);
        pt_read_offset(buf);
        pt_update_head(pt);
 
@@ -1673,11 +1673,10 @@ static long pt_event_snapshot_aux(struct perf_event *event,
        ret = perf_output_copy_aux(&pt->handle, handle, from, to);
 
        /*
-        * If the tracing was on when we turned up, restart it.
-        * Compiler barrier not needed as we couldn't have been
-        * preempted by anything that touches pt->handle_nmi.
+        * Here, handle_nmi tells us if the tracing was on.
+        * If the tracing was on, restart it.
         */
-       if (pt->handle_nmi)
+       if (READ_ONCE(pt->handle_nmi))
                pt_config_start(event);
 
        return ret;