lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c00462a5-c5c0-a749-15aa-c0efaf232e71@xen0n.name>
Date:   Fri, 2 Dec 2022 17:41:30 +0800
From:   WANG Xuerui <kernel@...0n.name>
To:     maobibo <maobibo@...ngson.cn>, Huacai Chen <chenhuacai@...nel.org>
Cc:     loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