[Bug 1813] SMP code: shared variable access

bugzilla-daemon at rtems.org bugzilla-daemon at rtems.org
Mon Jun 13 15:02:07 UTC 2011


https://www.rtems.org/bugzilla/show_bug.cgi?id=1813

--- Comment #4 from Gedare <giddyup44 at yahoo.com> 2011-06-13 10:02:06 CDT ---
(In reply to comment #3)
> (In reply to comment #0)
> > In the code submitted as PR 1796/cpukit, for instance in
> > cpukit/score/src/smp.c:
> > _SMP_Request_other_cores_to_perform_first_context_switch there are accesses to
> > shared variables not protected by any locks. This will not work on an
> > architecture without coherent caches. 
> > 
> > There are two possible options to resolve the problem:
> > 1. All accesses to _Per_CPU_Information use the proper locks.
> 
> There is no reason to do this because AFAIK all OS proper accesses are within
> thread dispatch disable sections.  This locks the structure.
> 
Does the thread dispatch disable section lock/prevent the OS on another
CPU/core from executing in parallel?

> When touching the message field, the lock in the per cpu is grabbed.
> 
> When calling _SMP_Request_other_cores_to_perform_first_context_switch(),
> the secondary cores are in a known state and we can take advantage of it.
> 
> > 2. Read accesses use a special BSP function like  bsp_smp_read_int() which
> > provides the right access method. Additionally, all such accesses made to
> > _Per_CPU_Information should cast _Per_CPU_Information to volatile. Forgotten
> > volatile will easily lead to hard to debug bugs, so a separate function may be
> > better also for this reason.
> 
> Gedare has been arguing against volatile for these fields.  I am ambivalent
> because I am more worried about the impact of caching than volatile.  We are
> pretty careful to use compiler memory barriers with the critical sections. That
> is stronger than volatile because nothing else should change the value while it
> is locked.
> 
A nice read is the "Volatile considered harmful" article:
http://www.kernel.org/doc/Documentation/volatile-considered-harmful.txt

The volatile keyword is known to cause problems in optimizing compilers:
http://blog.regehr.org/archives/503

For these reasons, I am hesitant to use it except when absolutely necessary. It
is much better to analyze such code for why volatile would be used, and replace
with explicit memory barriers or other constructs when needed. There are times
that volatile is useful, but it should not be relied on blindly.

> > I can provide a patch when the decision is made.

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.



More information about the bugs mailing list