Date: Thu, 11 Jan 2001 02:24:28 -0500 From: Jake Burkholder <jake@freebsd.org> To: "Justin T. Gibbs" <gibbs@scsiguy.com> Cc: John Baldwin <jhb@FreeBSD.org>, =?ISO-8859-1?Q?G=E9rard_Roudier?= <groudier@club-internet.fr>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, Bruce Evans <bde@zeta.org.au> Subject: Re: cvs commit: src/sys/alpha/alpha interrupt.c machdep.c mp_mac Message-ID: <20010111072428.B4955BA80@cr66388-a.rchrd1.on.wave.home.com> In-Reply-To: Message from "Justin T. Gibbs" <gibbs@scsiguy.com> of "Wed, 10 Jan 2001 21:27:38 MST." <200101110428.f0B4Rcs23445@aslan.scsiguy.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> >No, shared variables are just variables. Here is the problem. There =
is no
> >'curproc' variable. There is a gd_curproc member of a struct globalda=
ta, and
> >each CPU has a struct globaldata tied to it. We can obtain it via
> >PCPU_GET(curproc), or set it via PCPU_SET(curproc, myproc). Having a =
'curproc
> >'
> >"magic" variable that doesn't exist that we assume we can read and wri=
te to
> >like any other variable is a lot more hokie than admitting that we are=
> >accessing a special variable in a special way.
> =
> Anyone touching these variables had better understand what they are for=
> and how they operate. I think our developers are smart enough to under=
stand
> this without ugly upper-case macros. Heck "errno" is not spelled
> PTHREAD_GET(errno), or PTHREAD_SET(errno, error) and still people seem
> to be able to write useful threaded apps. 8-)
errno is just one variable, there are a number of per-cpu variables,
more than I can easily remember. When we started SMPng I had to look
at the headers repeatedly to find out exactly what was and what wasn't.
This code, the kernel in general, was not new to me either, I was already=
familiar with a good deal of it.
The old implementation, a separate macro for each variable, had the conte=
nts
of globaldata duplicated 9 times.
There was globaldata itself:
struct globaldata {
struct proc *gd_curproc;
There was an extern declaration for each variable used in the UP kernel:
extern struct proc *curproc;
There was a macro that produced inline functions for each variable:
GLOBAL_FUNC(curproc); =
There was a macro that aliased the variable to an appropriate inline to
access it:
#define curproc GLOBAL_RVALUE_NV(curproc, struct proc *)
There was a macro for each variable to deal with the differences between
elf and aout symbols for UP, or to provide the fs indirection for SMP
when used from assembly language:
#define FS(x) _x
#define FS(x) %fs:x
#define _curproc FS(curproc)
There were 2 global symbols for each variable in globals.s:
=2Eglobl gd_curproc
=2Eglobl _curproc
There were 2 .sets for each variable to give the value of the symbols:
=2Eset gd_curproc,globaldata + GD_CURPROC
=2Eset _curproc,globaldata + GD_CURPROC
Finally, there was an ASSYM for each variable in genassym.c to produce th=
e
offset used to produce the above symbols:
ASSYM(GD_CURPROC, offsetof(struct globaldata, gd_curproc);
Needless to say this made adding a per-cpu variable easy to screw up even=
if you knew about all this. Much of this still remains, but I intend to
remove it shortly.
Some of the macros were rvalue-like and some of them were lvalue-like.
curproc was an rvalue only, so this wouldn't compile:
curproc =3D p;
The rvalues were more efficient so there was a SET_CURPROC macro to
set the value of curproc, which called the inline _global_set_curproc.
The inlines did not have the correct type, they all returned int or void =
*,
which was cast by the GLOBAL_{L,R}VALUE macros. This reduced some of the=
namespace pollution because the types only occured in the macros, so it
was only necessary to include the appropriate headers where the variables=
were used. The only major namespace pollution was due to several variabl=
es
having type pt_entry_t *, which requires vm headers to be included. This
is a problem with the current (new) implementation, but it will be fixed
because these variables are not appropriate for the pre-emptive kernel.
The new implementation does not require any other macros at all. All tha=
t
is necessary to add a per-cpu variable is to add the field to struct
globaldata.
Both the new and old implementations are machine dependent, which means
that there are 3 copies of globaldata.h and globals.h; i386, alpha and ia=
64.
There would have been another 2 at least for arm and powerpc. I have pla=
ns
to make this machine independent before any more copies are added. It is=
easier to do this if there are accessor macros, and not a macro for each
variable.
The new macros make fewer assumptions about how the machine dependent cod=
e
provides per-cpu variables. This will allow for optimizations in the fut=
ure,
as I've shown.
dfr started using PCPU_GET and PCPU_SET a long time ago in his alpha smp
code and in the ia64 tree. Although I wasn't there, apparently it was
discussed at the SMP meeting that this would be done in general. I think=
we should try for more similarities between the code for the various
architectures FreeBSD supports, rather than less.
I've listed some of the pros that I see for the new implementation and
some of the cons for the old implementation.
The cons that I've heard so far are:
1 Machine independent code should not need to know that some variables ar=
e
per-cpu.
This is false because the intention of SMPng is to make the kernel aware =
of
multiple processors. Not just that but also re-entrant on a uni-processo=
r. =
2 Efficiency.
I've shown that this can be improved. Optimizations are not overly
important right now.
3 It is difficult to find their values from debuggers.
This can be fixed and I will do this once globaldata is made machine
independent. I use a debugger a lot, so this is in my best interest.
4 Namespace pollution.
This will be fixed.
5 Upper case is ugly.
Is this really important? I am under the impression that it is customary=
for macros to be in upper case. The accessors are best implemented as
macros because functions are dependent on type, while macros can be made
type independent.
I have spent about an hour typing this email, time that I would rather
have spent sleeping or working on SMPng. Much of the cleanup that I
discussed would already be committed.
Thank you
Jake
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010111072428.B4955BA80>