[PATCH] mt76: usb: grab rx_page_lock for legacy mode in mt76u_refill_rx

[PATCH] mt76: usb: grab rx_page_lock for legacy mode in mt76u_refill_rx

[Lorenzo Bianconi]

Grab rx_page_lock in mt76u_refill_rx allocating rx buffer for 'legacy'
mode. Remove gfp from mt76u_refill_rx signature since we need to
use GFP_ATOMIC

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/usb.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index ae6ada370597..7ce3c7f9d5d8 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -325,15 +325,17 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
 
 static int
 mt76u_refill_rx(struct mt76_dev *dev, struct mt76_queue *q,
-		struct mt76u_buf *buf, int nsgs, gfp_t gfp)
+		struct mt76u_buf *buf, int nsgs)
 {
-	if (dev->usb.sg_en) {
+	if (dev->usb.sg_en)
 		return mt76u_fill_rx_sg(dev, buf, nsgs, q->buf_size,
 					SKB_WITH_OVERHEAD(q->buf_size));
-	} else {
-		buf->buf = page_frag_alloc(&q->rx_page, q->buf_size, gfp);
-		return buf->buf ? 0 : -ENOMEM;
-	}
+
+	spin_lock_bh(&q->rx_page_lock);
+	buf->buf = page_frag_alloc(&q->rx_page, q->buf_size, GFP_ATOMIC);
+	spin_unlock_bh(&q->rx_page_lock);
+
+	return buf->buf ? 0 : -ENOMEM;
 }
 
 static int
@@ -358,7 +360,7 @@ mt76u_buf_alloc(struct mt76_dev *dev, struct mt76u_buf *buf)
 		sg_init_table(buf->urb->sg, MT_SG_MAX_SIZE);
 	}
 
-	return mt76u_refill_rx(dev, q, buf, MT_SG_MAX_SIZE, GFP_KERNEL);
+	return mt76u_refill_rx(dev, q, buf, MT_SG_MAX_SIZE);
 }
 
 static void mt76u_buf_free(struct mt76u_buf *buf)
@@ -529,8 +531,7 @@ static void mt76u_rx_tasklet(unsigned long data)
 
 		count = mt76u_process_rx_entry(dev, buf);
 		if (count > 0) {
-			err = mt76u_refill_rx(dev, q, buf, count,
-					      GFP_ATOMIC);
+			err = mt76u_refill_rx(dev, q, buf, count);
 			if (err < 0)
 				break;
 		}
-- 
2.20.1

Re: [PATCH] mt76: usb: grab rx_page_lock for legacy mode in mt76u_refill_rx

[Stanislaw Gruszka]

On Thu, Feb 28, 2019 at 12:08:45PM +0100, Lorenzo Bianconi wrote:
> Grab rx_page_lock in mt76u_refill_rx allocating rx buffer for 'legacy'
> mode. Remove gfp from mt76u_refill_rx signature since we need to
> use GFP_ATOMIC
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

I'm doing some cleanup/optimization work which amongs other things
will remove rx_page_lock. Please drop this patch.

Stanislaw

Re: [PATCH] mt76: usb: grab rx_page_lock for legacy mode in mt76u_refill_rx

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

> On Thu, Feb 28, 2019 at 12:08:45PM +0100, Lorenzo Bianconi wrote:
> > Grab rx_page_lock in mt76u_refill_rx allocating rx buffer for 'legacy'
> > mode. Remove gfp from mt76u_refill_rx signature since we need to
> > use GFP_ATOMIC
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> I'm doing some cleanup/optimization work which amongs other things
> will remove rx_page_lock. Please drop this patch.

Oh, ok. IIRC we need rx_page_lock to fix this:
481bb0432414f790066205fe77226b7d1877385d. Is it still necessary?
Moreover maybe we need this patch for 5.1 since net-next is almost close, no?

@Felix: are you planning to submit other patches for 5.1?

Regards,
Lorenzo

> 
> Stanislaw

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] mt76: usb: grab rx_page_lock for legacy mode in mt76u_refill_rx

[Stanislaw Gruszka]

On Thu, Feb 28, 2019 at 12:37:48PM +0100, Lorenzo Bianconi wrote:
> > On Thu, Feb 28, 2019 at 12:08:45PM +0100, Lorenzo Bianconi wrote:
> > > Grab rx_page_lock in mt76u_refill_rx allocating rx buffer for 'legacy'
> > > mode. Remove gfp from mt76u_refill_rx signature since we need to
> > > use GFP_ATOMIC
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > 
> > I'm doing some cleanup/optimization work which amongs other things
> > will remove rx_page_lock. Please drop this patch.
> 
> Oh, ok. IIRC we need rx_page_lock to fix this:
> 481bb0432414f790066205fe77226b7d1877385d. Is it still necessary?
> Moreover maybe we need this patch for 5.1 since net-next is almost close, no?

It is not neccessery now in -next after removing concurrent access to
mt76u_buf_alloc(), can be removed.

Stanislaw