[PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[Florian Westphal]

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(-)

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:
-- 
2.0.5

Re: [PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[Christian Lamparter]

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

Re: [PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[Florian Westphal]

Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 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-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
> > ---
> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH next] drivers: wireless: carl9170: shrink carl9170_tx_info

[Christian Lamparter]

On Saturday, March 14, 2015 06:55:32 PM Florian Westphal wrote:
> Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 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-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
> > > ---
> > 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html