All of lore.kernel.org
 help / color / mirror / Atom feed
From: Charlie Jenkins <charlie@rivosinc.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	linux-kernel@vger.kernel.org,
	"Matthew Wilcox \(Oracle\)" <willy@infradead.org>,
	Samuel Holland <samuel.holland@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH] cpumask: de-duplicate assign_cpu() API
Date: Thu, 1 Aug 2024 10:27:20 -0700	[thread overview]
Message-ID: <ZqvFeJVy4BpwcOjm@ghost> (raw)
In-Reply-To: <20240731195355.97488-1-yury.norov@gmail.com>

On Wed, Jul 31, 2024 at 12:53:54PM -0700, Yury Norov wrote:
> We've got cpumask_assign_cpu() function and assign_cpu() macro, both
> doing the same thing. We need to drop one to avoid unneeded duplicatioon.
> 
> Now that underlying assign_bit() implemented as a macro, it would make
> sense to keep assign_cpu() which is also implemented as a macro, in sake
> of unification.
> 
> This patch also removes __cpumask_assign_cpu() as the function is
> unused.
> 
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/riscv/mm/cacheflush.c |  2 +-
>  include/linux/cpumask.h    | 16 ----------------
>  2 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index a03c994eed3b..fa136627ccaa 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -171,7 +171,7 @@ static void set_icache_stale_mask(void)
>  	stale_cpu = cpumask_test_cpu(smp_processor_id(), mask);
>  
>  	cpumask_setall(mask);
> -	cpumask_assign_cpu(smp_processor_id(), mask, stale_cpu);
> +	assign_cpu(smp_processor_id(), mask, stale_cpu);
>  }
>  #endif
>  
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 801a7e524113..f896c6ffa78e 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -534,22 +534,6 @@ static __always_inline void __cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>  	__clear_bit(cpumask_check(cpu), cpumask_bits(dstp));
>  }
>  
> -/**
> - * cpumask_assign_cpu - assign a cpu in a cpumask
> - * @cpu: cpu number (< nr_cpu_ids)
> - * @dstp: the cpumask pointer
> - * @bool: the value to assign
> - */
> -static __always_inline void cpumask_assign_cpu(int cpu, struct cpumask *dstp, bool value)
> -{
> -	assign_bit(cpumask_check(cpu), cpumask_bits(dstp), value);
> -}
> -
> -static __always_inline void __cpumask_assign_cpu(int cpu, struct cpumask *dstp, bool value)
> -{
> -	__assign_bit(cpumask_check(cpu), cpumask_bits(dstp), value);
> -}

This deletion is the wrong way around. cpumask_assign_cpu already
existed when assign_cpu was added. cpumask_assign_cpu uses the same
naming convention as all of the other cpumask functions, so it is
inconsistent with the other defines in this file to drop the "cpumask"
prefix.

__cpumask_assign_cpu is not currently used but all of the other cpumask
functions have a "__" alternative so that's why I added it.

- Charlie

> -
>  /**
>   * cpumask_test_cpu - test for a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
> -- 

> 2.43.0
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Charlie Jenkins <charlie@rivosinc.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: linux-kernel@vger.kernel.org, Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Samuel Holland <samuel.holland@sifive.com>,
	Sunil V L <sunilvl@ventanamicro.com>,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH] cpumask: de-duplicate assign_cpu() API
Date: Thu, 1 Aug 2024 10:27:20 -0700	[thread overview]
Message-ID: <ZqvFeJVy4BpwcOjm@ghost> (raw)
In-Reply-To: <20240731195355.97488-1-yury.norov@gmail.com>

On Wed, Jul 31, 2024 at 12:53:54PM -0700, Yury Norov wrote:
> We've got cpumask_assign_cpu() function and assign_cpu() macro, both
> doing the same thing. We need to drop one to avoid unneeded duplicatioon.
> 
> Now that underlying assign_bit() implemented as a macro, it would make
> sense to keep assign_cpu() which is also implemented as a macro, in sake
> of unification.
> 
> This patch also removes __cpumask_assign_cpu() as the function is
> unused.
> 
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/riscv/mm/cacheflush.c |  2 +-
>  include/linux/cpumask.h    | 16 ----------------
>  2 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index a03c994eed3b..fa136627ccaa 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -171,7 +171,7 @@ static void set_icache_stale_mask(void)
>  	stale_cpu = cpumask_test_cpu(smp_processor_id(), mask);
>  
>  	cpumask_setall(mask);
> -	cpumask_assign_cpu(smp_processor_id(), mask, stale_cpu);
> +	assign_cpu(smp_processor_id(), mask, stale_cpu);
>  }
>  #endif
>  
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 801a7e524113..f896c6ffa78e 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -534,22 +534,6 @@ static __always_inline void __cpumask_clear_cpu(int cpu, struct cpumask *dstp)
>  	__clear_bit(cpumask_check(cpu), cpumask_bits(dstp));
>  }
>  
> -/**
> - * cpumask_assign_cpu - assign a cpu in a cpumask
> - * @cpu: cpu number (< nr_cpu_ids)
> - * @dstp: the cpumask pointer
> - * @bool: the value to assign
> - */
> -static __always_inline void cpumask_assign_cpu(int cpu, struct cpumask *dstp, bool value)
> -{
> -	assign_bit(cpumask_check(cpu), cpumask_bits(dstp), value);
> -}
> -
> -static __always_inline void __cpumask_assign_cpu(int cpu, struct cpumask *dstp, bool value)
> -{
> -	__assign_bit(cpumask_check(cpu), cpumask_bits(dstp), value);
> -}

This deletion is the wrong way around. cpumask_assign_cpu already
existed when assign_cpu was added. cpumask_assign_cpu uses the same
naming convention as all of the other cpumask functions, so it is
inconsistent with the other defines in this file to drop the "cpumask"
prefix.

__cpumask_assign_cpu is not currently used but all of the other cpumask
functions have a "__" alternative so that's why I added it.

- Charlie

> -
>  /**
>   * cpumask_test_cpu - test for a cpu in a cpumask
>   * @cpu: cpu number (< nr_cpu_ids)
> -- 

> 2.43.0
> 

  reply	other threads:[~2024-08-01 18:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-31 19:53 [PATCH] cpumask: de-duplicate assign_cpu() API Yury Norov
2024-07-31 19:53 ` Yury Norov
2024-08-01 17:27 ` Charlie Jenkins [this message]
2024-08-01 17:27   ` Charlie Jenkins

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=ZqvFeJVy4BpwcOjm@ghost \
    --to=charlie@rivosinc.com \
    --cc=alexghiti@rivosinc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=samuel.holland@sifive.com \
    --cc=willy@infradead.org \
    --cc=yury.norov@gmail.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.