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 globaldata, 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 write 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 understand
> 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 contents
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:
.globl gd_curproc
.globl _curproc
There were 2 .sets for each variable to give the value of the symbols:
.set gd_curproc,globaldata + GD_CURPROC
.set _curproc,globaldata + GD_CURPROC
Finally, there was an ASSYM for each variable in genassym.c to produce the
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 = 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 variables
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 that
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 ia64.
There would have been another 2 at least for arm and powerpc. I have plans
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 code
provides per-cpu variables. This will allow for optimizations in the future,
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 are
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-processor.
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>