svn commit: r216634 - in head/sys/amd64: amd64 ia32 include linux32

[Jung-uk Kim]

On Wednesday 22 December 2010 01:02 pm, Bruce Evans wrote:
> On Wed, 22 Dec 2010, Jung-uk Kim wrote:
> > On Wednesday 22 December 2010 08:03 am, John Baldwin wrote:
> >> On Tuesday, December 21, 2010 7:18:42 pm Jung-uk Kim wrote:
> >>> Log:
> >>>   Improve PCB flags handling and make it more robust.  Add two
> >>> new functions for manipulating pcb_flags.  These inline
> >>> functions are very similar to atomic_set_char(9) and
> >>> atomic_clear_char(9) but without unnecessary LOCK prefix for
> >>> SMP.  Add comments about the rationale[1].  Use these functions
> >>> wherever possible. Although there are some places where it is
> >>> not strictly necessary (e.g., a PCB is copied to create a new
> >>> PCB), it is done across the board for sake of consistency. 
> >>> Turn pcb_full_iret into a PCB flag as it is safe now.  Move
> >>> rarely used fields before pcb_flags and reduce size of
> >>> pcb_flags to one byte.  Fix some style(9) nits in pcb.h while I
> >>> am in the neighborhood.
> >>>
> >>>   Reviewed by:	kib
> >>>   Submitted by:	kib[1]
> >>>   MFC after:	2 months
> >>
> >> Is there really a need to have the flags field be a char instead
> >> of an int or long?  It seems to me that flags fields in general
> >> should be an int unless there is a strong need otherwise (e.g.
> >> hardware-defined flag).  'orl' will work just as well as 'orb'.
> >
> > Actually, there was little disagreement between kib and me and
> > committed version was fifth reincarnation of original patch.  I
>
> I would have a big disagreement :-).  I don't like using macros or
> functions to obfuscate setting and clearing of boolean flags.  Here
> the function may be needed for locking, but...

[Snipped FPU related comments]

> > originally used 'u_int pcb_flags' but used byte byte opcodes for
> > assembly to make it consistent with compiler generated code.  kib
>
> Unfortuneatly its easier for the compiler to maintain the
> optimization of doing byte accesses if that is good.
>
> > disliked the inconsistencies and told me to reconsider.  After
> > several versions, I proposed we use char and move forward for
> > now, then we increase the size if it ever grows more than 8 bits.
> >  So, we settled.  I have no problem with int version if there is
> > no measurable performance change.  I'll test it soon.
>
> I've never seen any performance loss from using 32-bit accesses,
> except with the original i386 since it had no cache and a worse
> instruction fetcher so that even the extra length of 32-bit
> immediate operands slowed it down.  With not-so-old CPUs it is the
> 8-bit accesses that are more likely to be slower, since they may be
> misaligned and/or cause store-to-load mismatch penalties.
>
> There are a couple of (read-only) accesses to td_flags and
> td_pflags in asm code very need the pcb_full_iret ones.  The former
> use simple testl's.

You're right, int is (marginally) better for pcb_flags.  Fixed.

Thanks,

Jung-uk Kim