| Message ID | 8c05c03018ca9f98047ff961028f09da2e1565d0.1543846816.git.lorenzo.bianconi@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | add rx buffer recycle support to mt76x2e/mt76x0e drivers | expand |
> > Add support for recycling rx buffers if they are not forwarded > to network stack instead of reallocate them from scratch > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- Felix, could you please drop this patch since it does not help to reduce pressure on page_frag_cache. Regards, Lorenzo
On Wed, Dec 05, 2018 at 11:37:33AM +0100, Lorenzo Bianconi wrote: > > > > Add support for recycling rx buffers if they are not forwarded > > to network stack instead of reallocate them from scratch > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > 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
> On Wed, Dec 05, 2018 at 11:37:33AM +0100, Lorenzo Bianconi wrote: > > > > > > Add support for recycling rx buffers if they are not forwarded > > > to network stack instead of reallocate them from scratch > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > --- > > > > 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 > > Thanks > Stanislaw
On Wed, Dec 05, 2018 at 04:17:31PM +0100, Lorenzo Bianconi wrote: > > On Wed, Dec 05, 2018 at 11:37:33AM +0100, Lorenzo Bianconi wrote: > > > > > > > > Add support for recycling rx buffers if they are not forwarded > > > > to network stack instead of reallocate them from scratch > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > > --- > > > > > > 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? 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
diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c index 1db163c40dcf..eceadfa3f980 100644 --- a/drivers/net/wireless/mediatek/mt76/dma.c +++ b/drivers/net/wireless/mediatek/mt76/dma.c @@ -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); diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h index 5cd508a68609..95546c744494 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76.h +++ b/drivers/net/wireless/mediatek/mt76/mt76.h @@ -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;
Add support for recycling rx buffers if they are not forwarded to network stack instead of reallocate them from scratch Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- drivers/net/wireless/mediatek/mt76/dma.c | 60 +++++++++++++++++++++-- drivers/net/wireless/mediatek/mt76/mt76.h | 3 ++ 2 files changed, 60 insertions(+), 3 deletions(-)