| Message ID | 1403239692-3352-1-git-send-email-bzhao@marvell.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 19 Jun 2014, Bing Zhao wrote: > @@ -143,8 +142,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len) > len - sizeof(struct ieee80211_hdr_3addr)); > > skb->priority = LOW_PRIO_TID; > - do_gettimeofday(&tv); > - skb->tstamp = timeval_to_ktime(tv); > + __net_timestamp(skb); and __net_timestamp(skb) does skb->tstamp = ktime_get_real(); ... > mwifiex_wmm_compute_drv_pkt_delay(struct mwifiex_private *priv, > const struct sk_buff *skb) > { > + u32 queue_delay = ktime_to_ms(ktime_sub(ktime_get(), skb->tstamp)); So now you do: delay = now(CLOCK_MONOTONIC) - tstamp(CLOCK_REALTIME); Brilliant brainfart! My original patch merily converted open coded crap to a consistent one. And all of this was using the same time base: CLOCK_REALTIME. Johannes rightfully started a discussion about CLOCK_REALTIME vs. CLOCk_MONOTONIC suitability for certain use cases and then you send a patch which subtracts different time bases and assign the responsibility for that to Johannes: > v3: don't use net_timedelta() (Johannes Berg) Did you ever test that patch? Clearly not. Simply because on every machine where now(CLOCK_MONOTONIC) != now(CLOCK_REALTIME) this falls apart in bits and pieces. And that's the sane case, not the stupid eval board without a RTC driver which defaults to 1970:00:00:00:00 and happens to have CLOCK_MONOTONIC and CLOCK_REALTIME in sync. So aside of not testing it proper, you clearly have no clue what you are doing and then you have the chuzpe to claim that Johannes asked you to do so. Nice try. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Thomas, > > @@ -143,8 +142,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len) > > len - sizeof(struct ieee80211_hdr_3addr)); > > > > skb->priority = LOW_PRIO_TID; > > - do_gettimeofday(&tv); > > - skb->tstamp = timeval_to_ktime(tv); > > + __net_timestamp(skb); > > and __net_timestamp(skb) does > > skb->tstamp = ktime_get_real(); Ah, I didn't realize that __net_timestamp also does ktime_get_real. Thanks for pointing it out! > > ... > > > mwifiex_wmm_compute_drv_pkt_delay(struct mwifiex_private *priv, > > const struct sk_buff *skb) > > { > > + u32 queue_delay = ktime_to_ms(ktime_sub(ktime_get(), skb->tstamp)); > > So now you do: > > delay = now(CLOCK_MONOTONIC) - tstamp(CLOCK_REALTIME); > > Brilliant brainfart! > > My original patch merily converted open coded crap to a consistent > one. And all of this was using the same time base: CLOCK_REALTIME. > > Johannes rightfully started a discussion about CLOCK_REALTIME > vs. CLOCk_MONOTONIC suitability for certain use cases and then you > send a patch which subtracts different time bases and assign the > responsibility for that to Johannes: > > > v3: don't use net_timedelta() (Johannes Berg) I just wanted to thank Johannes for commenting. > > Did you ever test that patch? Clearly not. Simply because on every > machine where now(CLOCK_MONOTONIC) != now(CLOCK_REALTIME) this falls > apart in bits and pieces. And that's the sane case, not the stupid > eval board without a RTC driver which defaults to 1970:00:00:00:00 and > happens to have CLOCK_MONOTONIC and CLOCK_REALTIME in sync. > > So aside of not testing it proper, you clearly have no clue what you > are doing and then you have the chuzpe to claim that Johannes asked > you to do so. I will revert this patch. Thank you again for pointing out my error. Regards, Bing > > Nice try. > > Thanks, > > tglx > > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c index 6ec2ee3..1a72b2f 100644 --- a/drivers/net/wireless/mwifiex/cfg80211.c +++ b/drivers/net/wireless/mwifiex/cfg80211.c @@ -121,7 +121,6 @@ mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len) u8 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}; u16 pkt_len; u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT; - struct timeval tv; pkt_len = len + ETH_ALEN; @@ -143,8 +142,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len) len - sizeof(struct ieee80211_hdr_3addr)); skb->priority = LOW_PRIO_TID; - do_gettimeofday(&tv); - skb->tstamp = timeval_to_ktime(tv); + __net_timestamp(skb); return 0; } diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c index 1261d9a..657504c 100644 --- a/drivers/net/wireless/mwifiex/main.c +++ b/drivers/net/wireless/mwifiex/main.c @@ -609,7 +609,6 @@ mwifiex_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev); struct sk_buff *new_skb; struct mwifiex_txinfo *tx_info; - struct timeval tv; dev_dbg(priv->adapter->dev, "data: %lu BSS(%d-%d): Data <= kernel\n", jiffies, priv->bss_type, priv->bss_num); @@ -656,8 +655,7 @@ mwifiex_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) * firmware for aggregate delay calculation for stats and * MSDU lifetime expiry. */ - do_gettimeofday(&tv); - skb->tstamp = timeval_to_ktime(tv); + __net_timestamp(skb); mwifiex_queue_tx_pkt(priv, skb); diff --git a/drivers/net/wireless/mwifiex/tdls.c b/drivers/net/wireless/mwifiex/tdls.c index e73034f..3efbcbe 100644 --- a/drivers/net/wireless/mwifiex/tdls.c +++ b/drivers/net/wireless/mwifiex/tdls.c @@ -530,7 +530,6 @@ int mwifiex_send_tdls_data_frame(struct mwifiex_private *priv, const u8 *peer, { struct sk_buff *skb; struct mwifiex_txinfo *tx_info; - struct timeval tv; int ret; u16 skb_len; @@ -608,8 +607,7 @@ int mwifiex_send_tdls_data_frame(struct mwifiex_private *priv, const u8 *peer, tx_info->bss_num = priv->bss_num; tx_info->bss_type = priv->bss_type; - do_gettimeofday(&tv); - skb->tstamp = timeval_to_ktime(tv); + __net_timestamp(skb); mwifiex_queue_tx_pkt(priv, skb); return 0; @@ -702,7 +700,6 @@ int mwifiex_send_tdls_action_frame(struct mwifiex_private *priv, const u8 *peer, { struct sk_buff *skb; struct mwifiex_txinfo *tx_info; - struct timeval tv; u8 *pos; u32 pkt_type, tx_control; u16 pkt_len, skb_len; @@ -767,8 +764,7 @@ int mwifiex_send_tdls_action_frame(struct mwifiex_private *priv, const u8 *peer, pkt_len = skb->len - MWIFIEX_MGMT_FRAME_HEADER_SIZE - sizeof(pkt_len); memcpy(skb->data + MWIFIEX_MGMT_FRAME_HEADER_SIZE, &pkt_len, sizeof(pkt_len)); - do_gettimeofday(&tv); - skb->tstamp = timeval_to_ktime(tv); + __net_timestamp(skb); mwifiex_queue_tx_pkt(priv, skb); return 0; diff --git a/drivers/net/wireless/mwifiex/uap_txrx.c b/drivers/net/wireless/mwifiex/uap_txrx.c index 57fa47d..ddfc3c6 100644 --- a/drivers/net/wireless/mwifiex/uap_txrx.c +++ b/drivers/net/wireless/mwifiex/uap_txrx.c @@ -96,7 +96,6 @@ static void mwifiex_uap_queue_bridged_pkt(struct mwifiex_private *priv, struct sk_buff *new_skb; struct mwifiex_txinfo *tx_info; int hdr_chop; - struct timeval tv; struct ethhdr *p_ethhdr; uap_rx_pd = (struct uap_rxpd *)(skb->data); @@ -192,8 +191,7 @@ static void mwifiex_uap_queue_bridged_pkt(struct mwifiex_private *priv, tx_info->pkt_len = skb->len; } - do_gettimeofday(&tv); - skb->tstamp = timeval_to_ktime(tv); + __net_timestamp(skb); mwifiex_wmm_add_buf_txqueue(priv, skb); atomic_inc(&adapter->tx_pending); atomic_inc(&adapter->pending_bridged_pkts); diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c index 8cd123e..e8556b6 100644 --- a/drivers/net/wireless/mwifiex/wmm.c +++ b/drivers/net/wireless/mwifiex/wmm.c @@ -878,15 +878,8 @@ u8 mwifiex_wmm_compute_drv_pkt_delay(struct mwifiex_private *priv, const struct sk_buff *skb) { + u32 queue_delay = ktime_to_ms(ktime_sub(ktime_get(), skb->tstamp)); u8 ret_val; - struct timeval out_tstamp, in_tstamp; - u32 queue_delay; - - do_gettimeofday(&out_tstamp); - in_tstamp = ktime_to_timeval(skb->tstamp); - - queue_delay = (out_tstamp.tv_sec - in_tstamp.tv_sec) * 1000; - queue_delay += (out_tstamp.tv_usec - in_tstamp.tv_usec) / 1000; /* * Queue delay is passed as a uint8 in units of 2ms (ms shifted