2022-12-02 08:57:16

by Bibo Mao

[permalink] [raw]
Subject: [PATCH] LoongArch: export symbol with function smp_send_reschedule

Function smp_send_reschedule is standard kernel ABI, which is
defined header file include/linux/smp.h, however on LoongArch
it is defined as inline function, so that kernel module can
not use this function.

Now define smp_send_reschedule as general function, and add
EXPORT_SYMBOL_GPL on this function, so that kernel modules can
use it.

Signed-off-by: Bibo Mao <[email protected]>
---
arch/loongarch/include/asm/smp.h | 10 ----------
arch/loongarch/kernel/smp.c | 11 +++++++++++
2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
index 3dd172d9ffea..d82687390b4a 100644
--- a/arch/loongarch/include/asm/smp.h
+++ b/arch/loongarch/include/asm/smp.h
@@ -78,16 +78,6 @@ extern void calculate_cpu_foreign_map(void);
*/
extern void show_ipi_list(struct seq_file *p, int prec);

-/*
- * This function sends a 'reschedule' IPI to another CPU.
- * it goes straight through and wastes no time serializing
- * anything. Worst case is that we lose a reschedule ...
- */
-static inline void smp_send_reschedule(int cpu)
-{
- loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
-}
-
static inline void arch_send_call_function_single_ipi(int cpu)
{
loongson_send_ipi_single(cpu, SMP_CALL_FUNCTION);
diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
index 6ed72f7ff278..51dd3c3f06cb 100644
--- a/arch/loongarch/kernel/smp.c
+++ b/arch/loongarch/kernel/smp.c
@@ -141,6 +141,17 @@ void loongson_send_ipi_single(int cpu, unsigned int action)
ipi_write_action(cpu_logical_map(cpu), (u32)action);
}

