[PATCH] net: Destroy previously created kthreads after failing to set napi threaded mode

[PATCH] net: Destroy previously created kthreads after failing to set napi threaded mode

[Zqiang]

When setting 1 to enable napi threaded mode, will traverse dev->napi_list
and create kthread for napi->thread, if creation fails, the dev->threaded
will be set to false and we will clear NAPI_STATE_THREADED bit for all
napi->state in dev->napi_list, even if some napi that has successfully
created the kthread before. as a result, for successfully created napi
kthread, they will never be used.

This commit therefore destroy previously created napi->thread if setting
napi threaded mode fails.

Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
---
 net/core/dev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 69a3e544676c..9929f0567150 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6317,10 +6317,13 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
 	 * This should not cause hiccups/stalls to the live traffic.
 	 */
 	list_for_each_entry(napi, &dev->napi_list, dev_list) {
-		if (threaded)
+		if (threaded) {
 			set_bit(NAPI_STATE_THREADED, &napi->state);
-		else
+		} else {
 			clear_bit(NAPI_STATE_THREADED, &napi->state);
+			if (napi->thread)
+				kthread_stop(napi->thread);
+		}
 	}
 
 	return err;
-- 
2.17.1

Re: [PATCH] net: Destroy previously created kthreads after failing to set napi threaded mode

[Eric Dumazet]

On Fri, Jun 30, 2023 at 5:27 AM Zqiang <qiang.zhang1211@gmail.com> wrote:
>
> When setting 1 to enable napi threaded mode, will traverse dev->napi_list
> and create kthread for napi->thread, if creation fails, the dev->threaded
> will be set to false and we will clear NAPI_STATE_THREADED bit for all
> napi->state in dev->napi_list, even if some napi that has successfully
> created the kthread before. as a result, for successfully created napi
> kthread, they will never be used.
>
> This commit therefore destroy previously created napi->thread if setting
> napi threaded mode fails.
>

I am not sure we need this, because these kthreads are not leaked at
present time.

pktgen also creates unused kthreads (one per cpu), even if in most
cases only one of them is used.

Leaving kthreads makes it possible to eventually succeed to enable
napi threaded mode
after several tries, for devices with 64 or more queues...

This would target net-next.

If you claim to fix a bug (thus targeting net tree), we would need a Fixes: tag.

Thanks.

Re: [PATCH] net: Destroy previously created kthreads after failing to set napi threaded mode

[Z qiang]

On Fri, Jun 30, 2023 at 1:33 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Jun 30, 2023 at 5:27 AM Zqiang <qiang.zhang1211@gmail.com> wrote:
> >
> > When setting 1 to enable napi threaded mode, will traverse dev->napi_list
> > and create kthread for napi->thread, if creation fails, the dev->threaded
> > will be set to false and we will clear NAPI_STATE_THREADED bit for all
> > napi->state in dev->napi_list, even if some napi that has successfully
> > created the kthread before. as a result, for successfully created napi
> > kthread, they will never be used.
> >
> > This commit therefore destroy previously created napi->thread if setting
> > napi threaded mode fails.
> >
>
> I am not sure we need this, because these kthreads are not leaked at
> present time.
>
> pktgen also creates unused kthreads (one per cpu), even if in most
> cases only one of them is used.
>
> Leaving kthreads makes it possible to eventually succeed to enable
> napi threaded mode
> after several tries, for devices with 64 or more queues...
>

Thanks for the reply,   I understand the approach in this way.
But for successfully created napi kthreads, should NAPI_STATE_THREADED
bits not be cleared ?

Thanks
Zqiang

>
> This would target net-next.
>
> If you claim to fix a bug (thus targeting net tree), we would need a Fixes: tag.
>
>
> Thanks.