2014-10-21 14:06:44

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 0/2] ath10k: minor htt tracing fix

Hi,

I've just noticed that rx_desc tracing is somewhat
wrong. This attempts to fix it and since the trace
function prototype changes I re-use some tracing
code as well.


Michal Kazior (2):
ath10k: remove tsf argument from rx_desc tracing
ath10k: re-use trace class

drivers/net/wireless/ath/ath10k/htt_rx.c | 4 +---
drivers/net/wireless/ath/ath10k/trace.h | 35 +++++---------------------------
2 files changed, 6 insertions(+), 33 deletions(-)

--
1.8.5.3



2014-10-24 13:34:46

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/2] ath10k: minor htt tracing fix

Michal Kazior <[email protected]> writes:

> I've just noticed that rx_desc tracing is somewhat
> wrong. This attempts to fix it and since the trace
> function prototype changes I re-use some tracing
> code as well.
>
>
> Michal Kazior (2):
> ath10k: remove tsf argument from rx_desc tracing
> ath10k: re-use trace class

Thanks, both patches applied.

--
Kalle Valo

2014-10-23 14:10:38

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath10k: remove tsf argument from rx_desc tracing

Michal Kazior <[email protected]> writes:

> On 21 October 2014 15:54, Michal Kazior <[email protected]> wrote:
>> Fundamtenally this was wrong. Tsf is only valid
>> in first MPDU of a PPDU. This means tsf value was
>
> s/first MPDU/last MPDU/
>
> I must've had a brain fart when writing the commit log yesterday.
>
> @Kalle: Should I re-spin or will you fix it before applying?

No need to re-spin, I edited the commit log in ath-next-test. I also
fixed a typo in "fundamentally".

commit d75153c577ddfffa9e7438e8225dfaf564f34552
Author: Michal Kazior <[email protected]>
Date: Thu Oct 23 17:04:27 2014 +0300

ath10k: remove tsf argument from rx_desc tracing

Fundamentally this was wrong. Tsf is only valid
in last MPDU of a PPDU. This means tsf value was
wrong most of the time during heavy traffic.

Also I don't see much point in exposing a
redundant (and broken) tsf value. Userspace can
already read it from the dumped rx descriptor
buffer.

Cc: Rajkumar Manoharan <[email protected]>
Signed-off-by: Michal Kazior <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>


--
Kalle Valo

2014-10-22 08:46:45

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath10k: remove tsf argument from rx_desc tracing

On 21 October 2014 15:54, Michal Kazior <[email protected]> wrote:
> Fundamtenally this was wrong. Tsf is only valid
> in first MPDU of a PPDU. This means tsf value was

s/first MPDU/last MPDU/

I must've had a brain fart when writing the commit log yesterday.

@Kalle: Should I re-spin or will you fix it before applying?


MichaƂ

2014-10-21 14:06:45

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 1/2] ath10k: remove tsf argument from rx_desc tracing

Fundamtenally this was wrong. Tsf is only valid
in first MPDU of a PPDU. This means tsf value was
wrong most of the time during heavy traffic.

Also I don't see much point in exposing a
redundant (and broken) tsf value. Userspace can
already read it from the dumped rx descriptor
buffer.

Cc: Rajkumar Manoharan <[email protected]>
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 4 +---
drivers/net/wireless/ath/ath10k/trace.h | 9 +++------
2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index fbb3175..d3741b4 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -319,7 +319,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
int msdu_len, msdu_chaining = 0;
struct sk_buff *msdu, *next;
struct htt_rx_desc *rx_desc;
- u32 tsf;

lockdep_assert_held(&htt->rx_ring.lock);

@@ -451,8 +450,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
last_msdu = __le32_to_cpu(rx_desc->msdu_end.info0) &
RX_MSDU_END_INFO0_LAST_MSDU;

- tsf = __le32_to_cpu(rx_desc->ppdu_end.tsf_timestamp);
- trace_ath10k_htt_rx_desc(ar, tsf, &rx_desc->attention,
+ trace_ath10k_htt_rx_desc(ar, &rx_desc->attention,
sizeof(*rx_desc) - sizeof(u32));
if (last_msdu) {
msdu->next = NULL;
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index 9d34e7f..2409cb5 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -282,14 +282,13 @@ TRACE_EVENT(ath10k_htt_pktlog,
);

TRACE_EVENT(ath10k_htt_rx_desc,
- TP_PROTO(struct ath10k *ar, u32 tsf, void *rxdesc, u16 len),
+ TP_PROTO(struct ath10k *ar, void *rxdesc, u16 len),

- TP_ARGS(ar, tsf, rxdesc, len),
+ TP_ARGS(ar, rxdesc, len),

TP_STRUCT__entry(
__string(device, dev_name(ar->dev))
__string(driver, dev_driver_string(ar->dev))
- __field(u32, tsf)
__field(u16, len)
__dynamic_array(u8, rxdesc, len)
),
@@ -297,16 +296,14 @@ TRACE_EVENT(ath10k_htt_rx_desc,
TP_fast_assign(
__assign_str(device, dev_name(ar->dev));
__assign_str(driver, dev_driver_string(ar->dev));
- __entry->tsf = tsf;
__entry->len = len;
memcpy(__get_dynamic_array(rxdesc), rxdesc, len);
),

TP_printk(
- "%s %s %u len %hu",
+ "%s %s len %hu",
__get_str(driver),
__get_str(device),
- __entry->tsf,
__entry->len
)
);
--
1.8.5.3


2014-10-21 14:06:47

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 2/2] ath10k: re-use trace class

Instead of defining a completely new tracepoint
use an existing tracepoint class.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/trace.h | 32 +++++---------------------------
1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index 2409cb5..b9a2ba6 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -281,33 +281,6 @@ TRACE_EVENT(ath10k_htt_pktlog,
)
);

-TRACE_EVENT(ath10k_htt_rx_desc,
- TP_PROTO(struct ath10k *ar, void *rxdesc, u16 len),
-
- TP_ARGS(ar, rxdesc, len),
-
- TP_STRUCT__entry(
- __string(device, dev_name(ar->dev))
- __string(driver, dev_driver_string(ar->dev))
- __field(u16, len)
- __dynamic_array(u8, rxdesc, len)
- ),
-
- TP_fast_assign(
- __assign_str(device, dev_name(ar->dev));
- __assign_str(driver, dev_driver_string(ar->dev));
- __entry->len = len;
- memcpy(__get_dynamic_array(rxdesc), rxdesc, len);
- ),
-
- TP_printk(
- "%s %s len %hu",
- __get_str(driver),
- __get_str(device),
- __entry->len
- )
-);
-
TRACE_EVENT(ath10k_htt_tx,
TP_PROTO(struct ath10k *ar, u16 msdu_id, u16 msdu_len,
u8 vdev_id, u8 tid),
@@ -414,6 +387,11 @@ DEFINE_EVENT(ath10k_data_event, ath10k_wmi_bcn_tx,
TP_PROTO(struct ath10k *ar, void *data, size_t len),
TP_ARGS(ar, data, len)
);
+
+DEFINE_EVENT(ath10k_data_event, ath10k_htt_rx_desc,
+ TP_PROTO(struct ath10k *ar, void *data, size_t len),
+ TP_ARGS(ar, data, len)
+);
#endif /* _TRACE_H_ || TRACE_HEADER_MULTI_READ*/

/* we don't want to use include/trace/events */
--
1.8.5.3