[Bug 1607] __RTEMS_SIZEOF_VOID_P__ flawed design

[bugzilla-daemon at rtems.org]

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

Gedare <giddyup44 at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |giddyup44 at yahoo.com

--- Comment #5 from Gedare <giddyup44 at yahoo.com> 2010-07-07 12:16:10 CDT ---
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > (In reply to comment #0)
> > > > > This change is a massive design flaw:
> > > > > 
> > > > > > 2010-06-29 Joel Sherrill <joel.sherrill at oarcorp.com>
> > > > > > 
> > > > > > * configure.ac, score/include/rtems/score/percpu.h: Add
> > > > > > __RTEMS_SIZEOF_VOID_P__ to cpuopts.h so percpu.h has this information
> > > > > > available during build and after installation.
> > > > > 
> > > > > Sizes are compile time constants and therefore may depend upon compiler 
> > > > > flags (e.g. multilibs) flags. Defining them in a globally exported 
> > > > > header being shared by different architecture variants qualifies is absurd.
> > > > > 
> > > > > Proposal: Joel to revert his whole __RTEMS_SIZEOF_VOID_P__ patch-set.
> > 
> > It was the least intrusive solution at hand.
> Wrong, this was an ugly hack.
> 
Is there a proper way to extract the sizeof(void*) (or more particularly
offsets of structures) with the assembler?

> > We have to have a per cpu
> > data structure to support SMP and it has to have a few fields accessed from
> > assembly language.
> My view differs.
> 
Per-cpu data is "good" for making scheduling decisions in SMP and multicore
systems because locks are not required to access the per-cpu structures. It is
bad to acquire locks in scheduling code, especially I suspect that real-time
guarantees will be worse.  So my opinion is that RTEMS needs per-cpu support if
it wants to support current and next-gen systems.

To use per-cpu structures in scheduling means having that code in the cpukit /
supercore.  It also is required in interrupt handling code, because RTEMS
supports preempting after an ISR and because dispatching across CPUs will
require using interprocessor interrupts, so the scheduler and dispatcher need
to interact with the IRQ handling code.  Current interrupt handling code lives
either in cpukit or libcpu.

It should be the case that scheduling code using per-cpu structures is
multi-lib C code only, and so it should not require this contentious macro (not
sure about dispatching across CPUs).  However, the interrupt handling makes
liberal use of assembly code and apparently requires this macro in .S files. 
So the problem at hand is how to access fields of the Per_CPU_Information
structure from assembly files for the interrupt handling.

> >  Please help find an acceptable way to support this.  
> With all due respect, provided you knew in advance that what you did is an ugly
> design flaw in advance (Even you said so, somewhere, either PM or in another
> BZ) I find reverting your patches and to send you "back to the drawing board"
> to be a reasonable response.
> 
> > > > Maybe we should add this define to a no-installation header file?
> > > Right. #1 candidate would be config.h.
> > 
> > I would be OK with that EXCEPT that some (or all to follow your proposal
> > below) of the interrupt processing code is outside the CPU Kit and needs to
> > know this.
> Correct, this reveils another piece of "questionable design" (lack of an
> interface definion).
> 
I think it actually reveals the lack of uniformity in the existing interrupt
handling code.  Some architectures have managed to keep IRQ code in score/cpu,
while others have relocated that code to libcpu.

> > > I.e. an alternative to reverting Joel's stuff would be to extract all defines
> > > which are using __RTEMS_SIZEOF_VOID_P__ to a private per-BSP header and to
> > > remove all use case from cpukit (Unfortunately, this doesn't seem to easily 
> > > achievable).
> > 
> > That will require moving all IRQ processing code from the CPU Kit and moving it
> > to libbsp somewhere. 
> Correct ... direct consequence of your design.
> 
> > Plus adding some check in each BSP configure.ac.
> Correct, however such checks can be centralized.
> 
Centralized to each libbsp/CPU_MODEL/configure.ac?  Will that be correct in all
cases?

In that case should IRQ handling and the ac_check be in libcpu?

I think the real "design flaw" here is that scheduling code in score and IRQ
handling code in libcpu both need to know about the percpu structure.  This
would be fine, except that some of IRQ handling code uses assembly to index the
structures and apparently there is no compile-time sizeof() macro available.

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