All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Alexander H Duyck <alexander.duyck@gmail.com>,
	netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
	pabeni@redhat.com, lorenzo.bianconi@redhat.com, nbd@nbd.name,
	john@phrozen.org, sean.wang@mediatek.com,
	Mark-MC.Lee@mediatek.com, sujuan.chen@mediatek.com,
	daniel@makrotopia.org
Subject: Re: [PATCH net-next] net: ethernet: mtk_wed: get rid of queue lock for rx queue
Date: Mon, 9 Jan 2023 19:37:21 -0800	[thread overview]
Message-ID: <20230109193721.7d05d24b@kernel.org> (raw)
In-Reply-To: <be4814483f1b320eaaa49ba8d59d81b2a51f932b.camel@gmail.com>

On Mon, 09 Jan 2023 16:50:55 -0800 Alexander H Duyck wrote:
> On Sat, 2023-01-07 at 15:41 +0100, Lorenzo Bianconi wrote:
> > mtk_wed_wo_queue_rx_clean and mtk_wed_wo_queue_refill routines can't run
> > concurrently so get rid of spinlock for rx queues.

You say "for rx queues" but mtk_wed_wo_queue_refill() is also called
for tx queues.

> My understanding is that mtk_wed_wo_queue_refill will only be called
> during init and by the tasklet. The mtk_wed_wo_queue_rx_clean function
> is only called during deinit and only after the tasklet has been
> disabled. That is the reason they cannot run at the same time correct?
> 
> It would be nice if you explained why they couldn't run concurrently
> rather than just stating it is so in the patch description. It makes it
> easier to verify assumptions that way. Otherwise the patch itself looks
> good to me.

Agreed, please respin with a better commit message.

  reply	other threads:[~2023-01-10  3:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-07 14:41 [PATCH net-next] net: ethernet: mtk_wed: get rid of queue lock for rx queue Lorenzo Bianconi
2023-01-10  0:50 ` Alexander H Duyck
2023-01-10  3:37   ` Jakub Kicinski [this message]
2023-01-10  9:10     ` Lorenzo Bianconi
2023-01-10  9:09   ` Lorenzo Bianconi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230109193721.7d05d24b@kernel.org \
    --to=kuba@kernel.org \
    --cc=Mark-MC.Lee@mediatek.com \
    --cc=alexander.duyck@gmail.com \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john@phrozen.org \
    --cc=lorenzo.bianconi@redhat.com \
    --cc=lorenzo@kernel.org \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sean.wang@mediatek.com \
    --cc=sujuan.chen@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.