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. :) 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 > 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> > --- > 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 https://lists.sourceforge.net/lists/listinfo/edk2-devel
