| Message ID | 1426261045-22373-1-git-send-email-fw@strlen.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On Friday, March 13, 2015 04:37:25 PM Florian Westphal wrote: > its embededded inside rate_driver_data of the ieee80211_tx_info struct, > which in turn is stored in skb->cb[]. > > In order to shrink cb, we need to shrink ieee80211_tx_info which means > to downsize all users first. > > Alternatively, one might be able to remove kref but > its less intrusive/simpler to use u32 for timeout handling. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- u32 jiffies... that's a lot of pointing (well not so much) and really ugly casting (a lot). I guess it would be easier to just use a "per-queue" timeout watchdog like almost everybody else. This way, the driver will be ready for the next skb->cb shrink as well. carl9170_tx_ampdu_timeout can be completely removed. Regards, Christian -- 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
Christian Lamparter <chunkeey@googlemail.com> wrote: > On Friday, March 13, 2015 04:37:25 PM Florian Westphal wrote: > > its embededded inside rate_driver_data of the ieee80211_tx_info struct, > > which in turn is stored in skb->cb[]. > > > > In order to shrink cb, we need to shrink ieee80211_tx_info which means > > to downsize all users first. > > > > Alternatively, one might be able to remove kref but > > its less intrusive/simpler to use u32 for timeout handling. > > > > Signed-off-by: Florian Westphal <fw@strlen.de> > > --- > u32 jiffies... that's a lot of pointing (well not so much) and really > ugly casting (a lot). I guess it would be easier to just use a "per-queue" > timeout watchdog like almost everybody else. This way, the driver will > be ready for the next skb->cb shrink as well. > > carl9170_tx_ampdu_timeout can be completely removed. Maybe, but there is only so much I am willing to do with "compile tested only" patches... -- 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
On Saturday, March 14, 2015 06:55:32 PM Florian Westphal wrote: > Christian Lamparter <chunkeey@googlemail.com> wrote: > > On Friday, March 13, 2015 04:37:25 PM Florian Westphal wrote: > > > its embededded inside rate_driver_data of the ieee80211_tx_info struct, > > > which in turn is stored in skb->cb[]. > > > > > > In order to shrink cb, we need to shrink ieee80211_tx_info which means > > > to downsize all users first. > > > > > > Alternatively, one might be able to remove kref but > > > its less intrusive/simpler to use u32 for timeout handling. > > > > > > Signed-off-by: Florian Westphal <fw@strlen.de> > > > --- > > u32 jiffies... that's a lot of pointing (well not so much) and really > > ugly casting (a lot). I guess it would be easier to just use a "per-queue" > > timeout watchdog like almost everybody else. This way, the driver will > > be ready for the next skb->cb shrink as well. > > > > carl9170_tx_ampdu_timeout can be completely removed. > > Maybe, but there is only so much I am willing to do with "compile > tested only" patches... well, reinventing jiffies_32 is just a ugly hack and not "intrusive/simpler" than removing kref here. Please try again. Don't worry about testing, I'm sure this is not a problem and someone will test the patches with the hw on 32-bit and 64-bit platforms. Regards, Christian -- 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/ath/carl9170/carl9170.h b/drivers/net/wireless/ath/carl9170/carl9170.h index 237d0cd..a785300 100644 --- a/drivers/net/wireless/ath/carl9170/carl9170.h +++ b/drivers/net/wireless/ath/carl9170/carl9170.h @@ -491,9 +491,9 @@ struct carl9170_sta_info { }; struct carl9170_tx_info { - unsigned long timeout; - struct ar9170 *ar; + u32 timeout32; struct kref ref; + struct ar9170 *ar; }; #define CHK_DEV_STATE(a, s) (((struct ar9170 *)a)->state >= (s)) diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c index 6808db4..d00ab9d 100644 --- a/drivers/net/wireless/ath/carl9170/debug.c +++ b/drivers/net/wireless/ath/carl9170/debug.c @@ -291,7 +291,7 @@ static void carl9170_debugfs_format_frame(struct ar9170 *ar, "pc:%.8x, to:%d ms\n", prefix, skb, txc->s.cookie, ieee80211_get_DA(hdr), get_seq_h(hdr), le16_to_cpu(txc->f.mac_control), le32_to_cpu(txc->f.phy_control), - jiffies_to_msecs(jiffies - arinfo->timeout)); + jiffies_to_msecs(((u32)jiffies) - arinfo->timeout32)); } diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c index ae86a600..d576747 100644 --- a/drivers/net/wireless/ath/carl9170/tx.c +++ b/drivers/net/wireless/ath/carl9170/tx.c @@ -555,6 +555,20 @@ static void carl9170_tx_fill_rateinfo(struct ar9170 *ar, unsigned int rix, } } +static inline bool carl9170_time_is_before_jiffies(u32 timeout) +{ + u32 now = (u32)jiffies; + + return (s32)(now - timeout) >= 0; +} + +static inline bool carl9170_time_is_after_jiffies(u32 timeout) +{ + u32 now = (u32)jiffies; + + return (s32)(now - timeout) < 0; +} + static void carl9170_check_queue_stop_timeout(struct ar9170 *ar) { int i; @@ -574,8 +588,8 @@ static void carl9170_check_queue_stop_timeout(struct ar9170 *ar) txinfo = IEEE80211_SKB_CB(skb); arinfo = (void *) txinfo->rate_driver_data; - if (time_is_before_jiffies(arinfo->timeout + - msecs_to_jiffies(CARL9170_QUEUE_STUCK_TIMEOUT)) == true) + if (carl9170_time_is_before_jiffies(arinfo->timeout32 + + (u32)msecs_to_jiffies(CARL9170_QUEUE_STUCK_TIMEOUT))) restart = true; next: @@ -620,7 +634,7 @@ static void carl9170_tx_ampdu_timeout(struct ar9170 *ar) txinfo = IEEE80211_SKB_CB(skb); arinfo = (void *)txinfo->rate_driver_data; - if (time_is_after_jiffies(arinfo->timeout + + if (carl9170_time_is_after_jiffies(arinfo->timeout32 + msecs_to_jiffies(CARL9170_QUEUE_TIMEOUT))) goto unlock; @@ -1066,7 +1080,7 @@ static int carl9170_tx_prepare(struct ar9170 *ar, txc->f.mac_control = mac_tmp; arinfo = (void *)info->rate_driver_data; - arinfo->timeout = jiffies; + arinfo->timeout32 = (u32)jiffies; arinfo->ar = ar; kref_init(&arinfo->ref); return 0; @@ -1259,7 +1273,7 @@ static struct sk_buff *carl9170_tx_pick_skb(struct ar9170 *ar, info = IEEE80211_SKB_CB(skb); arinfo = (void *) info->rate_driver_data; - arinfo->timeout = jiffies; + arinfo->timeout32 = (u32)jiffies; return skb; err_unlock:
its embededded inside rate_driver_data of the ieee80211_tx_info struct, which in turn is stored in skb->cb[]. In order to shrink cb, we need to shrink ieee80211_tx_info which means to downsize all users first. Alternatively, one might be able to remove kref but its less intrusive/simpler to use u32 for timeout handling. Signed-off-by: Florian Westphal <fw@strlen.de> --- drivers/net/wireless/ath/carl9170/carl9170.h | 4 ++-- drivers/net/wireless/ath/carl9170/debug.c | 2 +- drivers/net/wireless/ath/carl9170/tx.c | 24 +++++++++++++++++++----- 3 files changed, 22 insertions(+), 8 deletions(-)