* [RFC 1/2] mt76: remove rx_page_lock
@ 2018-10-03 9:17 Stanislaw Gruszka
2018-10-03 9:17 ` [RFC 2/2] mt76: make frag_cache global per cpu structure Stanislaw Gruszka
0 siblings, 1 reply; 5+ messages in thread
From: Stanislaw Gruszka @ 2018-10-03 9:17 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Lorenzo Bianconi, linux-wireless
Extra serializaion for protecting q->rx_page is not needed,
we stop rx_tasklet before we nulify it in mt76u_free_rx().
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/mediatek/mt76/mt76.h | 1 -
drivers/net/wireless/mediatek/mt76/usb.c | 8 +-------
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index f2dd4d87e355..2ab524c8f14f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -122,7 +122,6 @@ struct mt76_queue {
dma_addr_t desc_dma;
struct sk_buff *rx_head;
struct page_frag_cache rx_page;
- spinlock_t rx_page_lock;
};
struct mt76_mcu_ops {
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index 6b643ea701e3..a103b77ae8c4 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -280,7 +280,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
struct urb *urb = buf->urb;
int i;
- spin_lock_bh(&q->rx_page_lock);
for (i = 0; i < nsgs; i++) {
struct page *page;
void *data;
@@ -294,7 +293,6 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
offset = data - page_address(page);
sg_set_page(&urb->sg[i], page, sglen, offset);
}
- spin_unlock_bh(&q->rx_page_lock);
if (i < nsgs) {
int j;
@@ -523,7 +521,6 @@ static int mt76u_alloc_rx(struct mt76_dev *dev)
struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
int i, err, nsgs;
- spin_lock_init(&q->rx_page_lock);
spin_lock_init(&q->lock);
q->entry = devm_kzalloc(dev->dev,
MT_NUM_RX_ENTRIES * sizeof(*q->entry),
@@ -561,15 +558,12 @@ static void mt76u_free_rx(struct mt76_dev *dev)
for (i = 0; i < q->ndesc; i++)
mt76u_buf_free(&q->entry[i].ubuf);
- spin_lock_bh(&q->rx_page_lock);
if (!q->rx_page.va)
- goto out;
+ return;
page = virt_to_page(q->rx_page.va);
__page_frag_cache_drain(page, q->rx_page.pagecnt_bias);
memset(&q->rx_page, 0, sizeof(q->rx_page));
-out:
- spin_unlock_bh(&q->rx_page_lock);
}
static void mt76u_stop_rx(struct mt76_dev *dev)
--
2.7.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC 2/2] mt76: make frag_cache global per cpu structure
2018-10-03 9:17 [RFC 1/2] mt76: remove rx_page_lock Stanislaw Gruszka
@ 2018-10-03 9:17 ` Stanislaw Gruszka
2018-10-03 9:38 ` Lorenzo Bianconi
2018-10-03 10:50 ` Felix Fietkau
0 siblings, 2 replies; 5+ messages in thread
From: Stanislaw Gruszka @ 2018-10-03 9:17 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Lorenzo Bianconi, linux-wireless
Make mt76 frag cache similar to netdev frag cache. This should
make frag allocation safe regarding concurrent access and also
be more efficient since we will use pages that most likely are
hot on particular cpu.
And we don't need to clean up the cache up during device removal.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/mediatek/mt76/dma.c | 2 +-
drivers/net/wireless/mediatek/mt76/usb.c | 10 +---------
drivers/net/wireless/mediatek/mt76/util.c | 16 ++++++++++++++++
drivers/net/wireless/mediatek/mt76/util.h | 1 +
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
index f7fbd7016403..59453a7781c5 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -328,7 +328,7 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct mt76_queue *q, bool napi)
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_alloc_frag(q->buf_size);
if (!buf)
break;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index a103b77ae8c4..a892f59a32d8 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -276,7 +276,6 @@ static int
mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
int nsgs, int len, int sglen)
{
- struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
struct urb *urb = buf->urb;
int i;
@@ -285,7 +284,7 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
void *data;
int offset;
- data = page_frag_alloc(&q->rx_page, len, GFP_ATOMIC);
+ data = mt76_alloc_frag(len);
if (!data)
break;
@@ -557,13 +556,6 @@ static void mt76u_free_rx(struct mt76_dev *dev)
for (i = 0; i < q->ndesc; i++)
mt76u_buf_free(&q->entry[i].ubuf);
-
- if (!q->rx_page.va)
- return;
-
- page = virt_to_page(q->rx_page.va);
- __page_frag_cache_drain(page, q->rx_page.pagecnt_bias);
- memset(&q->rx_page, 0, sizeof(q->rx_page));
}
static void mt76u_stop_rx(struct mt76_dev *dev)
diff --git a/drivers/net/wireless/mediatek/mt76/util.c b/drivers/net/wireless/mediatek/mt76/util.c
index 0c35b8db58cd..def2a1b841b9 100644
--- a/drivers/net/wireless/mediatek/mt76/util.c
+++ b/drivers/net/wireless/mediatek/mt76/util.c
@@ -75,4 +75,20 @@ int mt76_wcid_alloc(unsigned long *mask, int size)
}
EXPORT_SYMBOL_GPL(mt76_wcid_alloc);
+static DEFINE_PER_CPU(struct page_frag_cache, mt76_frag_cache);
+
+void *mt76_alloc_frag(unsigned int fragsz)
+{
+ struct page_frag_cache *fc;
+ unsigned long flags;
+ void *data;
+
+ local_irq_save(flags);
+ fc = this_cpu_ptr(&mt76_frag_cache);
+ data = page_frag_alloc(fc, fragsz, GFP_ATOMIC);
+ local_irq_restore(flags);
+ return data;
+}
+EXPORT_SYMBOL_GPL(mt76_alloc_frag);
+
MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/wireless/mediatek/mt76/util.h b/drivers/net/wireless/mediatek/mt76/util.h
index 018d475504a2..6cb6c0e993c4 100644
--- a/drivers/net/wireless/mediatek/mt76/util.h
+++ b/drivers/net/wireless/mediatek/mt76/util.h
@@ -41,4 +41,5 @@ mt76_skb_set_moredata(struct sk_buff *skb, bool enable)
hdr->frame_control &= ~cpu_to_le16(IEEE80211_FCTL_MOREDATA);
}
+void *mt76_alloc_frag(unsigned int fragsz);
#endif
--
2.7.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] mt76: make frag_cache global per cpu structure
2018-10-03 9:17 ` [RFC 2/2] mt76: make frag_cache global per cpu structure Stanislaw Gruszka
@ 2018-10-03 9:38 ` Lorenzo Bianconi
2018-10-03 10:22 ` Stanislaw Gruszka
2018-10-03 10:50 ` Felix Fietkau
1 sibling, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2018-10-03 9:38 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless
> +static DEFINE_PER_CPU(struct page_frag_cache, mt76_frag_cache);
> +
> +void *mt76_alloc_frag(unsigned int fragsz)
> +{
> + struct page_frag_cache *fc;
> + unsigned long flags;
> + void *data;
> +
> + local_irq_save(flags);
I like this approach since we will avoid a cache miss for the spinlock :)
Do we still need to disable local_irq here since (not considering fw upload)
I guess there is no contention for mt76_frag_cache
Regards,
Lorenzo
> + fc = this_cpu_ptr(&mt76_frag_cache);
> + data = page_frag_alloc(fc, fragsz, GFP_ATOMIC);
> + local_irq_restore(flags);
> + return data;
> +}
> +EXPORT_SYMBOL_GPL(mt76_alloc_frag);
> +
> MODULE_LICENSE("Dual BSD/GPL");
> diff --git a/drivers/net/wireless/mediatek/mt76/util.h b/drivers/net/wireless/mediatek/mt76/util.h
> index 018d475504a2..6cb6c0e993c4 100644
> --- a/drivers/net/wireless/mediatek/mt76/util.h
> +++ b/drivers/net/wireless/mediatek/mt76/util.h
> @@ -41,4 +41,5 @@ mt76_skb_set_moredata(struct sk_buff *skb, bool enable)
> hdr->frame_control &= ~cpu_to_le16(IEEE80211_FCTL_MOREDATA);
> }
>
> +void *mt76_alloc_frag(unsigned int fragsz);
> #endif
> --
> 2.7.5
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] mt76: make frag_cache global per cpu structure
2018-10-03 9:38 ` Lorenzo Bianconi
@ 2018-10-03 10:22 ` Stanislaw Gruszka
0 siblings, 0 replies; 5+ messages in thread
From: Stanislaw Gruszka @ 2018-10-03 10:22 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: Felix Fietkau, linux-wireless
On Wed, Oct 03, 2018 at 11:38:54AM +0200, Lorenzo Bianconi wrote:
> > +static DEFINE_PER_CPU(struct page_frag_cache, mt76_frag_cache);
> > +
> > +void *mt76_alloc_frag(unsigned int fragsz)
> > +{
> > + struct page_frag_cache *fc;
> > + unsigned long flags;
> > + void *data;
> > +
> > + local_irq_save(flags);
>
> I like this approach since we will avoid a cache miss for the spinlock :)
> Do we still need to disable local_irq here since (not considering fw upload)
> I guess there is no contention for mt76_frag_cache
I think is needed if we have more than one device, but I think we can
change to local_irq_disable() / local_irq_enable() .
Thanks
Stanislaw
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] mt76: make frag_cache global per cpu structure
2018-10-03 9:17 ` [RFC 2/2] mt76: make frag_cache global per cpu structure Stanislaw Gruszka
2018-10-03 9:38 ` Lorenzo Bianconi
@ 2018-10-03 10:50 ` Felix Fietkau
1 sibling, 0 replies; 5+ messages in thread
From: Felix Fietkau @ 2018-10-03 10:50 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, linux-wireless
> On 3. Oct 2018, at 11:17, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
> Make mt76 frag cache similar to netdev frag cache. This should
> make frag allocation safe regarding concurrent access and also
> be more efficient since we will use pages that most likely are
> hot on particular cpu.
>
> And we don't need to clean up the cache up during device removal.
The problem with feeding multiple queues with buffers from the same fragment cache is the fact that this re-introduces the problem of large compound pages staying pinned in memory (often with only one or two fragments actually being used) far too long, leading to excessive RAM usage.
That is much more critical than the spinlock on alloc issue.
Please keep allocation the way it is now.
- Felix
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-03 10:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 9:17 [RFC 1/2] mt76: remove rx_page_lock Stanislaw Gruszka
2018-10-03 9:17 ` [RFC 2/2] mt76: make frag_cache global per cpu structure Stanislaw Gruszka
2018-10-03 9:38 ` Lorenzo Bianconi
2018-10-03 10:22 ` Stanislaw Gruszka
2018-10-03 10:50 ` Felix Fietkau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.