linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
@ 2022-10-04  6:25 Leonardo Bras
  2022-10-05 12:57 ` Marcelo Tosatti
  0 siblings, 1 reply; 6+ messages in thread
From: Leonardo Bras @ 2022-10-04  6:25 UTC (permalink / raw)
  To: Steffen Klassert, Herbert Xu, David S. Miller, mtosatti
  Cc: Leonardo Bras, linux-crypto, linux-kernel

Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
from any online cpus. Later padata_reorder() will queue_work_on() the
chosen cb_cpu.

This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
isolcpus=... kernel parameter), since the work queued will interfere with
the workload on the isolated cpu.

Make sure isolated cpus are not used for pcrypt.

Signed-off-by: Leonardo Bras <leobras@redhat.com>
---
 crypto/pcrypt.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 9d10b846ccf73..9017d08c91a8d 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -16,6 +16,7 @@
 #include <linux/kobject.h>
 #include <linux/cpu.h>
 #include <crypto/pcrypt.h>
+#include <linux/sched/isolation.h>
 
 static struct padata_instance *pencrypt;
 static struct padata_instance *pdecrypt;
@@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
 	struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
 	struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
 	struct crypto_aead *cipher;
+	struct cpumask non_isolated;
+
+	cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));
 
 	cpu_index = (unsigned int)atomic_inc_return(&ictx->tfm_count) %
-		    cpumask_weight(cpu_online_mask);
+		    cpumask_weight(&non_isolated);
 
-	ctx->cb_cpu = cpumask_first(cpu_online_mask);
+	ctx->cb_cpu = cpumask_first(&non_isolated);
 	for (cpu = 0; cpu < cpu_index; cpu++)
-		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, cpu_online_mask);
+		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, &non_isolated);
 
 	cipher = crypto_spawn_aead(&ictx->spawn);
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
  2022-10-04  6:25 [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback Leonardo Bras
@ 2022-10-05 12:57 ` Marcelo Tosatti
  2022-10-07 21:42   ` Leonardo Brás
  0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2022-10-05 12:57 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, linux-crypto,
	linux-kernel

On Tue, Oct 04, 2022 at 03:25:37AM -0300, Leonardo Bras wrote:
> Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
> from any online cpus. Later padata_reorder() will queue_work_on() the
> chosen cb_cpu.
> 
> This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
> isolcpus=... kernel parameter), since the work queued will interfere with
> the workload on the isolated cpu.
> 
> Make sure isolated cpus are not used for pcrypt.
> 
> Signed-off-by: Leonardo Bras <leobras@redhat.com>
> ---
>  crypto/pcrypt.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> index 9d10b846ccf73..9017d08c91a8d 100644
> --- a/crypto/pcrypt.c
> +++ b/crypto/pcrypt.c
> @@ -16,6 +16,7 @@
>  #include <linux/kobject.h>
>  #include <linux/cpu.h>
>  #include <crypto/pcrypt.h>
> +#include <linux/sched/isolation.h>
>  
>  static struct padata_instance *pencrypt;
>  static struct padata_instance *pdecrypt;
> @@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
>  	struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
>  	struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
>  	struct crypto_aead *cipher;
> +	struct cpumask non_isolated;
> +
> +	cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));

Since certain systems do not use isolcpus=domain, so please use a flag
that is setup by nohz_full=, for example HK_FLAG_MISC:

static int __init housekeeping_nohz_full_setup(char *str)
{
        unsigned long flags;

        flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
                HK_FLAG_MISC | HK_FLAG_KTHREAD;

        return housekeeping_setup(str, flags);
}
__setup("nohz_full=", housekeeping_nohz_full_setup);

Also, shouldnt you use cpumask_t ?

Looks good otherwise.

Thanks!


>  
>  	cpu_index = (unsigned int)atomic_inc_return(&ictx->tfm_count) %
> -		    cpumask_weight(cpu_online_mask);
> +		    cpumask_weight(&non_isolated);
>  
> -	ctx->cb_cpu = cpumask_first(cpu_online_mask);
> +	ctx->cb_cpu = cpumask_first(&non_isolated);
>  	for (cpu = 0; cpu < cpu_index; cpu++)
> -		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, cpu_online_mask);
> +		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, &non_isolated);
>  
>  	cipher = crypto_spawn_aead(&ictx->spawn);
>  
> -- 
> 2.37.3
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
  2022-10-05 12:57 ` Marcelo Tosatti
