msm: rmnet_bam: Handle Network TX Queue Race Condition
The netif TX queue is started and stopped based upon the watermark state
maintained by BAM DMUX. The queue is checked after TX-enqueue
operations to disable the queue if the high watermark is hit and after
TX-dequeue operations to enable the queue if the low watermark is hit.
Without locking, this opens a race condition where the TX-dequeue check
and the call to stop the queue can happen after the write-done
completion has occurred which would restart the queue. This results in
the queue being stopped indefinitely which results in a permanent data
stall.
CRs-Fixed: 347174
Change-Id: I5975815f7ce774c1230f0254b40dfb3f056482c4
Signed-off-by: Eric Holmberg <eholmber@codeaurora.org>
diff --git a/arch/arm/mach-msm/bam_dmux.c b/arch/arm/mach-msm/bam_dmux.c
index 89d7fc9..6361d6d 100644
--- a/arch/arm/mach-msm/bam_dmux.c
+++ b/arch/arm/mach-msm/bam_dmux.c
@@ -982,11 +982,13 @@
int msm_bam_dmux_is_ch_low(uint32_t id)
{
+ unsigned long flags;
int ret;
if (id >= BAM_DMUX_NUM_CHANNELS)
return -EINVAL;
+ spin_lock_irqsave(&bam_ch[id].lock, flags);
bam_ch[id].use_wm = 1;
ret = bam_ch[id].num_tx_pkts <= LOW_WATERMARK;
DBG("%s: ch %d num tx pkts=%d, LWM=%d\n", __func__,
@@ -995,6 +997,7 @@
ret = -ENODEV;
pr_err("%s: port not open: %d\n", __func__, bam_ch[id].status);
}
+ spin_unlock_irqrestore(&bam_ch[id].lock, flags);
return ret;
}
diff --git a/drivers/net/msm_rmnet_bam.c b/drivers/net/msm_rmnet_bam.c
index 401b63c..bbcf3c4 100644
--- a/drivers/net/msm_rmnet_bam.c
+++ b/drivers/net/msm_rmnet_bam.c
@@ -79,6 +79,7 @@
#endif
struct sk_buff *waiting_for_ul_skb;
spinlock_t lock;
+ spinlock_t tx_queue_lock;
struct tasklet_struct tsklt;
u32 operation_mode; /* IOCTL specified mode (protocol, QoS header) */
uint8_t device_up;
@@ -321,6 +322,7 @@
{
struct rmnet_private *p = netdev_priv(dev);
u32 opmode = p->operation_mode;
+ unsigned long flags;
DBG1("%s: write complete\n", __func__);
if (RMNET_IS_MODE_IP(opmode) ||
@@ -335,12 +337,15 @@
((struct net_device *)(dev))->name, p->stats.tx_packets,
skb->len, skb->mark);
dev_kfree_skb_any(skb);
+
+ spin_lock_irqsave(&p->tx_queue_lock, flags);
if (netif_queue_stopped(dev) &&
msm_bam_dmux_is_ch_low(p->ch_id)) {
DBG0("%s: Low WM hit, waking queue=%p\n",
__func__, skb);
netif_wake_queue(dev);
}
+ spin_unlock_irqrestore(&p->tx_queue_lock, flags);
}
static void bam_notify(void *dev, int event, unsigned long data)
@@ -509,10 +514,12 @@
goto exit;
}
+ spin_lock_irqsave(&p->tx_queue_lock, flags);
if (msm_bam_dmux_is_ch_full(p->ch_id)) {
netif_stop_queue(dev);
DBG0("%s: High WM hit, stopping queue=%p\n", __func__, skb);
}
+ spin_unlock_irqrestore(&p->tx_queue_lock, flags);
exit:
msm_bam_dmux_ul_power_unvote();
@@ -770,6 +777,7 @@
p->waiting_for_ul_skb = NULL;
p->in_reset = 0;
spin_lock_init(&p->lock);
+ spin_lock_init(&p->tx_queue_lock);
#ifdef CONFIG_MSM_RMNET_DEBUG
p->timeout_us = timeout_us;
p->wakeups_xmit = p->wakeups_rcv = 0;