Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Tue, 2 Aug 2022 13:03:42 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Yi Sun <yi.sun@intel.com>
Cc:     dave.hansen@intel.com, sohil.mehta@intel.com, tony.luck@intel.com,
        linux-kernel@vger.kernel.org, x86@kernel.org,
        david.laight@aculab.com, heng.su@intel.com
Subject: Re: [PATCH v2 2/2] scripts/x86: Add script to consume trace log of
 xsave latency
Message-ID: <YukEjqcU9UK8IVgf@gmail.com>
References: <20220729095608.1475138-1-yi.sun@intel.com>
 <20220729095608.1475138-3-yi.sun@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220729095608.1475138-3-yi.sun@intel.com>
Precedence: bulk


* Yi Sun <yi.sun@intel.com> wrote:

> Consume the trace log dumped by trace points x86_fpu_latency_xsave and
> x86_fpu_latency_xrstor, calculate latency ranges for each RFBM and
> XINUSE combination including min, max, average and 97% tail latency.
> 
> Add the average of 97% tail latency to remove the unreasonable
> data which is introduced by interrupts or other noise. By adding the
> experimental code disabling interrupts before the calculation of
> latency, it's obvious to get the 3% tail latency has been filtered.
> 
> Make use of sqlite3 to make the data statistics more efficient and
> concise. The output looks like following:
> 
> EVENTs                	RFBM   	XINUSE	lat_min	lat_max	lat_avg	lat_avg(97%)
> ----------------------	-------	------	-------	-------	-------	------------
> x86_fpu_latency_xrstor	0x206e7	0x0   	364    	364    	364    	364
> x86_fpu_latency_xrstor	0x206e7	0x202 	112    	1152   	300    	276
> x86_fpu_latency_xsave 	0x206e7	0x202 	80     	278    	141    	137
> x86_fpu_latency_xsave 	0x206e7	0x246 	108    	234    	180    	177
> 
> The XSAVE/XRSTOR latency trace log can be got by two ways:
> 1. Generated by Kernel debugfs
>   echo 1 > /sys/kernel/debug/tracing/events/x86_fpu/enable
>   cat /sys/kernel/debug/tracing/trace_pipe > trace-log
> 
> 2. Generated by helper tool like 'trace-cmd'
>    trace-cmd record -e x86_fpu -F <command>
>    trace-cmd report > trace-log
> 
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Yi Sun <yi.sun@intel.com>
> 
> diff --git a/scripts/x86/xsave-latency-trace.sh b/scripts/x86/xsave-latency-trace.sh
> new file mode 100755
> index 000000000000..d45563984fd6
> --- /dev/null
> +++ b/scripts/x86/xsave-latency-trace.sh

The patchset looks mostly good to me, but I don't think we are supposed to 
add arch dependent instrumentation scripts to scripts/, please put this new 
script somewhere in tools/testing/x86/ or so.

Thanks,

	Ingo