@ 2022-10-07 21:42   ` Leonardo Brás
  2022-10-11 18:20     ` Leonardo Brás
  0 siblings, 1 reply; 6+ messages in thread
From: Leonardo Brás @ 2022-10-07 21:42 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, linux-crypto,
	linux-kernel

On Wed, 2022-10-05 at 09:57 -0300, Marcelo Tosatti wrote:
> On Tue, Oct 04, 2022 at 03:25:37AM -0300, Leonardo Bras wrote:
> > Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
> > from any online cpus. Later padata_reorder() will queue_work_on() the
> > chosen cb_cpu.
> > 
> > This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
> > isolcpus=... kernel parameter), since the work queued will interfere with
> > the workload on the isolated cpu.
> > 
> > Make sure isolated cpus are not used for pcrypt.
> > 
> > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > ---
> >  crypto/pcrypt.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> > index 9d10b846ccf73..9017d08c91a8d 100644
> > --- a/crypto/pcrypt.c
> > +++ b/crypto/pcrypt.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/kobject.h>
> >  #include <linux/cpu.h>
> >  #include <crypto/pcrypt.h>
> > +#include <linux/sched/isolation.h>
> >  
> >  static struct padata_instance *pencrypt;
> >  static struct padata_instance *pdecrypt;
> > @@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
> >  	struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
> >  	struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
> >  	struct crypto_aead *cipher;
> > +	struct cpumask non_isolated;
> > +
> > +	cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));
> 
> Since certain systems do not use isolcpus=domain, so please use a flag
> that is setup by nohz_full=, for example HK_FLAG_MISC:
> 
> static int __init housekeeping_nohz_full_setup(char *str)
> {
>         unsigned long flags;
> 
>         flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
>                 HK_FLAG_MISC | HK_FLAG_KTHREAD;
> 
>         return housekeeping_setup(str, flags);
> }
> __setup("nohz_full=", housekeeping_nohz_full_setup);

Oh, sure. 
Since we are talking about WorkQueues, I think it makes sense to pick
HK_FLAG_WQ. 

> 
> Also, shouldnt you use cpumask_t ?/

Yeah, I think so. 
I was quick to choose the 'struct cpumask' because all functions would operate
in this variable type, but yeah, I think it makes sense to have this variable
being opaque here.

> 
> Looks good otherwise.
> 
> Thanks!

Thank you for reviewing! 

Leo