+/*
+ * This function sends a 'reschedule' IPI to another CPU.
+ * it goes straight through and wastes no time serializing
+ * anything. Worst case is that we lose a reschedule ...
+ */
+void smp_send_reschedule(int cpu)
+{
+ loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
+}
+EXPORT_SYMBOL_GPL(smp_send_reschedule);
+
void loongson_send_ipi_mask(const struct cpumask *mask, unsigned int action)
{
unsigned int i;
--
2.27.0


2022-12-02 09:22:08

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: export symbol with function smp_send_reschedule

On 2022/12/2 15:58, Bibo Mao wrote:
> Function smp_send_reschedule is standard kernel ABI, which is
> defined header file include/linux/smp.h, however on LoongArch
> it is defined as inline function, so that kernel module can
> not use this function.
>
> Now define smp_send_reschedule as general function, and add
> EXPORT_SYMBOL_GPL on this function, so that kernel modules can
> use it.
>
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> arch/loongarch/include/asm/smp.h | 10 ----------
> arch/loongarch/kernel/smp.c | 11 +++++++++++
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
> index 3dd172d9ffea..d82687390b4a 100644
> --- a/arch/loongarch/include/asm/smp.h
> +++ b/arch/loongarch/include/asm/smp.h
> @@ -78,16 +78,6 @@ extern void calculate_cpu_foreign_map(void);
> */
> extern void show_ipi_list(struct seq_file *p, int prec);
>
> -/*
> - * This function sends a 'reschedule' IPI to another CPU.
> - * it goes straight through and wastes no time serializing
> - * anything. Worst case is that we lose a reschedule ...
> - */
> -static inline void smp_send_reschedule(int cpu)
> -{
> - loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
> -}
> -
> static inline void arch_send_call_function_single_ipi(int cpu)
> {
> loongson_send_ipi_single(cpu, SMP_CALL_FUNCTION);
> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> index 6ed72f7ff278..51dd3c3f06cb 100644
> --- a/arch/loongarch/kernel/smp.c
> +++ b/arch/loongarch/kernel/smp.c
> @@ -141,6 +141,17 @@ void loongson_send_ipi_single(int cpu, unsigned int action)
> ipi_write_action(cpu_logical_map(cpu), (u32)action);
> }
>
> +/*
> + * This function sends a 'reschedule' IPI to another CPU.
> + * it goes straight through and wastes no time serializing
> + * anything. Worst case is that we lose a reschedule ...
> + */
> +void smp_send_reschedule(int cpu)
> +{
> + loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
> +}
> +EXPORT_SYMBOL_GPL(smp_send_reschedule);
> +

While the change is in itself okay (one less case of mips legacy,
getting in line with ia64, powerpc and riscv that all EXPORT_SYMBOL_GPL
this), I'd suggest you batch this patch with the subsequent changes you
plan to enable with this one, so reviewers would have more context and
hopefully avoid churn. (I, by my familiarity with Loongson and LoongArch
development, know you're probably aiming to use this with KVM, but
others probably don't know, and again it's always better to have more
context.)

> void loongson_send_ipi_mask(const struct cpumask *mask, unsigned int action)
> {
> unsigned int i;

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

2022-12-02 09:23:41

by Bibo Mao

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: export symbol with function smp_send_reschedule



在 2022/12/2 16:25, WANG Xuerui 写道:
> On 2022/12/2 15:58, Bibo Mao wrote:
>> Function smp_send_reschedule is standard kernel ABI, which is
>> defined header file include/linux/smp.h, however on LoongArch
>> it is defined as inline function, so that kernel module can
>> not use this function.
>>
>> Now define smp_send_reschedule as general function, and add
>> EXPORT_SYMBOL_GPL on this function, so that kernel modules can
>> use it.
>>
>> Signed-off-by: Bibo Mao <[email protected]>
>> ---
>>   arch/loongarch/include/asm/smp.h | 10 ----------
>>   arch/loongarch/kernel/smp.c      | 11 +++++++++++
>>   2 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/smp.h b/arch/loongarch/include/asm/smp.h
>> index 3dd172d9ffea..d82687390b4a 100644
>> --- a/arch/loongarch/include/asm/smp.h
>> +++ b/arch/loongarch/include/asm/smp.h
>> @@ -78,16 +78,6 @@ extern void calculate_cpu_foreign_map(void);
>>    */
>>   extern void show_ipi_list(struct seq_file *p, int prec);
>>   -/*
>> - * This function sends a 'reschedule' IPI to another CPU.
>> - * it goes straight through and wastes no time serializing
>> - * anything. Worst case is that we lose a reschedule ...
>> - */
>> -static inline void smp_send_reschedule(int cpu)
>> -{
>> -    loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
>> -}
>> -
>>   static inline void arch_send_call_function_single_ipi(int cpu)
>>   {
>>       loongson_send_ipi_single(cpu, SMP_CALL_FUNCTION);
>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>> index 6ed72f7ff278..51dd3c3f06cb 100644
>> --- a/arch/loongarch/kernel/smp.c
>> +++ b/arch/loongarch/kernel/smp.c
>> @@ -141,6 +141,17 @@ void loongson_send_ipi_single(int cpu, unsigned int action)
>>       ipi_write_action(cpu_logical_map(cpu), (u32)action);
>>   }
>>   +/*
>> + * This function sends a 'reschedule' IPI to another CPU.
>> + * it goes straight through and wastes no time serializing
>> + * anything. Worst case is that we lose a reschedule ...
>> + */
>> +void smp_send_reschedule(int cpu)
>> +{
>> +    loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
>> +}
>> +EXPORT_SYMBOL_GPL(smp_send_reschedule);
>> +
>
> While the change is in itself okay (one less case of mips legacy, getting in line with ia64, powerpc and riscv that all EXPORT_SYMBOL_GPL this), I'd suggest you batch this patch with the subsequent changes you plan to enable with this one, so reviewers would have more context and hopefully avoid churn. (I, by my familiarity with Loongson and LoongArch development, know you're probably aiming to use this with KVM, but others probably don't know, and again it's always better to have more context.)
>

yes, kvm module depends on function smp_send_reschedule, only that it is not mature now. And this function is standard API, not arch specified API, it is normal for modules to use it :)

regards
bibo,mao
>>   void loongson_send_ipi_mask(const struct cpumask *mask, unsigned int action)
>>   {
>>       unsigned int i;
>

2022-12-02 10:38:43

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: export symbol with function smp_send_reschedule

On 2022/12/2 17:03, maobibo wrote:
>>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
>>> index 6ed72f7ff278..51dd3c3f06cb 100644
>>> --- a/arch/loongarch/kernel/smp.c
>>> +++ b/arch/loongarch/kernel/smp.c
>>> @@ -141,6 +141,17 @@ void loongson_send_ipi_single(int cpu, unsigned int action)
>>>       ipi_write_action(cpu_logical_map(cpu), (u32)action);
>>>   }
>>>   +/*
>>> + * This function sends a 'reschedule' IPI to another CPU.
>>> + * it goes straight through and wastes no time serializing
>>> + * anything. Worst case is that we lose a reschedule ...
>>> + */
>>> +void smp_send_reschedule(int cpu)
>>> +{
>>> +    loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
>>> +}
>>> +EXPORT_SYMBOL_GPL(smp_send_reschedule);
>>> +
>>
>> While the change is in itself okay (one less case of mips legacy, getting in line with ia64, powerpc and riscv that all EXPORT_SYMBOL_GPL this), I'd suggest you batch this patch with the subsequent changes you plan to enable with this one, so reviewers would have more context and hopefully avoid churn. (I, by my familiarity with Loongson and LoongArch development, know you're probably aiming to use this with KVM, but others probably don't know, and again it's always better to have more context.)
>>
>
> yes, kvm module depends on function smp_send_reschedule, only that it is not mature now. And this function is standard API, not arch specified API, it is normal for modules to use it :)

