Packet is not reclaimed when ath6kl_htc_rx_process_hdr() fails.
Fix this by deferring the packet deletion from comp_pktq till
ath6kl_htc_rx_process_hdr() returns success.
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/htc.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/htc.c b/drivers/net/wireless/ath/ath6kl/htc.c
index f88a7c9..7bc9884 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.c
+++ b/drivers/net/wireless/ath/ath6kl/htc.c
@@ -1643,7 +1643,6 @@ static int ath6kl_htc_rx_process_packets(struct htc_target *target,
int status = 0;
list_for_each_entry_safe(packet, tmp_pkt, comp_pktq, list) {
- list_del(&packet->list);
ep = &target->endpoint[packet->endpoint];
/* process header for each of the recv packet */
@@ -1652,6 +1651,8 @@ static int ath6kl_htc_rx_process_packets(struct htc_target *target,
if (status)
return status;
+ list_del(&packet->list);
+
if (list_empty(comp_pktq)) {
/*
* Last packet's more packet flag is set
--
1.7.0.4
It is just a four byte information of the received message
from ath6kl_htc_rxmsg_pending_handler(). Remove unnecessary
array representaion.
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/htc.c | 4 ++--
drivers/net/wireless/ath/ath6kl/htc.h | 2 +-
drivers/net/wireless/ath/ath6kl/htc_hif.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/htc.c b/drivers/net/wireless/ath/ath6kl/htc.c
index ca3d084..9a9eae5 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.c
+++ b/drivers/net/wireless/ath/ath6kl/htc.c
@@ -1770,7 +1770,7 @@ fail_rx:
}
int ath6kl_htc_rxmsg_pending_handler(struct htc_target *target,
- u32 msg_look_ahead[], int *num_pkts)
+ u32 msg_look_ahead, int *num_pkts)
{
struct htc_packet *packets, *tmp_pkt;
struct htc_endpoint *endpoint;
@@ -1787,7 +1787,7 @@ int ath6kl_htc_rxmsg_pending_handler(struct htc_target *target,
* On first entry copy the look_aheads into our temp array for
* processing
*/
- memcpy(look_aheads, msg_look_ahead, sizeof(look_aheads));
+ look_aheads[0] = msg_look_ahead;
while (true) {
diff --git a/drivers/net/wireless/ath/ath6kl/htc.h b/drivers/net/wireless/ath/ath6kl/htc.h
index 8ce0c2c..69d44e3 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.h
+++ b/drivers/net/wireless/ath/ath6kl/htc.h
@@ -563,7 +563,7 @@ int ath6kl_htc_get_rxbuf_num(struct htc_target *target,
int ath6kl_htc_add_rxbuf_multiple(struct htc_target *target,
struct list_head *pktq);
int ath6kl_htc_rxmsg_pending_handler(struct htc_target *target,
- u32 msg_look_ahead[], int *n_pkts);
+ u32 msg_look_ahead, int *n_pkts);
static inline void set_htc_pkt_info(struct htc_packet *packet, void *context,
u8 *buf, unsigned int len,
diff --git a/drivers/net/wireless/ath/ath6kl/htc_hif.c b/drivers/net/wireless/ath/ath6kl/htc_hif.c
index 86b1cc7..37a13f0 100644
--- a/drivers/net/wireless/ath/ath6kl/htc_hif.c
+++ b/drivers/net/wireless/ath/ath6kl/htc_hif.c
@@ -417,7 +417,7 @@ static int proc_pending_irqs(struct ath6kl_device *dev, bool *done)
* we rapidly pull packets.
*/
status = ath6kl_htc_rxmsg_pending_handler(dev->htc_cnxt,
- &lk_ahd, &fetched);
+ lk_ahd, &fetched);
if (status)
goto out;
--
1.7.0.4
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/htc.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/htc.c b/drivers/net/wireless/ath/ath6kl/htc.c
index 7bc9884..4a03dac 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.c
+++ b/drivers/net/wireless/ath/ath6kl/htc.c
@@ -1715,12 +1715,10 @@ static int ath6kl_htc_rx_fetch(struct htc_target *target,
packet = list_first_entry(rx_pktq, struct htc_packet,
list);
- list_del(&packet->list);
-
/* fully synchronous */
packet->completion = NULL;
- if (!list_empty(rx_pktq))
+ if (!list_is_singular(rx_pktq))
/*
* look_aheads in all packet
* except the last one in the
@@ -1735,7 +1733,7 @@ static int ath6kl_htc_rx_fetch(struct htc_target *target,
if (status)
return status;
- list_add_tail(&packet->list, comp_pktq);
+ list_move_tail(&packet->list, comp_pktq);
}
}
--
1.7.0.4
It is not necessary to process an htc_packet which is allocated for rx
but failed in sdio rx.
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
drivers/net/wireless/ath/ath6kl/htc.c | 48 +++++++++++++++++++++++---------
1 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/htc.c b/drivers/net/wireless/ath/ath6kl/htc.c
index 4a03dac..ca3d084 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.c
+++ b/drivers/net/wireless/ath/ath6kl/htc.c
@@ -1687,11 +1687,15 @@ static int ath6kl_htc_rx_fetch(struct htc_target *target,
int fetched_pkts;
bool part_bundle = false;
int status = 0;
+ struct list_head tmp_rxq;
+ struct htc_packet *packet, *tmp_pkt;
/* now go fetch the list of HTC packets */
while (!list_empty(rx_pktq)) {
fetched_pkts = 0;
+ INIT_LIST_HEAD(&tmp_rxq);
+
if (target->rx_bndl_enable && (get_queue_depth(rx_pktq) > 1)) {
/*
* There are enough packets to attempt a
@@ -1699,18 +1703,19 @@ static int ath6kl_htc_rx_fetch(struct htc_target *target,
* allowed.
*/
status = ath6kl_htc_rx_bundle(target, rx_pktq,
- comp_pktq,
+ &tmp_rxq,
&fetched_pkts,
part_bundle);
if (status)
- return status;
+ goto fail_rx;
if (!list_empty(rx_pktq))
part_bundle = true;
+
+ list_splice_tail_init(&tmp_rxq, comp_pktq);
}
if (!fetched_pkts) {
- struct htc_packet *packet;
packet = list_first_entry(rx_pktq, struct htc_packet,
list);
@@ -1730,13 +1735,37 @@ static int ath6kl_htc_rx_fetch(struct htc_target *target,
/* go fetch the packet */
status = ath6kl_htc_rx_packet(target, packet,
packet->act_len);
+
+ list_move_tail(&packet->list, &tmp_rxq);
+
if (status)
- return status;
+ goto fail_rx;
- list_move_tail(&packet->list, comp_pktq);
+ list_splice_tail_init(&tmp_rxq, comp_pktq);
}
}
+ return 0;
+
+fail_rx:
+
+ /*
+ * Cleanup any packets we allocated but didn't use to
+ * actually fetch any packets.
+ */
+
+ list_for_each_entry_safe(packet, tmp_pkt, rx_pktq, list) {
+ list_del(&packet->list);
+ htc_reclaim_rxbuf(target, packet,
+ &target->endpoint[packet->endpoint]);
+ }
+
+ list_for_each_entry_safe(packet, tmp_pkt, &tmp_rxq, list) {
+ list_del(&packet->list);
+ htc_reclaim_rxbuf(target, packet,
+ &target->endpoint[packet->endpoint]);
+ }
+
return status;
}
@@ -1826,15 +1855,6 @@ int ath6kl_htc_rxmsg_pending_handler(struct htc_target *target,
if (status) {
ath6kl_err("failed to get pending recv messages: %d\n",
status);
- /*
- * Cleanup any packets we allocated but didn't use to
- * actually fetch any packets.
- */
- list_for_each_entry_safe(packets, tmp_pkt, &rx_pktq, list) {
- list_del(&packets->list);
- htc_reclaim_rxbuf(target, packets,
- &target->endpoint[packets->endpoint]);
- }
/* cleanup any packets in sync completion queue */
list_for_each_entry_safe(packets, tmp_pkt, &comp_pktq, list) {
--
1.7.0.4
On 10/03/2011 02:41 PM, Vasanthakumar Thiagarajan wrote:
> On Mon, Oct 03, 2011 at 02:18:01PM +0300, Kalle Valo wrote:
>> On 09/29/2011 06:01 PM, Vasanthakumar Thiagarajan wrote:
>>> It is not necessary to process an htc_packet which is allocated for rx
>>> but failed in sdio rx.
>>
>> Same question here: does it fix a visible bug or is it more like a
>> theoretical issue?
>>
>> I would prefer that fixes would also state the impact of the bug: is it
>> a theoretical issue, a random bug which almost nobody sees or really bad
>> bug visible almost everywhere? That way it's easier to decide which
>> trees it should be sent.
>
> Ok, I can add this to commit log. I'll resend everything with V2.
Excellent, thanks a lot.
Kalle
On 09/29/2011 06:01 PM, Vasanthakumar Thiagarajan wrote:
> Packet is not reclaimed when ath6kl_htc_rx_process_hdr() fails.
> Fix this by deferring the packet deletion from comp_pktq till
> ath6kl_htc_rx_process_hdr() returns success.
Does this fix a user visible bug? Or is this just something you found
while doing code review?
Kalle
On Mon, Oct 03, 2011 at 02:11:50PM +0300, Kalle Valo wrote:
> On 09/29/2011 06:01 PM, Vasanthakumar Thiagarajan wrote:
> > Packet is not reclaimed when ath6kl_htc_rx_process_hdr() fails.
> > Fix this by deferring the packet deletion from comp_pktq till
> > ath6kl_htc_rx_process_hdr() returns success.
>
> Does this fix a user visible bug? Or is this just something you found
> while doing code review?
This one is found in code review, it does not fix any visible bug.
Vasanth
On Mon, Oct 03, 2011 at 02:18:01PM +0300, Kalle Valo wrote:
> On 09/29/2011 06:01 PM, Vasanthakumar Thiagarajan wrote:
> > It is not necessary to process an htc_packet which is allocated for rx
> > but failed in sdio rx.
>
> Same question here: does it fix a visible bug or is it more like a
> theoretical issue?
>
> I would prefer that fixes would also state the impact of the bug: is it
> a theoretical issue, a random bug which almost nobody sees or really bad
> bug visible almost everywhere? That way it's easier to decide which
> trees it should be sent.
Ok, I can add this to commit log. I'll resend everything with V2.
Vasanth
On 09/29/2011 06:01 PM, Vasanthakumar Thiagarajan wrote:
> It is not necessary to process an htc_packet which is allocated for rx
> but failed in sdio rx.
Same question here: does it fix a visible bug or is it more like a
theoretical issue?
I would prefer that fixes would also state the impact of the bug: is it
a theoretical issue, a random bug which almost nobody sees or really bad
bug visible almost everywhere? That way it's easier to decide which
trees it should be sent.
Kalle