Andrew is right.
Volatile makes compiler read variable value from memory each time. Reading from
memory does not mean that it will skip the cache. It still could get the value
from the cache if cache is enabled and valid. Volatile and cache are different
stores.
Thanks!
Jeff
From: Andrew Fish [mailto:af...@apple.com]
Sent: Thursday, March 05, 2015 10:31 AM
To: edk2-devel@lists.sourceforge.net
Cc: Fan, Jeff
Subject: Re: [edk2] [v2 4/4] UefiCpuPkg/MpSerivce: remove volatile qualifier
On Mar 4, 2015, at 5:34 PM, Chen Fan
<chen.fan.f...@cn.fujitsu.com<mailto:chen.fan.f...@cn.fujitsu.com>> wrote:
Hi Jeff,
Thanks for your explanation, I didn't encounter any issue if leaving it
there.
I just think if use Volatile qualifier may cause the problem of
performance. because
access the value need to copy the address value from main memory to cpu
cache
each time and not use the cache.
and IIRC, in a shared memory multiprocessor system, the cache coherence may
keep the consistency of shared resource data in each processor local
caches.
if the value is updated by one processor in local cache, the copy of
memory data in
other processor caches will be marked as invalid. and then other
processors read the
value will access from the main memory.
if I misunderstand, please correct me. :)
Correction!
Volatile in C has nothing to do with hardware and caching. It has to do with
the abstract memory model for the compiler as defined by the C standard. The
more aggressive the compiler optimizes the code the more you are exposed to
potential issues. If the compiler knows about all the code in the program that
accesses a variable it can assume that memory location will only be changed by
the compiler, and make optimizations (like storing a pointer in a register).
A common optimization is to avoid memory reads that are not needed. Here is a
simple example in clang, probably the most aggressive optimizing compiler I
know of…. (My comments start with #):
~/work/Compiler>cat volatile.c
int
main (int argc, char **argv)
{
int *i = (int *)0xFFFFFFF0;
while (*i != 0);
return 0;
}
~/work/Compiler>clang volatile.c -S -O3
~/work/Compiler>cat volatile.S
.section __TEXT,__text,regular,pure_instructions
.globl _main
.align 4, 0x90
_main: ## @main
pushq %rbp
movq %rsp, %rbp
movl $4294967280, %eax ## imm = 0xFFFFFFF0
cmpl $0, (%rax)
je LBB0_2
# Notice the compiler only does a single read. The memory model of C implies
this memory will not change.
.align 4, 0x90
LBB0_1: ## %..split_crit_edge
## =>This Inner Loop Header: Depth=1
jmp LBB0_1
# Notice this while loop is just an infinite loop? There is no point reading
the memory since the C spec states it will not change.
LBB0_2: ## %.split1
xorl %eax, %eax
popq %rbp
retq
If I make i a volatile variable it forces the compiler to read it every time:
volatile int *i = (int *)0xFFFFFFF0;
~/work/Compiler>cat volatile.S
.section __TEXT,__text,regular,pure_instructions
.globl _main
.align 4, 0x90
_main: ## @main
pushq %rbp
movq %rsp, %rbp
movl $4294967280, %eax ## imm = 0xFFFFFFF0
.align 4, 0x90
LBB0_1: ## =>This Inner Loop Header: Depth=1
cmpl $0, (%rax)
jne LBB0_1
# Now it reads every iteration of the while loop
xorl %eax, %eax
popq %rbp
retq
So the point of making it volatile is to force the compiler to read the value
every time the C code asks for the value.
Thanks,
Andrew Fish
Thanks,
Chen
On 03/04/2015 03:56 PM, Fan, Jeff wrote:
Chen,
Volatile is to tell compiler this variable maybe changed by other threads.
Compiler must access the value from memory instead of the old value in its
registers. This is different with spinlock that is to protect shared memory
between multiple threads.
For this case, CpuState may be updated by BSP/AP. I prefer to keep volatile
type. Do you meet any issue if keepping this type?
Besides it, I also suggest to add volatile for the following variable.
CpuData->Parameter
CpuData->Procedure
Thanks!
Jeff
-----Original Message-----
From: Chen Fan [mailto:chen.fan.f...@cn.fujitsu.com]
Sent: Tuesday, March 03, 2015 1:06 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Cc: Fan, Jeff; Justen, Jordan L
Subject: [v2 4/4] UefiCpuPkg/MpSerivce: remove volatile qualifier
because manipulating CpuState always with lock protection. so volatile
qualifier is redundant.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chen Fan
<chen.fan.f...@cn.fujitsu.com<mailto:chen.fan.f...@cn.fujitsu.com>>
---
UefiCpuPkg/CpuDxe/CpuMp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h index
cb3460f..e54b571 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.h
+++ b/UefiCpuPkg/CpuDxe/CpuMp.h
@@ -92,7 +92,7 @@ typedef struct {
EFI_PROCESSOR_INFORMATION Info;
SPIN_LOCK CpuDataLock;
INTN LockSelf;
- volatile CPU_STATE State;
+ CPU_STATE State;
EFI_AP_PROCEDURE Procedure;
VOID *Parameter;
--
1.9.3
.
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
