From: Yicong Yang <[email protected]>
This series contains several updates for PTT driver:
- Disable interrupt when trace stops, reverse to what we do in trace start
- Always handle the interrupt in hardirq context
- Optimize the AUX buffer handling to make consumer have more time to process
the data
- Since we're a uncore PMU so block any task attach operation
- Add a dummy pmu::read() callback since the perf core may use
Change since v2:
- Add fix tag for Patch 5/5
- refine the commit in Patch 3/5, trying to make it more clear
Link: https://lore.kernel.org/all/[email protected]/
Change since v1:
- Add Jonathan's tag, thanks
Link: https://lore.kernel.org/all/[email protected]/
Junhao He (1):
hwtracing: hisi_ptt: Add dummy callback pmu::read()
Yicong Yang (4):
hwtracing: hisi_ptt: Disable interrupt after trace end
hwtracing: hisi_ptt: Handle the interrupt in hardirq context
hwtracing: hisi_ptt: Optimize the trace data committing
hwtracing: hisi_ptt: Don't try to attach a task
drivers/hwtracing/ptt/hisi_ptt.c | 33 +++++++++++++++++++++-----------
drivers/hwtracing/ptt/hisi_ptt.h | 1 +
2 files changed, 23 insertions(+), 11 deletions(-)
--
2.24.0
From: Yicong Yang <[email protected]>
Handle the trace interrupt in the hardirq context, make sure the irq
core won't threaded it by declaring IRQF_NO_THREAD and userspace won't
balance it by declaring IRQF_NOBALANCING. Otherwise we may violate the
synchronization requirements of the perf core, referenced to the
change of arm-ccn PMU commit 0811ef7e2f54 ("bus: arm-ccn: fix PMU interrupt flags").
In the interrupt handler we mainly doing 2 things:
- Copy the data from the local DMA buffer to the AUX buffer
- Commit the data in the AUX buffer
Signed-off-by: Yicong Yang <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/hwtracing/ptt/hisi_ptt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 428cca54217e..3041238a6e54 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -346,9 +346,9 @@ static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt)
return ret;
hisi_ptt->trace_irq = pci_irq_vector(pdev, HISI_PTT_TRACE_DMA_IRQ);
- ret = devm_request_threaded_irq(&pdev->dev, hisi_ptt->trace_irq,
- NULL, hisi_ptt_isr, 0,
- DRV_NAME, hisi_ptt);
+ ret = devm_request_irq(&pdev->dev, hisi_ptt->trace_irq, hisi_ptt_isr,
+ IRQF_NOBALANCING | IRQF_NO_THREAD, DRV_NAME,
+ hisi_ptt);
if (ret) {
pci_err(pdev, "failed to request irq %d, ret = %d\n",
hisi_ptt->trace_irq, ret);
--
2.24.0
From: Yicong Yang <[email protected]>
PTT is an uncore PMU and shouldn't be attached to any task. Block
the usage in pmu::event_init().
Signed-off-by: Yicong Yang <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/hwtracing/ptt/hisi_ptt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 4f355df8da23..62a444f5228e 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -1003,6 +1003,9 @@ static int hisi_ptt_pmu_event_init(struct perf_event *event)
return -EOPNOTSUPP;
}
+ if (event->attach_state & PERF_ATTACH_TASK)
+ return -EOPNOTSUPP;
+
if (event->attr.type != hisi_ptt->hisi_ptt_pmu.type)
return -ENOENT;
--
2.24.0
From: Junhao He <[email protected]>
When start trace with perf option "-C $cpu" and immediately stop it
with SIGTERM or others, the perf core will invoke pmu::read() while
the driver doesn't implement it. Add a dummy pmu::read() to avoid
any issues.
Fixes: ff0de066b463 ("hwtracing: hisi_ptt: Add trace function support for HiSilicon PCIe Tune and Trace device")
Signed-off-by: Junhao He <[email protected]>
Signed-off-by: Yicong Yang <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/hwtracing/ptt/hisi_ptt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 62a444f5228e..c1b5fd2b8974 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -1184,6 +1184,10 @@ static void hisi_ptt_pmu_del(struct perf_event *event, int flags)
hisi_ptt_pmu_stop(event, PERF_EF_UPDATE);
}
+static void hisi_ptt_pmu_read(struct perf_event *event)
+{
+}
+
static void hisi_ptt_remove_cpuhp_instance(void *hotplug_node)
{
cpuhp_state_remove_instance_nocalls(hisi_ptt_pmu_online, hotplug_node);
@@ -1227,6 +1231,7 @@ static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt)
.stop = hisi_ptt_pmu_stop,
.add = hisi_ptt_pmu_add,
.del = hisi_ptt_pmu_del,
+ .read = hisi_ptt_pmu_read,
};
reg = readl(hisi_ptt->iobase + HISI_PTT_LOCATION);
--
2.24.0
From: Yicong Yang <[email protected]>
In the current implementation, there're 4*4MiB trace buffer and hardware
will fill the buffer one by one. The driver will get notified if one
buffer is full and then copy data to the AUX buffer. If there's no
enough room for the next trace buffer, we'll commit the AUX buffer to
the perf core and try to apply a new one. In a typical configuration
the AUX buffer will be 16MiB, so we'll commit the data after the whole
AUX buffer is occupied. Then the driver cannot apply a new AUX buffer
immediately until the committed data is consumed by userspace and then
there's room in the AUX buffer again.
This patch tries to optimize this by commit the data after one single
trace buffer is filled. Since there's still room in the AUX buffer,
driver can apply a new one without failure and don't need to wait for
the userspace to consume the data.
Signed-off-by: Yicong Yang <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/hwtracing/ptt/hisi_ptt.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 3041238a6e54..4f355df8da23 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -274,15 +274,14 @@ static int hisi_ptt_update_aux(struct hisi_ptt *hisi_ptt, int index, bool stop)
buf->pos += size;
/*
- * Just commit the traced data if we're going to stop. Otherwise if the
- * resident AUX buffer cannot contain the data of next trace buffer,
- * apply a new one.
+ * Always commit the data to the AUX buffer in time to make sure
+ * userspace got enough time to consume the data.
+ *
+ * If we're not going to stop, apply a new one and check whether
+ * there's enough room for the next trace.
*/
- if (stop) {
- perf_aux_output_end(handle, buf->pos);
- } else if (buf->length - buf->pos < HISI_PTT_TRACE_BUF_SIZE) {
- perf_aux_output_end(handle, buf->pos);
-
+ perf_aux_output_end(handle, size);
+ if (!stop) {
buf = perf_aux_output_begin(handle, event);
if (!buf)
return -EINVAL;
--
2.24.0
Hi Suzuki,
a gentle ping..
Hope all previous comments are addressed/clarified. Any further comment on this?
Thanks.
On 2023/10/10 16:47, Yicong Yang wrote:
> From: Yicong Yang <[email protected]>
>
> This series contains several updates for PTT driver:
> - Disable interrupt when trace stops, reverse to what we do in trace start
> - Always handle the interrupt in hardirq context
> - Optimize the AUX buffer handling to make consumer have more time to process
> the data
> - Since we're a uncore PMU so block any task attach operation
> - Add a dummy pmu::read() callback since the perf core may use
>
> Change since v2:
> - Add fix tag for Patch 5/5
> - refine the commit in Patch 3/5, trying to make it more clear
> Link: https://lore.kernel.org/all/[email protected]/
>
> Change since v1:
> - Add Jonathan's tag, thanks
> Link: https://lore.kernel.org/all/[email protected]/
>
> Junhao He (1):
> hwtracing: hisi_ptt: Add dummy callback pmu::read()
>
> Yicong Yang (4):
> hwtracing: hisi_ptt: Disable interrupt after trace end
> hwtracing: hisi_ptt: Handle the interrupt in hardirq context
> hwtracing: hisi_ptt: Optimize the trace data committing
> hwtracing: hisi_ptt: Don't try to attach a task
>
> drivers/hwtracing/ptt/hisi_ptt.c | 33 +++++++++++++++++++++-----------
> drivers/hwtracing/ptt/hisi_ptt.h | 1 +
> 2 files changed, 23 insertions(+), 11 deletions(-)
>
On Tue, 10 Oct 2023 16:47:26 +0800, Yicong Yang wrote:
> From: Yicong Yang <[email protected]>
>
> This series contains several updates for PTT driver:
> - Disable interrupt when trace stops, reverse to what we do in trace start
> - Always handle the interrupt in hardirq context
I wrapped the commit description to 75 chars for this patch to suppress the
following checkpatch warning and queued it.
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#10:
change of arm-ccn PMU commit 0811ef7e2f54 ("bus: arm-ccn: fix PMU interrupt flags").
total: 0 errors, 1 warnings, 12 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] hwtracing: hisi_ptt: Handle the interrupt in hardirq context" has style problems, please review.
> - Optimize the AUX buffer handling to make consumer have more time to process
> the data
> - Since we're a uncore PMU so block any task attach operation
> - Add a dummy pmu::read() callback since the perf core may use
>
> [...]
Applied, thanks!
[1/5] hwtracing: hisi_ptt: Disable interrupt after trace end
https://git.kernel.org/coresight/c/4669551e797a
[2/5] hwtracing: hisi_ptt: Handle the interrupt in hardirq context
https://git.kernel.org/coresight/c/e8b7d8718c51
[3/5] hwtracing: hisi_ptt: Optimize the trace data committing
https://git.kernel.org/coresight/c/7a527d4d9273
[4/5] hwtracing: hisi_ptt: Don't try to attach a task
https://git.kernel.org/coresight/c/7d52e2cfef91
[5/5] hwtracing: hisi_ptt: Add dummy callback pmu::read()
https://git.kernel.org/coresight/c/4708eada8bd6
Best regards,
--
Suzuki K Poulose <[email protected]>
On 2023/10/18 1:01, Suzuki K Poulose wrote:
> On Tue, 10 Oct 2023 16:47:26 +0800, Yicong Yang wrote:
>> From: Yicong Yang <[email protected]>
>>
>> This series contains several updates for PTT driver:
>> - Disable interrupt when trace stops, reverse to what we do in trace start
>> - Always handle the interrupt in hardirq context
>
> I wrapped the commit description to 75 chars for this patch to suppress the
> following checkpatch warning and queued it.
Thanks! Will take care next time.
>
> WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
> #10:
> change of arm-ccn PMU commit 0811ef7e2f54 ("bus: arm-ccn: fix PMU interrupt flags").
>
> total: 0 errors, 1 warnings, 12 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] hwtracing: hisi_ptt: Handle the interrupt in hardirq context" has style problems, please review.
>
>
>> - Optimize the AUX buffer handling to make consumer have more time to process
>> the data
>> - Since we're a uncore PMU so block any task attach operation
>> - Add a dummy pmu::read() callback since the perf core may use
>>
>> [...]
>
> Applied, thanks!
>
> [1/5] hwtracing: hisi_ptt: Disable interrupt after trace end
> https://git.kernel.org/coresight/c/4669551e797a
> [2/5] hwtracing: hisi_ptt: Handle the interrupt in hardirq context
> https://git.kernel.org/coresight/c/e8b7d8718c51
> [3/5] hwtracing: hisi_ptt: Optimize the trace data committing
> https://git.kernel.org/coresight/c/7a527d4d9273
> [4/5] hwtracing: hisi_ptt: Don't try to attach a task
> https://git.kernel.org/coresight/c/7d52e2cfef91
> [5/5] hwtracing: hisi_ptt: Add dummy callback pmu::read()
> https://git.kernel.org/coresight/c/4708eada8bd6
>
> Best regards,
>