> 
> 
> >  
> >  	cpu_index = (unsigned int)atomic_inc_return(&ictx->tfm_count) %
> > -		    cpumask_weight(cpu_online_mask);
> > +		    cpumask_weight(&non_isolated);
> >  
> > -	ctx->cb_cpu = cpumask_first(cpu_online_mask);
> > +	ctx->cb_cpu = cpumask_first(&non_isolated);
> >  	for (cpu = 0; cpu < cpu_index; cpu++)
> > -		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, cpu_online_mask);
> > +		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, &non_isolated);
> >  
> >  	cipher = crypto_spawn_aead(&ictx->spawn);
> >  
> > -- 
> > 2.37.3
> > 
> > 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
  2022-10-07 21:42   ` Leonardo Brás
@ 2022-10-11 18:20     ` Leonardo Brás
  2022-11-01 17:35       ` Marcelo Tosatti
  0 siblings, 1 reply; 6+ messages in thread
From: Leonardo Brás @ 2022-10-11 18:20 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, linux-crypto,
	linux-kernel

On Fri, 2022-10-07 at 18:42 -0300, Leonardo Brás wrote:
> On Wed, 2022-10-05 at 09:57 -0300, Marcelo Tosatti wrote:
> > On Tue, Oct 04, 2022 at 03:25:37AM -0300, Leonardo Bras wrote:
> > > Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
> > > from any online cpus. Later padata_reorder() will queue_work_on() the
> > > chosen cb_cpu.
> > > 
> > > This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
> > > isolcpus=... kernel parameter), since the work queued will interfere with
> > > the workload on the isolated cpu.
> > > 
> > > Make sure isolated cpus are not used for pcrypt.
> > > 
> > > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > > ---
> > >  crypto/pcrypt.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> > > index 9d10b846ccf73..9017d08c91a8d 100644
> > > --- a/crypto/pcrypt.c
> > > +++ b/crypto/pcrypt.c
> > > @@ -16,6 +16,7 @@
> > >  #include <linux/kobject.h>
> > >  #include <linux/cpu.h>
> > >  #include <crypto/pcrypt.h>
> > > +#include <linux/sched/isolation.h>
> > >  
> > >  static struct padata_instance *pencrypt;
> > >  static struct padata_instance *pdecrypt;
> > > @@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
> > >  	struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
> > >  	struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
> > >  	struct crypto_aead *cipher;
> > > +	struct cpumask non_isolated;
> > > +
> > > +	cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));
> > 
> > Since certain systems do not use isolcpus=domain, so please use a flag
> > that is setup by nohz_full=, for example HK_FLAG_MISC:
> > 
> > static int __init housekeeping_nohz_full_setup(char *str)
> > {
> >         unsigned long flags;
> > 
> >         flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> >                 HK_FLAG_MISC | HK_FLAG_KTHREAD;
> > 
> >         return housekeeping_setup(str, flags);
> > }
> > __setup("nohz_full=", housekeeping_nohz_full_setup);
> 
> Oh, sure. 
> Since we are talking about WorkQueues, I think it makes sense to pick
> HK_FLAG_WQ. 
> 
> > 
> > Also, shouldnt you use cpumask_t ?/
> 
> Yeah, I think so. 
> I was quick to choose the 'struct cpumask' because all functions would operate
> in this variable type, but yeah, I think it makes sense to have this variable
> being opaque here.

In fact, it seems neither 'struct cpumask' nor 'cpumask_t' are recommended to be
used allocated in the stack, due to the large size it can get (up to 1kB). 

At include/linux/cpumask.h we have:
'cpumask_var_t: struct cpumask for stack usage'
which should work better at least for init functions like this.

In other cases, I see 'static cpumask_t' being used to avoid the allocation
overhead, but it's probably due to the functions being called in very specific
scenarios. It could mean trouble if multiple cpus try to use it at once.

What do you recommend on it?

Best regards,
Leo

> 
> > 
> > Looks good otherwise.
> > 
> > Thanks!
> 
> Thank you for reviewing! 
> 
> Leo
> 
> > 
> > 
> > >  
> > >  	cpu_index = (unsigned int)atomic_inc_return(&ictx->tfm_count) %
> > > -		    cpumask_weight(cpu_online_mask);
> > > +		    cpumask_weight(&non_isolated);
> > >  
> > > -	ctx->cb_cpu = cpumask_first(cpu_online_mask);
> > > +	ctx->cb_cpu = cpumask_first(&non_isolated);
> > >  	for (cpu = 0; cpu < cpu_index; cpu++)
> > > -		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, cpu_online_mask);
> > > +		ctx->cb_cpu = cpumask_next(ctx->cb_cpu, &non_isolated);
> > >  
> > >  	cipher = crypto_spawn_aead(&ictx->spawn);
> > >  
> > > -- 
> > > 2.37.3
> > > 
> > > 
> > 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
  2022-10-11 18:20     ` Leonardo Brás
@ 2022-11-01 17:35       ` Marcelo Tosatti
  2022-11-02  2:09         ` Leonardo Bras Soares Passos
  0 siblings, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2022-11-01 17:35 UTC (permalink / raw)
  To: Leonardo Brás
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, linux-crypto,
	linux-kernel