Hmm, maybe you could post some kind of "sneak peek" code for early
reviews on broader things like overall approach and architecture?
Frankly speaking, experience suggests that code from Loongson usually
needs much refactoring to meet mainline standards, and posting your
design and some initial implementation could save you and the community
a *huge* amount of time and hassle.

And I'm not arguing this patch shouldn't get included, it's the
opposite, but I don't see any difference in applying it now or later
when the whole LoongArch KVM support gets mainlined, so maybe it's
better to wait so we don't cause any churn if the change turns out
unnecessary. For example, in my grepping I found that x86 doesn't have
smp_send_reschedule exported, yet its KVM port has no problem using it;
and that the s390 and riscv KVM ports don't invoke smp_send_reschedule
at all. So it's entirely possible that LoongArch won't need this change
for KVM after all, and I'm suggesting to save everyone some time.

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

2022-12-08 13:42:37

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: export symbol with function smp_send_reschedule

Applied, thanks.

Huacai

On Fri, Dec 2, 2022 at 5:41 PM WANG Xuerui <[email protected]> wrote:
>
> On 2022/12/2 17:03, maobibo wrote:
> >>> diff --git a/arch/loongarch/kernel/smp.c b/arch/loongarch/kernel/smp.c
> >>> index 6ed72f7ff278..51dd3c3f06cb 100644
> >>> --- a/arch/loongarch/kernel/smp.c
> >>> +++ b/arch/loongarch/kernel/smp.c
> >>> @@ -141,6 +141,17 @@ void loongson_send_ipi_single(int cpu, unsigned int action)
> >>> ipi_write_action(cpu_logical_map(cpu), (u32)action);
> >>> }
> >>> +/*
> >>> + * This function sends a 'reschedule' IPI to another CPU.
> >>> + * it goes straight through and wastes no time serializing
> >>> + * anything. Worst case is that we lose a reschedule ...
> >>> + */
> >>> +void smp_send_reschedule(int cpu)
> >>> +{
> >>> + loongson_send_ipi_single(cpu, SMP_RESCHEDULE);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(smp_send_reschedule);
> >>> +
> >>
> >> While the change is in itself okay (one less case of mips legacy, getting in line with ia64, powerpc and riscv that all EXPORT_SYMBOL_GPL this), I'd suggest you batch this patch with the subsequent changes you plan to enable with this one, so reviewers would have more context and hopefully avoid churn. (I, by my familiarity with Loongson and LoongArch development, know you're probably aiming to use this with KVM, but others probably don't know, and again it's always better to have more context.)
> >>
> >
> > yes, kvm module depends on function smp_send_reschedule, only that it is not mature now. And this function is standard API, not arch specified API, it is normal for modules to use it :)
>
> Hmm, maybe you could post some kind of "sneak peek" code for early
> reviews on broader things like overall approach and architecture?
> Frankly speaking, experience suggests that code from Loongson usually
> needs much refactoring to meet mainline standards, and posting your
> design and some initial implementation could save you and the community
> a *huge* amount of time and hassle.
>
> And I'm not arguing this patch shouldn't get included, it's the
> opposite, but I don't see any difference in applying it now or later
> when the whole LoongArch KVM support gets mainlined, so maybe it's
> better to wait so we don't cause any churn if the change turns out
> unnecessary. For example, in my grepping I found that x86 doesn't have
> smp_send_reschedule exported, yet its KVM port has no problem using it;
> and that the s390 and riscv KVM ports don't invoke smp_send_reschedule
> at all. So it's entirely possible that LoongArch won't need this change
> for KVM after all, and I'm suggesting to save everyone some time.
>
> --
> WANG "xen0n" Xuerui
>
> Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
>
>