This patch series is to correct the pointer usages for the snapshot
mode.
Patch 01 is to polish code, it removes the redundant header maintained
in tmc-etr driver and directly uses pointer perf_output_handle::head.
Patch 02 removes the callback cs_etm_find_snapshot() which wrongly
calculates the buffer headers; we can simply use the perf's common
function __auxtrace_mmap__read() for headers calculation. Patch 03 is
to update comments in CoreSight drivers to reflect the changes
introduced by patch 02.
This patch can be cleanly applied on the mainline kernel with:
commit dbe69e433722 ("Merge tag 'net-next-5.14' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")
And it has been tested on Arm64 Juno board.
Changes from v1:
- Dropped the patch "coresight: etm-perf: Correct buffer syncing for
snapshot", after a long discussion, the patch doesn't really resolve
any issues for snapshot mode. And another reason for unlike this
patch is now the CoreSight and Intel-PT have the consistent behaviour
(Suzuki/James/Mathieu);
- Added the patch 03 to updates drivers' comments (James);
- Added Suzuki's review tag for patch 01;
- Added James' review and testing tags for patch 02.
Leo Yan (3):
coresight: tmc-etr: Use perf_output_handle::head for AUX ring buffer
perf cs-etm: Remove callback cs_etm_find_snapshot()
coresight: Update comments for removing cs_etm_find_snapshot()
drivers/hwtracing/coresight/coresight-etb10.c | 2 +-
.../hwtracing/coresight/coresight-tmc-etf.c | 2 +-
.../hwtracing/coresight/coresight-tmc-etr.c | 12 +-
tools/perf/arch/arm/util/cs-etm.c | 133 ------------------
4 files changed, 6 insertions(+), 143 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
When enable the Arm CoreSight PMU event, the context for AUX ring buffer
is prepared in the structure perf_output_handle, and its field "head"
points the head of the AUX ring buffer and it is updated after filling
AUX trace data into buffer.
Current code uses an extra field etr_perf_buffer::head to maintain the
header for the AUX ring buffer which is not necessary; alternatively,
it's better to directly perf_output_handle::head.
This patch removes the field etr_perf_buffer::head and directly used
perf_output_handle::head for the head of AUX ring buffer.
Signed-off-by: Leo Yan <redacted>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
@@ -1553,7 +1551,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,/* Insert barrier packets at the beginning, if there was an overflow */if(lost)tmc_etr_buf_insert_barrier_packet(etr_buf,offset);-tmc_etr_sync_perf_buffer(etr_perf,offset,size);+tmc_etr_sync_perf_buffer(etr_perf,handle->head,offset,size);/**Insnapshotmodewesimplyincrementtheheadbythenumberofbyte
The callback cs_etm_find_snapshot() is invoked for snapshot mode, its
main purpose is to find the correct AUX trace data and returns "head"
and "old" (we can call "old" as "old head") to the caller, the caller
__auxtrace_mmap__read() uses these two pointers to decide the AUX trace
data size.
This patch removes cs_etm_find_snapshot() with below reasons:
- The first thing in cs_etm_find_snapshot() is to check if the head has
wrapped around, if it is not, directly bails out. The checking is
pointless, this is because the "head" and "old" pointers both are
monotonical increasing so they never wrap around.
- cs_etm_find_snapshot() adjusts the "head" and "old" pointers and
assumes the AUX ring buffer is fully filled with the hardware trace
data, so it always subtracts the difference "mm->len" from "head" to
get "old". Let's imagine the snapshot is taken in very short
interval, the tracers only fill a small chunk of the trace data into
the AUX ring buffer, in this case, it's wrongly to copy the whole the
AUX ring buffer to perf file.
- As the "head" and "old" pointers are monotonically increased, the
function __auxtrace_mmap__read() handles these two pointers properly.
It calculates the reminders for these two pointers, and the size is
clamped to be never more than "snapshot_size". We can simply reply on
the function __auxtrace_mmap__read() to calculate the correct result
for data copying, it's not necessary to add Arm CoreSight specific
callback.
Signed-off-by: Leo Yan <redacted>
Reviewed-by: James Clark <redacted>
Tested-by: James Clark <redacted>
---
tools/perf/arch/arm/util/cs-etm.c | 133 ------------------------------
1 file changed, 133 deletions(-)
@@ -734,135 +732,6 @@ static int cs_etm_info_fill(struct auxtrace_record *itr,return0;}-staticintcs_etm_alloc_wrapped_array(structcs_etm_recording*ptr,intidx)-{-bool*wrapped;-intcnt=ptr->wrapped_cnt;--/* Make @ptr->wrapped as big as @idx */-while(cnt<=idx)-cnt++;--/*-*Free'edincs_etm_recording_free().Usingrealloc()toavoid-*crosscompilationproblemswherethehost'ssystemsupports-*reallocarray()butnotthetarget.-*/-wrapped=realloc(ptr->wrapped,cnt*sizeof(bool));-if(!wrapped)-return-ENOMEM;--wrapped[cnt-1]=false;-ptr->wrapped_cnt=cnt;-ptr->wrapped=wrapped;--return0;-}--staticboolcs_etm_buffer_has_wrapped(unsignedchar*buffer,-size_tbuffer_size,u64head)-{-u64i,watermark;-u64*buf=(u64*)buffer;-size_tbuf_size=buffer_size;--/*-*Wewanttolooktheverylast512byte(chosenarbitrarily)in-*theringbuffer.-*/-watermark=buf_size-512;--/*-*@headiscontinuouslyincreasing-ifitsvalueisequalorgreater-*thanthesizeoftheringbuffer,ithaswrappedaround.-*/-if(head>=buffer_size)-returntrue;--/*-*Thevalueof@headissomewherewithinthesizeoftheringbuffer.-*Thiscanbethattherehasn'tbeenenoughdatatofillthering-*bufferyetorthetracetimewassolongthat@headhasnumerically-*wrappedaround.Tofindweneedtocheckifwehavedataatthevery-*endoftheringbuffer.Wecanreliablydothisbecausemmap'ed-*pagesarezeroedoutandthereisafreshmappingwitheverynew-*session.-*/--/* @head is less than 512 byte from the end of the ring buffer */-if(head>watermark)-watermark=head;--/*-*Speedthingsupbyusing64bittransactions(see"u64 *buf"above)-*/-watermark>>=3;-buf_size>>=3;--/*-*Ifwefindtracedataattheendoftheringbuffer,@headhas-*beenthereandhasnumericallywrappedaroundatleastonce.-*/-for(i=watermark;i<buf_size;i++)-if(buf[i])-returntrue;--returnfalse;-}--staticintcs_etm_find_snapshot(structauxtrace_record*itr,-intidx,structauxtrace_mmap*mm,-unsignedchar*data,-u64*head,u64*old)-{-interr;-boolwrapped;-structcs_etm_recording*ptr=-container_of(itr,structcs_etm_recording,itr);--/*-*Allocatememorytokeeptrackofwrappingifthisisthefirst-*timewedealwiththis*mm.-*/-if(idx>=ptr->wrapped_cnt){-err=cs_etm_alloc_wrapped_array(ptr,idx);-if(err)-returnerr;-}--/*-*Checktoseeif*headhaswrappedaround.Ifithasn'tonlythe-*amountofdatabetween*headand*oldissnapshot'edtoavoid-*bloatingtheperf.datafilewithzeros.Butassoonas*headhas-*wrappedaroundtheentiresizeoftheAUXringbufferittaken.-*/-wrapped=ptr->wrapped[idx];-if(!wrapped&&cs_etm_buffer_has_wrapped(data,mm->len,*head)){-wrapped=true;-ptr->wrapped[idx]=true;-}--pr_debug3("%s: mmap index %d old head %zu new head %zu size %zu\n",-__func__,idx,(size_t)*old,(size_t)*head,mm->len);--/* No wrap has occurred, we can just use *head and *old. */-if(!wrapped)-return0;--/*-**headhaswrappedaround-adjust*headand*oldtopickupthe-*entirecontentoftheAUXbuffer.-*/-if(*head>=mm->len){-*old=*head-mm->len;-}else{-*head+=mm->len;-*old=*head-mm->len;-}--return0;-}-staticintcs_etm_snapshot_start(structauxtrace_record*itr){structcs_etm_recording*ptr=
Since the function cs_etm_find_snapshot() has been removed in the user
space, it directly uses the perf common function __auxtrace_mmap__read()
to calcualte the head and size for AUX trace data in snapshot mode.
Updates the comments in drivers to reflect the changes.
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etb10.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc-etf.c | 2 +-
drivers/hwtracing/coresight/coresight-tmc-etr.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
From: Arnaldo Carvalho de Melo <acme@kernel.org> Date: 2021-07-01 16:25:20
Em Thu, Jul 01, 2021 at 05:35:36PM +0800, Leo Yan escreveu:
The callback cs_etm_find_snapshot() is invoked for snapshot mode, its
main purpose is to find the correct AUX trace data and returns "head"
and "old" (we can call "old" as "old head") to the caller, the caller
__auxtrace_mmap__read() uses these two pointers to decide the AUX trace
data size.
This patch removes cs_etm_find_snapshot() with below reasons:
- The first thing in cs_etm_find_snapshot() is to check if the head has
wrapped around, if it is not, directly bails out. The checking is
pointless, this is because the "head" and "old" pointers both are
monotonical increasing so they never wrap around.
- cs_etm_find_snapshot() adjusts the "head" and "old" pointers and
assumes the AUX ring buffer is fully filled with the hardware trace
data, so it always subtracts the difference "mm->len" from "head" to
get "old". Let's imagine the snapshot is taken in very short
interval, the tracers only fill a small chunk of the trace data into
the AUX ring buffer, in this case, it's wrongly to copy the whole the
AUX ring buffer to perf file.
- As the "head" and "old" pointers are monotonically increased, the
function __auxtrace_mmap__read() handles these two pointers properly.
It calculates the reminders for these two pointers, and the size is
clamped to be never more than "snapshot_size". We can simply reply on
the function __auxtrace_mmap__read() to calculate the correct result
for data copying, it's not necessary to add Arm CoreSight specific
callback.
On Thu, Jul 01, 2021 at 01:25:14PM -0300, Arnaldo Carvalho de Melo wrote:
Em Thu, Jul 01, 2021 at 05:35:36PM +0800, Leo Yan escreveu:
quoted
The callback cs_etm_find_snapshot() is invoked for snapshot mode, its
main purpose is to find the correct AUX trace data and returns "head"
and "old" (we can call "old" as "old head") to the caller, the caller
__auxtrace_mmap__read() uses these two pointers to decide the AUX trace
data size.
This patch removes cs_etm_find_snapshot() with below reasons:
- The first thing in cs_etm_find_snapshot() is to check if the head has
wrapped around, if it is not, directly bails out. The checking is
pointless, this is because the "head" and "old" pointers both are
monotonical increasing so they never wrap around.
- cs_etm_find_snapshot() adjusts the "head" and "old" pointers and
assumes the AUX ring buffer is fully filled with the hardware trace
data, so it always subtracts the difference "mm->len" from "head" to
get "old". Let's imagine the snapshot is taken in very short
interval, the tracers only fill a small chunk of the trace data into
the AUX ring buffer, in this case, it's wrongly to copy the whole the
AUX ring buffer to perf file.
- As the "head" and "old" pointers are monotonically increased, the
function __auxtrace_mmap__read() handles these two pointers properly.
It calculates the reminders for these two pointers, and the size is
clamped to be never more than "snapshot_size". We can simply reply on
the function __auxtrace_mmap__read() to calculate the correct result
for data copying, it's not necessary to add Arm CoreSight specific
callback.
Thanks, applied.
Thanks a lot for picking up the patch, Arnaldo!
Hi Mathieu, I supposed to get your review before merging; since
Arnaldo moves quickly, if you want me to follow up anything relevant
to this change, please let me know. Thanks!
Leo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, 1 Jul 2021 at 19:10, Leo Yan [off-list ref] wrote:
On Thu, Jul 01, 2021 at 01:25:14PM -0300, Arnaldo Carvalho de Melo wrote:
quoted
Em Thu, Jul 01, 2021 at 05:35:36PM +0800, Leo Yan escreveu:
quoted
The callback cs_etm_find_snapshot() is invoked for snapshot mode, its
main purpose is to find the correct AUX trace data and returns "head"
and "old" (we can call "old" as "old head") to the caller, the caller
__auxtrace_mmap__read() uses these two pointers to decide the AUX trace
data size.
This patch removes cs_etm_find_snapshot() with below reasons:
- The first thing in cs_etm_find_snapshot() is to check if the head has
wrapped around, if it is not, directly bails out. The checking is
pointless, this is because the "head" and "old" pointers both are
monotonical increasing so they never wrap around.
- cs_etm_find_snapshot() adjusts the "head" and "old" pointers and
assumes the AUX ring buffer is fully filled with the hardware trace
data, so it always subtracts the difference "mm->len" from "head" to
get "old". Let's imagine the snapshot is taken in very short
interval, the tracers only fill a small chunk of the trace data into
the AUX ring buffer, in this case, it's wrongly to copy the whole the
AUX ring buffer to perf file.
- As the "head" and "old" pointers are monotonically increased, the
function __auxtrace_mmap__read() handles these two pointers properly.
It calculates the reminders for these two pointers, and the size is
clamped to be never more than "snapshot_size". We can simply reply on
the function __auxtrace_mmap__read() to calculate the correct result
for data copying, it's not necessary to add Arm CoreSight specific
callback.
Thanks, applied.
Thanks a lot for picking up the patch, Arnaldo!
Hi Mathieu, I supposed to get your review before merging; since
Arnaldo moves quickly, if you want me to follow up anything relevant
to this change, please let me know. Thanks!
I was going to review this set next week. But it is fine if Arnaldo
has picked it up since enough people have already looked at and tested
this code.
Thanks,
Mathieu