date:20240301

[PATCH v2] ring-buffer: use READ_ONCE() to read cpu_buffer->commit_page in concurrent environment

2024-03-01 Thread linke li

In function ring_buffer_iter_empty(), cpu_buffer->commit_page is read
while other threads may change it. It may cause the time_stamp that read
in the next line come from a different page. Use READ_ONCE() to avoid
having to reason about compiler optimizations now and in future.

Signed-off-by: linke li 
---
v1 -> v2: only add READ_ONCE() to read cpu_buffer->commit_page, make change log 
clear

 kernel/trace/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 0699027b4f4c..c7203a436d3c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4337,7 +4337,7 @@ int ring_buffer_iter_empty(struct ring_buffer_iter *iter)
cpu_buffer = iter->cpu_buffer;
reader = cpu_buffer->reader_page;
head_page = cpu_buffer->head_page;
-   commit_page = cpu_buffer->commit_page;
+   commit_page = READ_ONCE(cpu_buffer->commit_page);
commit_ts = commit_page->page->time_stamp;
 
/*
-- 
2.39.3 (Apple Git-145)

Re: [PATCH v5 3/6] LoongArch: KVM: Add cpucfg area for kvm hypervisor

2024-03-01 Thread maobibo

On 2024/2/27 下午6:19, WANG Xuerui wrote:

On 2/27/24 18:12, maobibo wrote:

On 2024/2/27 下午5:10, WANG Xuerui wrote:

On 2/27/24 11:14, maobibo wrote:

On 2024/2/27 上午4:02, Jiaxun Yang wrote:

在2024年2月26日二月 上午8:04，maobibo写道：

On 2024/2/26 下午2:12, Huacai Chen wrote:
On Mon, Feb 26, 2024 at 10:04 AM maobibo
wrote:

On 2024/2/24 下午5:13, Huacai Chen wrote:

Hi, Bibo,

On Thu, Feb 22, 2024 at 11:28 AM Bibo Mao
wrote:

Instruction cpucfg can be used to get processor features. And
there

is trap exception when it is executed in VM mode, and also it is
to provide cpu features to VM. On real hardware cpucfg area 0
- 20
is used. Here one specified area 0x4000 -- 0x40ff is
used
for KVM hypervisor to privide PV features, and the area can be
extended

for other hypervisors in future. This area will never be used for
real HW, it is only used by software.
After reading and thinking, I find that the hypercall method
which is

used in our productive kernel is better than this cpucfg method.
Because hypercall is more simple and straightforward, plus we
don't

worry about conflicting with the real hardware.
No, I do not think so. cpucfg is simper than hypercall,
hypercall can
be in effect when system runs in guest mode. In some scenario
like TCG

mode, hypercall is illegal intruction, however cpucfg can work.

Nearly all architectures use hypercall except x86 for its historical
Only x86 support multiple hypervisors and there is multiple
hypervisor

in x86 only. It is an advantage, not historical reason.

I do believe that all those stuff should not be exposed to guest
user space

for security reasons.
Can you add PLV checking when cpucfg 0x4000-0x40FF is
emulated? if it is user mode return value is zero and it is kernel
mode emulated value will be returned. It can avoid information leaking.

I've suggested this approach in another reply [1], but I've rechecked
the manual, and it turns out this behavior is not permitted by the
current wording. See LoongArch Reference Manual v1.10, Volume 1,
Section 2.2.10.5 "CPUCFG":

> CPUCFG 访问未定义的配置字将读回全 0 值。
>
> Reads of undefined CPUCFG configuration words shall return
all-zeroes.

This sentence mentions no distinction based on privilege modes, so it
can only mean the behavior applies universally regardless of
privilege modes.

I think if you want to make CPUCFG behavior PLV-dependent, you may
have to ask the LoongArch spec editors, internally or in public, for
a new spec revision.
No, CPUCFG behavior between CPUCFG0-CPUCFG21 is unchanged, only that
it can be defined by software since CPUCFG 0x4 is used by
software.

The 0x4000 range is not mentioned in the manuals. I know you've
confirmed privately with HW team but this needs to be properly
documented for public projects to properly rely on.

(There are already multiple third-party LoongArch implementers as of
late 2023, so any ISA-level change like this would best be
coordinated, to minimize surprises.)

With document Vol 4-23
https://www.intel.com/content/dam/develop/external/us/en/documents/335592-sdm-vol-4.pdf

There is one line "MSR address range between 4000H - 40FFH is
marked as a specially reserved range. All existing and
future processors will not implement any features using any MSR in
this range."

Thanks for providing this info, now at least we know why it's this
specific range of 0x40XX that's chosen.

It only says that it is reserved, it does not say detailed software
behavior. Software behavior is defined in hypervisor such as:
https://github.com/MicrosoftDocs/Virtualization-Documentation/blob/main/tlfs/Requirements%20for%20Implementing%20the%20Microsoft%20Hypervisor%20Interface.pdf

https://kb.vmware.com/s/article/1009458

If hypercall method is used, there should be ABI also like aarch64:
https://documentation-service.arm.com/static/6013e5faeee5236980d08619

Yes proper documentation of public API surface is always necessary
*before* doing real work. Because right now the hypercall provider is
Linux KVM, maybe we can document the existing and planned hypercall
usage and ABI in the kernel docs along with code changes.

yes, I will add hypercall in kernel docs.

SMC calling convention is ABI between OS and secure firmware, LoongArch
has no secure mode, it not necessary to has such doc.The hypercall
calling convention is relative with hypervisor SW, not relative with CPU
HW vendor. Each hypervisor maybe has its separate hypercall calling
convention just like syscall ABIs.

Regards
Bibo Mao

Re: [PATCH RFC ftrace] Chose RCU Tasks based on TASKS_RCU rather than PREEMPTION

2024-03-01 Thread Paul E. McKenney

On Fri, Mar 01, 2024 at 03:30:01PM -0500, Steven Rostedt wrote:
> On Fri, 1 Mar 2024 12:25:10 -0800
> "Paul E. McKenney"  wrote:
> 
> > > That would work for me.  If there are no objections, I will make this
> > > change.  
> > 
> > But I did check the latency of synchronize_rcu_tasks_rude() (about 100ms)
> > and synchronize_rcu() (about 20ms).  This is on a 80-hardware-thread
> > x86 system that is being flooded with calls to one or the other of
> > these two functions, but is otherwise idle.  So adding that unnecessary
> > synchronize_rcu() adds about 20% to that synchronization delay.
> > 
> > Which might still be OK, but...  In the immortal words of MS-DOS,
> > "Are you sure?".  ;-)
> 
> It's just safe to keep it. It's definitely not a fast path.

OK, you got it!  ;-)

Thanx, Paul

57 matches

Mail list logo