* [PATCH] wireguard/queueing: simplify wg_cpumask_next_online()
@ 2025-06-04 23:36 Yury Norov
2025-06-05 4:23 ` Yury Norov
0 siblings, 1 reply; 5+ messages in thread
From: Yury Norov @ 2025-06-04 23:36 UTC (permalink / raw)
To: Jason A. Donenfeld, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, wireguard, netdev, linux-kernel
Cc: Yury Norov
wg_cpumask_choose_online() opencodes cpumask_nth(). Use it and make the
function significantly simpler. While there, fix opencoded cpu_online()
too.
Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
drivers/net/wireguard/queueing.h | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
index 7eb76724b3ed..3bfe16f71af0 100644
--- a/drivers/net/wireguard/queueing.h
+++ b/drivers/net/wireguard/queueing.h
@@ -104,17 +104,11 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
{
- unsigned int cpu = *stored_cpu, cpu_index, i;
+ if (likely(*stored_cpu < nr_cpu_ids && cpu_online(*stored_cpu)))
+ return cpu;
- if (unlikely(cpu >= nr_cpu_ids ||
- !cpumask_test_cpu(cpu, cpu_online_mask))) {
- cpu_index = id % cpumask_weight(cpu_online_mask);
- cpu = cpumask_first(cpu_online_mask);
- for (i = 0; i < cpu_index; ++i)
- cpu = cpumask_next(cpu, cpu_online_mask);
- *stored_cpu = cpu;
- }
- return cpu;
+ *stored_cpu = cpumask_nth(id % num_online_cpus(), cpu_online_mask);
+ return *stored_cpu;
}
/* This function is racy, in the sense that it's called while last_cpu is
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wireguard/queueing: simplify wg_cpumask_next_online()
2025-06-04 23:36 [PATCH] wireguard/queueing: simplify wg_cpumask_next_online() Yury Norov
@ 2025-06-05 4:23 ` Yury Norov
2025-06-05 11:33 ` Jason A. Donenfeld
0 siblings, 1 reply; 5+ messages in thread
From: Yury Norov @ 2025-06-05 4:23 UTC (permalink / raw)
To: Jason A. Donenfeld, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, wireguard, netdev, linux-kernel
On Wed, Jun 04, 2025 at 07:36:55PM -0400, Yury Norov wrote:
> wg_cpumask_choose_online() opencodes cpumask_nth(). Use it and make the
> function significantly simpler. While there, fix opencoded cpu_online()
> too.
>
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
> drivers/net/wireguard/queueing.h | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
> index 7eb76724b3ed..3bfe16f71af0 100644
> --- a/drivers/net/wireguard/queueing.h
> +++ b/drivers/net/wireguard/queueing.h
> @@ -104,17 +104,11 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
>
> static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
> {
> - unsigned int cpu = *stored_cpu, cpu_index, i;
> + if (likely(*stored_cpu < nr_cpu_ids && cpu_online(*stored_cpu)))
> + return cpu;
Oops... This should be
return *stored_cpu;
I'll resend, sorry for noise.
>
> - if (unlikely(cpu >= nr_cpu_ids ||
> - !cpumask_test_cpu(cpu, cpu_online_mask))) {
> - cpu_index = id % cpumask_weight(cpu_online_mask);
> - cpu = cpumask_first(cpu_online_mask);
> - for (i = 0; i < cpu_index; ++i)
> - cpu = cpumask_next(cpu, cpu_online_mask);
> - *stored_cpu = cpu;
> - }
> - return cpu;
> + *stored_cpu = cpumask_nth(id % num_online_cpus(), cpu_online_mask);
> + return *stored_cpu;
> }
>
> /* This function is racy, in the sense that it's called while last_cpu is
> --
> 2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wireguard/queueing: simplify wg_cpumask_next_online()
2025-06-05 4:23 ` Yury Norov
@ 2025-06-05 11:33 ` Jason A. Donenfeld
2025-06-05 14:24 ` Yury Norov
0 siblings, 1 reply; 5+ messages in thread
From: Jason A. Donenfeld @ 2025-06-05 11:33 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, wireguard, netdev, linux-kernel
On Thu, Jun 05, 2025 at 12:23:29AM -0400, Yury Norov wrote:
> On Wed, Jun 04, 2025 at 07:36:55PM -0400, Yury Norov wrote:
> > wg_cpumask_choose_online() opencodes cpumask_nth(). Use it and make the
> > function significantly simpler. While there, fix opencoded cpu_online()
> > too.
> >
> > Signed-off-by: Yury Norov <yury.norov@gmail.com>
> > ---
> > drivers/net/wireguard/queueing.h | 14 ++++----------
> > 1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
> > index 7eb76724b3ed..3bfe16f71af0 100644
> > --- a/drivers/net/wireguard/queueing.h
> > +++ b/drivers/net/wireguard/queueing.h
> > @@ -104,17 +104,11 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
> >
> > static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
> > {
> > - unsigned int cpu = *stored_cpu, cpu_index, i;
> > + if (likely(*stored_cpu < nr_cpu_ids && cpu_online(*stored_cpu)))
> > + return cpu;
>
> Oops... This should be
> return *stored_cpu;
Maybe it's best to structure the function something like:
unsigned int cpu = *stored_cpu;
if (unlikely(cpu >= nr_cpu_ids || !cpu_online(cpu))) {
cpu = *stored_cpu = cpumask_nth(id % num_online_cpus(), cpu_online_mask);
return cpu;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wireguard/queueing: simplify wg_cpumask_next_online()
2025-06-05 11:33 ` Jason A. Donenfeld
@ 2025-06-05 14:24 ` Yury Norov
2025-06-05 15:47 ` Jason A. Donenfeld
0 siblings, 1 reply; 5+ messages in thread
From: Yury Norov @ 2025-06-05 14:24 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, wireguard, netdev, linux-kernel
On Thu, Jun 05, 2025 at 01:33:19PM +0200, Jason A. Donenfeld wrote:
> On Thu, Jun 05, 2025 at 12:23:29AM -0400, Yury Norov wrote:
> > On Wed, Jun 04, 2025 at 07:36:55PM -0400, Yury Norov wrote:
> > > wg_cpumask_choose_online() opencodes cpumask_nth(). Use it and make the
> > > function significantly simpler. While there, fix opencoded cpu_online()
> > > too.
> > >
> > > Signed-off-by: Yury Norov <yury.norov@gmail.com>
> > > ---
> > > drivers/net/wireguard/queueing.h | 14 ++++----------
> > > 1 file changed, 4 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
> > > index 7eb76724b3ed..3bfe16f71af0 100644
> > > --- a/drivers/net/wireguard/queueing.h
> > > +++ b/drivers/net/wireguard/queueing.h
> > > @@ -104,17 +104,11 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
> > >
> > > static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
> > > {
> > > - unsigned int cpu = *stored_cpu, cpu_index, i;
> > > + if (likely(*stored_cpu < nr_cpu_ids && cpu_online(*stored_cpu)))
> > > + return cpu;
> >
> > Oops... This should be
> > return *stored_cpu;
>
> Maybe it's best to structure the function something like:
>
> unsigned int cpu = *stored_cpu;
> if (unlikely(cpu >= nr_cpu_ids || !cpu_online(cpu))) {
> cpu = *stored_cpu = cpumask_nth(id % num_online_cpus(), cpu_online_mask);
> return cpu;
If you prefer. I'll send v2 shortly
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wireguard/queueing: simplify wg_cpumask_next_online()
2025-06-05 14:24 ` Yury Norov
@ 2025-06-05 15:47 ` Jason A. Donenfeld
0 siblings, 0 replies; 5+ messages in thread
From: Jason A. Donenfeld @ 2025-06-05 15:47 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, wireguard, netdev, linux-kernel
On Thu, Jun 05, 2025 at 10:24:32AM -0400, Yury Norov wrote:
> On Thu, Jun 05, 2025 at 01:33:19PM +0200, Jason A. Donenfeld wrote:
> > On Thu, Jun 05, 2025 at 12:23:29AM -0400, Yury Norov wrote:
> > > On Wed, Jun 04, 2025 at 07:36:55PM -0400, Yury Norov wrote:
> > > > wg_cpumask_choose_online() opencodes cpumask_nth(). Use it and make the
> > > > function significantly simpler. While there, fix opencoded cpu_online()
> > > > too.
> > > >
> > > > Signed-off-by: Yury Norov <yury.norov@gmail.com>
> > > > ---
> > > > drivers/net/wireguard/queueing.h | 14 ++++----------
> > > > 1 file changed, 4 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
> > > > index 7eb76724b3ed..3bfe16f71af0 100644
> > > > --- a/drivers/net/wireguard/queueing.h
> > > > +++ b/drivers/net/wireguard/queueing.h
> > > > @@ -104,17 +104,11 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating)
> > > >
> > > > static inline int wg_cpumask_choose_online(int *stored_cpu, unsigned int id)
> > > > {
> > > > - unsigned int cpu = *stored_cpu, cpu_index, i;
> > > > + if (likely(*stored_cpu < nr_cpu_ids && cpu_online(*stored_cpu)))
> > > > + return cpu;
> > >
> > > Oops... This should be
> > > return *stored_cpu;
> >
> > Maybe it's best to structure the function something like:
> >
> > unsigned int cpu = *stored_cpu;
> > if (unlikely(cpu >= nr_cpu_ids || !cpu_online(cpu))) {
> > cpu = *stored_cpu = cpumask_nth(id % num_online_cpus(), cpu_online_mask);
> > return cpu;
>
> If you prefer. I'll send v2 shortly
While you're at it, fix the commit subject to match the format used by
every single other wireguard commit. `$ git log --oneline drivers/net/wireguard`
to see what I mean.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-05 15:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-04 23:36 [PATCH] wireguard/queueing: simplify wg_cpumask_next_online() Yury Norov
2025-06-05 4:23 ` Yury Norov
2025-06-05 11:33 ` Jason A. Donenfeld
2025-06-05 14:24 ` Yury Norov
2025-06-05 15:47 ` Jason A. Donenfeld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).