Date: Wed, 22 Dec 2010 15:02:12 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org> Subject: Re: svn commit: r216634 - in head/sys/amd64: amd64 ia32 include linux32 Message-ID: <201012221502.13419.jkim@FreeBSD.org> In-Reply-To: <20101223041210.B2346@besplex.bde.org> References: <201012220018.oBM0IgOg079632@svn.freebsd.org> <201012221202.07774.jkim@FreeBSD.org> <20101223041210.B2346@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201012221502.13419.jkim>