On Tue, Oct 11, 2022 at 03:20:39PM -0300, Leonardo Brás wrote:
> On Fri, 2022-10-07 at 18:42 -0300, Leonardo Brás wrote:
> > On Wed, 2022-10-05 at 09:57 -0300, Marcelo Tosatti wrote:
> > > On Tue, Oct 04, 2022 at 03:25:37AM -0300, Leonardo Bras wrote:
> > > > Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
> > > > from any online cpus. Later padata_reorder() will queue_work_on() the
> > > > chosen cb_cpu.
> > > > 
> > > > This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
> > > > isolcpus=... kernel parameter), since the work queued will interfere with
> > > > the workload on the isolated cpu.
> > > > 
> > > > Make sure isolated cpus are not used for pcrypt.
> > > > 
> > > > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > > > ---
> > > >  crypto/pcrypt.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> > > > index 9d10b846ccf73..9017d08c91a8d 100644
> > > > --- a/crypto/pcrypt.c
> > > > +++ b/crypto/pcrypt.c
> > > > @@ -16,6 +16,7 @@
> > > >  #include <linux/kobject.h>
> > > >  #include <linux/cpu.h>
> > > >  #include <crypto/pcrypt.h>
> > > > +#include <linux/sched/isolation.h>
> > > >  
> > > >  static struct padata_instance *pencrypt;
> > > >  static struct padata_instance *pdecrypt;
> > > > @@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
> > > >  	struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
> > > >  	struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
> > > >  	struct crypto_aead *cipher;
> > > > +	struct cpumask non_isolated;
> > > > +
> > > > +	cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));
> > > 
> > > Since certain systems do not use isolcpus=domain, so please use a flag
> > > that is setup by nohz_full=, for example HK_FLAG_MISC:
> > > 
> > > static int __init housekeeping_nohz_full_setup(char *str)
> > > {
> > >         unsigned long flags;
> > > 
> > >         flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> > >                 HK_FLAG_MISC | HK_FLAG_KTHREAD;
> > > 
> > >         return housekeeping_setup(str, flags);
> > > }
> > > __setup("nohz_full=", housekeeping_nohz_full_setup);
> > 
> > Oh, sure. 
> > Since we are talking about WorkQueues, I think it makes sense to pick
> > HK_FLAG_WQ. 
> > 
> > > 
> > > Also, shouldnt you use cpumask_t ?/
> > 
> > Yeah, I think so. 
> > I was quick to choose the 'struct cpumask' because all functions would operate
> > in this variable type, but yeah, I think it makes sense to have this variable
> > being opaque here.
> 
> In fact, it seems neither 'struct cpumask' nor 'cpumask_t' are recommended to be
> used allocated in the stack, due to the large size it can get (up to 1kB). 
> 
> At include/linux/cpumask.h we have:
> 'cpumask_var_t: struct cpumask for stack usage'
> which should work better at least for init functions like this.
> 
> In other cases, I see 'static cpumask_t' being used to avoid the allocation
> overhead, but it's probably due to the functions being called in very specific
> scenarios. It could mean trouble if multiple cpus try to use it at once.
> 
> What do you recommend on it?

Sorry for the delay. I suppose allocating and freeing is OK in this context, since
its initialization time and not a hot path?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback
  2022-11-01 17:35       ` Marcelo Tosatti
@ 2022-11-02  2:09         ` Leonardo Bras Soares Passos
  0 siblings, 0 replies; 6+ messages in thread
From: Leonardo Bras Soares Passos @ 2022-11-02  2:09 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, linux-crypto,
	linux-kernel

