[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZqvFeJVy4BpwcOjm@ghost>
Date: Thu, 1 Aug 2024 10:27:20 -0700
From: Charlie Jenkins <charlie@...osinc.com>
To: Yury Norov <yury.norov@...il.com>
Cc: linux-kernel@...r.kernel.org, Albert Ou <aou@...s.berkeley.edu>,
Alexandre Ghiti <alexghiti@...osinc.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Samuel Holland <samuel.holland@...ive.com>,
Sunil V L <sunilvl@...tanamicro.com>,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH] cpumask: de-duplicate assign_cpu() API
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@...il.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
>
Powered by blists - more mailing lists