| Message ID | 20240911112846.285033-1-webmaster@jbsky.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] mvneta: fix "napi poll" infinite loop | expand |
Hello Julien, On Wed, 11 Sep 2024 13:28:46 +0200 Julien Blais <webmaster@jbsky.fr> wrote: > In percpu mode, when there's a network load, one of the cpus can be > solicited without having anything to process. > If 0 is returned to napi poll, napi will ignore the next requests, > causing an infinite loop with ISR handling. > > Without this change, patches hang around fixing the queue at 0 and > the interrupt remains stuck on the 1st CPU. > The percpu conf is useless in this case, so we might as well remove it. > > Signed-off-by: Julien Blais <webmaster@jbsky.fr> If this patch is a fix, you must also include a "Fixes" tag stating which commit you are fixing. You must also include in the subject whether you are targetting the "net-next" tree or "net". In your case, this would be the "net" tree, that gathers the bugfixes. > --- > drivers/net/ethernet/marvell/mvneta.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c > index 3f124268b..b6e89b888 100644 > --- a/drivers/net/ethernet/marvell/mvneta.c > +++ b/drivers/net/ethernet/marvell/mvneta.c > @@ -3186,7 +3186,10 @@ static int mvneta_poll(struct napi_struct *napi, int budget) > > if (rx_done < budget) { > cause_rx_tx = 0; > - napi_complete_done(napi, rx_done); > + if (rx_done) > + napi_complete_done(napi, rx_done); > + else > + napi_complete(napi); I don't quite get this patch. as napi_complete is just calling napi_complete_done(napi, 0), so this is basically if (rx_done != 0) napi_complete_done(napi, rx_done); else napi_complete_done(napi, 0); So, nothing actually changes. Can you elaborate more on the issue you are facing ? Thanks, Maxime
On Wed, 11 Sep 2024 13:28:46 +0200 Julien Blais wrote: > This e-mail and any attached files are confidential and may be legally privileged. If you are not the addressee, any disclosure, reproduction, copying, distribution, or other dissemination or use of this communication is strictly prohibited. If you have received this transmission in error please notify the sender immediately and then delete this mail. > E-mail transmission cannot be guaranteed to be secure or error free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message which arise as a result of e-mail transmission or changes to transmitted date not specifically approved by the sender. > If this e-mail or attached files contain information which do not relate to our professional activity we do not accept liability for such information. In addition to answering Maxime's question / reworking the patch you'll need to figure out a way to post the code without this disclaimer.
Hello, Maxime, you've enlightened me, yes, what I've committed is useless as it's dealt with further on. Anyway, I can't reproduce it, I was on the wrong track. Yes, for my next commit I'll test the mail so as not to have this “disclamer” anymore. Thanks for your feedback! All the best, Julien Blais -- This e-mail and any attached files are confidential and may be legally privileged. If you are not the addressee, any disclosure, reproduction, copying, distribution, or other dissemination or use of this communication is strictly prohibited. If you have received this transmission in error please notify the sender immediately and then delete this mail. E-mail transmission cannot be guaranteed to be secure or error free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message which arise as a result of e-mail transmission or changes to transmitted date not specifically approved by the sender. If this e-mail or attached files contain information which do not relate to our professional activity we do not accept liability for such information.
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 3f124268b..b6e89b888 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -3186,7 +3186,10 @@ static int mvneta_poll(struct napi_struct *napi, int budget) if (rx_done < budget) { cause_rx_tx = 0; - napi_complete_done(napi, rx_done); + if (rx_done) + napi_complete_done(napi, rx_done); + else + napi_complete(napi); if (pp->neta_armada3700) { unsigned long flags;
In percpu mode, when there's a network load, one of the cpus can be solicited without having anything to process. If 0 is returned to napi poll, napi will ignore the next requests, causing an infinite loop with ISR handling. Without this change, patches hang around fixing the queue at 0 and the interrupt remains stuck on the 1st CPU. The percpu conf is useless in this case, so we might as well remove it. Signed-off-by: Julien Blais <webmaster@jbsky.fr> --- drivers/net/ethernet/marvell/mvneta.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)