On Tue, Nov 1, 2022 at 2:42 PM Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> On Tue, Oct 11, 2022 at 03:20:39PM -0300, Leonardo Brás wrote:
> > On Fri, 2022-10-07 at 18:42 -0300, Leonardo Brás wrote:
> > > On Wed, 2022-10-05 at 09:57 -0300, Marcelo Tosatti wrote:
> > > > On Tue, Oct 04, 2022 at 03:25:37AM -0300, Leonardo Bras wrote:
> > > > > Currently pcrypt_aead_init_tfm() will pick callback cpus (ctx->cb_cpu)
> > > > > from any online cpus. Later padata_reorder() will queue_work_on() the
> > > > > chosen cb_cpu.
> > > > >
> > > > > This is undesired if the chosen cb_cpu is listed as isolated (i.e. using
> > > > > isolcpus=... kernel parameter), since the work queued will interfere with
> > > > > the workload on the isolated cpu.
> > > > >
> > > > > Make sure isolated cpus are not used for pcrypt.
> > > > >
> > > > > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> > > > > ---
> > > > >  crypto/pcrypt.c | 10 +++++++---
> > > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> > > > > index 9d10b846ccf73..9017d08c91a8d 100644
> > > > > --- a/crypto/pcrypt.c
> > > > > +++ b/crypto/pcrypt.c
> > > > > @@ -16,6 +16,7 @@
> > > > >  #include <linux/kobject.h>
> > > > >  #include <linux/cpu.h>
> > > > >  #include <crypto/pcrypt.h>
> > > > > +#include <linux/sched/isolation.h>
> > > > >
> > > > >  static struct padata_instance *pencrypt;
> > > > >  static struct padata_instance *pdecrypt;
> > > > > @@ -175,13 +176,16 @@ static int pcrypt_aead_init_tfm(struct crypto_aead *tfm)
> > > > >         struct pcrypt_instance_ctx *ictx = aead_instance_ctx(inst);
> > > > >         struct pcrypt_aead_ctx *ctx = crypto_aead_ctx(tfm);
> > > > >         struct crypto_aead *cipher;
> > > > > +       struct cpumask non_isolated;
> > > > > +
> > > > > +       cpumask_and(&non_isolated, cpu_online_mask, housekeeping_cpumask(HK_TYPE_DOMAIN));
> > > >
> > > > Since certain systems do not use isolcpus=domain, so please use a flag
> > > > that is setup by nohz_full=, for example HK_FLAG_MISC:
> > > >
> > > > static int __init housekeeping_nohz_full_setup(char *str)
> > > > {
> > > >         unsigned long flags;
> > > >
> > > >         flags = HK_FLAG_TICK | HK_FLAG_WQ | HK_FLAG_TIMER | HK_FLAG_RCU |
> > > >                 HK_FLAG_MISC | HK_FLAG_KTHREAD;
> > > >
> > > >         return housekeeping_setup(str, flags);
> > > > }
> > > > __setup("nohz_full=", housekeeping_nohz_full_setup);
> > >
> > > Oh, sure.
> > > Since we are talking about WorkQueues, I think it makes sense to pick
> > > HK_FLAG_WQ.
> > >
> > > >
> > > > Also, shouldnt you use cpumask_t ?/
> > >
> > > Yeah, I think so.
> > > I was quick to choose the 'struct cpumask' because all functions would operate
> > > in this variable type, but yeah, I think it makes sense to have this variable
> > > being opaque here.
> >
> > In fact, it seems neither 'struct cpumask' nor 'cpumask_t' are recommended to be
> > used allocated in the stack, due to the large size it can get (up to 1kB).
> >
> > At include/linux/cpumask.h we have:
> > 'cpumask_var_t: struct cpumask for stack usage'
> > which should work better at least for init functions like this.
> >
> > In other cases, I see 'static cpumask_t' being used to avoid the allocation
> > overhead, but it's probably due to the functions being called in very specific
> > scenarios. It could mean trouble if multiple cpus try to use it at once.
> >
> > What do you recommend on it?
>
> Sorry for the delay. I suppose allocating and freeing is OK in this context, since
> its initialization time and not a hot path?

Yeah, it makes sense this way. I will allocate as suggested!
(Unless my other change gets in, so this new variable and a lot of
overhead can be avoided.)

Thanks Marcelo!


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-02  2:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04  6:25 [PATCH v1 1/1] crypto/pcrypt: Do not use isolated CPUs for callback Leonardo Bras
2022-10-05 12:57 ` Marcelo Tosatti
2022-10-07 21:42   ` Leonardo Brás
2022-10-11 18:20     ` Leonardo Brás
2022-11-01 17:35       ` Marcelo Tosatti
2022-11-02  2:09         ` Leonardo Bras Soares Passos

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).