Skip to contentSkip to conversation

add rx buffer recycle support to mt76x2e/mt76x0e drivers

PATCH Applied Lorenzo Bianconi 3 participants 9 messages Dec 3, 2018 – Dec 28, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover add rx buffer recycle support to mt76x2e/mt76x0e drivers

Add the capability to recycle rx buffers if they are not consumed by networking stack.
This series is based on 'mt76: add size check for additional rx fragments' series:
https://patchwork.kernel.org/patch/10709453/

Lorenzo Bianconi (2):
mt76: dma: do not build skb if reported len does not fit in buf_size
mt76: dma: add rx buffer recycle support

 drivers/net/wireless/mediatek/mt76/dma.c  | 75 +++++++++++++++++++----
 drivers/net/wireless/mediatek/mt76/mt76.h |  3 +
 2 files changed, 67 insertions(+), 11 deletions(-)
Signature · 1 line
2.19.2
#1 lore raw

PATCH 1/2 mt76: dma: do not build skb if reported len does not fit in buf_size

Precompute data length in order to avoid to allocate the related skb data structure if reported length does not fit in queue buf_size

---
 drivers/net/wireless/mediatek/mt76/dma.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/net/wireless/mediatek/mt76/dma.c +7 -8
index b7fd2e110663..1db163c40dcf 100644
@@ -417,10 +417,9 @@ mt76_add_fragment(struct mt76_dev *dev, struct mt76_queue *q, void *data,
 static int
 mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 {
+	int len, data_len, done = 0;
 	struct sk_buff *skb;
 	unsigned char *data;
-	int len;
-	int done = 0;
 	bool more;
 
 	while (done < budget) {
@@ -430,7 +429,12 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 		if (!data)
 			break;
 
-		if (q->buf_size < len + q->buf_offset) {
+		if (q->rx_head)
+			data_len = q->buf_size;
+		else
+			data_len = SKB_WITH_OVERHEAD(q->buf_size);
+
+		if (data_len < len + q->buf_offset) {
 			dev_kfree_skb(q->rx_head);
 			q->rx_head = NULL;
 
@@ -448,12 +452,7 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 			skb_free_frag(data);
 			continue;
 		}
-
 		skb_reserve(skb, q->buf_offset);
-		if (skb->tail + len > skb->end) {
-			dev_kfree_skb(skb);
-			continue;
-		}
 
 		if (q == &dev->q_rx[MT_RXQ_MCU]) {
 			u32 *rxfce = (u32 *) skb->cb;
Signature · 1 line
2.19.2
#2 lore raw

Re: [PATCH 1/2] mt76: dma: do not build skb if reported len does not fit in buf_size

Quoted 53 lines

Precompute data length in order to avoid to allocate the related skb data structure if reported length does not fit in queue buf_size

---
 drivers/net/wireless/mediatek/mt76/dma.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/net/wireless/mediatek/mt76/dma.c +7 -9
index b7fd2e110663..1db163c40dcf 100644
@@ -417,10 +417,9 @@ mt76_add_fragment(struct mt76_dev *dev, struct mt76_queue *q, void *data,
 static int
 mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 {
+       int len, data_len, done = 0;
        struct sk_buff *skb;
        unsigned char *data;
-       int len;
-       int done = 0;
        bool more;
 
        while (done < budget) {
@@ -430,7 +429,12 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
                if (!data)
                        break;
 
-               if (q->buf_size < len + q->buf_offset) {
+               if (q->rx_head)
+                       data_len = q->buf_size;
+               else
+                       data_len = SKB_WITH_OVERHEAD(q->buf_size);
+
+               if (data_len < len + q->buf_offset) {
                        dev_kfree_skb(q->rx_head);
                        q->rx_head = NULL;
 
@@ -448,12 +452,7 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
                        skb_free_frag(data);
                        continue;
                }
-
                skb_reserve(skb, q->buf_offset);
-               if (skb->tail + len > skb->end) {
-                       dev_kfree_skb(skb);
-                       continue;
-               }
 
                if (q == &dev->q_rx[MT_RXQ_MCU]) {
                        u32 *rxfce = (u32 *) skb->cb;
--
Quoted 2 lines

2.19.2

Hi Felix,

what about this patch?

Regards,
Lorenzo

#4 lore raw

PATCH 2/2 mt76: dma: add rx buffer recycle support

In reply to Lorenzo Bianconi

Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch

---
 drivers/net/wireless/mediatek/mt76/dma.c  | 60 +++++++++++++++++++++--
 drivers/net/wireless/mediatek/mt76/mt76.h |  3 ++
 2 files changed, 60 insertions(+), 3 deletions(-)
drivers/net/wireless/mediatek/mt76/dma.c +57 -3
index 1db163c40dcf..eceadfa3f980 100644
@@ -39,6 +39,15 @@ mt76_dma_alloc_queue(struct mt76_dev *dev, struct mt76_queue *q)
 	if (!q->entry)
 		return -ENOMEM;
 
+	/* allocate recycle buffer ring */
+	if (q == &dev->q_rx[MT_RXQ_MCU] ||
+	    q == &dev->q_rx[MT_RXQ_MAIN]) {
+		size = q->ndesc * sizeof(*q->recycle);
+		q->recycle = devm_kzalloc(dev->dev, size, GFP_KERNEL);
+		if (!q->recycle)
+			return -ENOMEM;
+	}
+
 	/* clear descriptors */
 	for (i = 0; i < q->ndesc; i++)
 		q->desc[i].ctrl = cpu_to_le32(MT_DMA_CTL_DMA_DONE);
@@ -317,6 +326,49 @@ int mt76_dma_tx_queue_skb(struct mt76_dev *dev, struct mt76_queue *q,
 }
 EXPORT_SYMBOL_GPL(mt76_dma_tx_queue_skb);
 
+/* caller must hold mt76_queue spinlock */
+static u8 *mt76_dma_get_free_buf(struct mt76_queue *q, bool flush)
+{
+	if (q->recycle[q->rhead] || flush) {
+		u8 *buff = q->recycle[q->rhead];
+
+		q->recycle[q->rhead] = NULL;
+		q->rhead = (q->rhead + 1) % q->ndesc;
+		return buff;
+	}
+
+	return page_frag_alloc(&q->rx_page, q->buf_size, GFP_ATOMIC);
+}
+
+static void
+mt76_dma_set_recycle_buf(struct mt76_queue *q, u8 *data)
+{
+	spin_lock_bh(&q->lock);
+	if (!q->recycle[q->rtail]) {
+		q->recycle[q->rtail] = data;
+		q->rtail = (q->rtail + 1) % q->ndesc;
+	} else {
+		skb_free_frag(data);
+	}
+	spin_unlock_bh(&q->lock);
+}
+
+static void
+mt76_dma_free_recycle_ring(struct mt76_queue *q)
+{
+	u8 *buf;
+
+	spin_lock_bh(&q->lock);
+	while (true) {
+		buf = mt76_dma_get_free_buf(q, true);
+		if (!buf)
+			break;
+
+		skb_free_frag(buf);
+	}
+	spin_unlock_bh(&q->lock);
+}
+
 static int
 mt76_dma_rx_fill(struct mt76_dev *dev, struct mt76_queue *q)
 {
@@ -332,7 +384,7 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct mt76_queue *q)
 	while (q->queued < q->ndesc - 1) {
 		struct mt76_queue_buf qbuf;
 
-		buf = page_frag_alloc(&q->rx_page, q->buf_size, GFP_ATOMIC);
+		buf = mt76_dma_get_free_buf(q, false);
 		if (!buf)
 			break;
 
@@ -373,6 +425,8 @@ mt76_dma_rx_cleanup(struct mt76_dev *dev, struct mt76_queue *q)
 	} while (1);
 	spin_unlock_bh(&q->lock);
 
+	mt76_dma_free_recycle_ring(q);
+
 	if (!q->rx_page.va)
 		return;
 
@@ -438,7 +492,7 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 			dev_kfree_skb(q->rx_head);
 			q->rx_head = NULL;
 
-			skb_free_frag(data);
+			mt76_dma_set_recycle_buf(q, data);
 			continue;
 		}
 
@@ -449,7 +503,7 @@ mt76_dma_rx_process(struct mt76_dev *dev, struct mt76_queue *q, int budget)
 
 		skb = build_skb(data, q->buf_size);
 		if (!skb) {
-			skb_free_frag(data);
+			mt76_dma_set_recycle_buf(q, data);
 			continue;
 		}
 		skb_reserve(skb, q->buf_offset);
drivers/net/wireless/mediatek/mt76/mt76.h +3 -0
index 5cd508a68609..95546c744494 100644
@@ -114,6 +114,9 @@ struct mt76_queue {
 	spinlock_t lock;
 	struct mt76_queue_entry *entry;
 	struct mt76_desc *desc;
+	/* recycle ring */
+	u16 rhead, rtail;
+	u8 **recycle;
 
 	struct list_head swq;
 	int swq_queued;
Signature · 1 line
2.19.2
#5 lore raw

Re: [PATCH 2/2] mt76: dma: add rx buffer recycle support

Quoted 6 lines

Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch

---

Felix,

could you please drop this patch since it does not help to reduce pressure on page_frag_cache.

Regards,
Lorenzo

#6 lore raw

Re: [PATCH 2/2] mt76: dma: add rx buffer recycle support

Quoted 11 lines · Lorenzo Bianconi
Quoted 6 lines

Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch

---

Felix,

could you please drop this patch since it does not help to reduce pressure on page_frag_cache.

What is the problem ? Maybe using kmalloc() instead of page_frag_alloc() could help (kmalloc has standard kmem_cache for 2048 bytes object) ?

Thanks
Stanislaw

#7 lore raw

Re: [PATCH 2/2] mt76: dma: add rx buffer recycle support

Quoted 8 lines · Lorenzo Bianconi
Quoted 7 lines
Quoted 6 lines

Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch

---

Quoted 7 lines
Quoted 4 lines

Felix,

could you please drop this patch since it does not help to reduce pressure on page_frag_cache.

What is the problem ? Maybe using kmalloc() instead of page_frag_alloc() could help (kmalloc has standard kmem_cache for 2048 bytes object) ?

Hi Stanislaw,

I think the only difference in using a recycle buffer with page_frag_cache is we are a little bit less greedy in consuming the compound page since in case of error we will reuse the previously allocated fragment. However we will need to reallocate a new compound page if we have a leftover fragment that 'locks' the previous compound (we have the same issue if we do not use the recycle buffer). Does this 'little' improvement worth a more complex code? Do you agree or is there something I am missing here?

Regards,
Lorenzo

Quoted 3 lines

Thanks
Stanislaw

#8 lore raw

Re: [PATCH 2/2] mt76: dma: add rx buffer recycle support

Quoted 18 lines · Lorenzo Bianconi
Quoted 8 lines · Lorenzo Bianconi
Quoted 7 lines
Quoted 6 lines

Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch

---

Quoted 7 lines
Quoted 4 lines

Felix,

could you please drop this patch since it does not help to reduce pressure on page_frag_cache.

What is the problem ? Maybe using kmalloc() instead of page_frag_alloc() could help (kmalloc has standard kmem_cache for 2048 bytes object) ?

Hi Stanislaw,

Quoted 7 lines

I think the only difference in using a recycle buffer with page_frag_cache is we are a little bit less greedy in consuming the compound page since in case of error we will reuse the previously allocated fragment. However we will need to reallocate a new compound page if we have a leftover fragment that 'locks' the previous compound (we have the same issue if we do not use the recycle buffer). Does this 'little' improvement worth a more complex code? Do you agree or is there something I am missing here?

I was not asking about the patch. I agree it should be droped.

I was asking what is the problem with "pressure on page_frag_cache" and if using kmalloc() instead of page_frag_alloc() whould be potential solution.

Regards
Stanislaw