Date: Mon, 15 Nov 1999 11:55:52 +0100 From: Martin Cracauer <cracauer@cons.org> To: Marcel Moolenaar <marcel@scc.nl> Cc: Martin Cracauer <cracauer@cons.org>, Bruce Evans <bde@zeta.org.au>, freebsd-arch@freebsd.org Subject: Re: cvs commit: src/sys/i386/include signal.h Message-ID: <19991115115552.A27870@cons.org> In-Reply-To: <382FDBE2.D075594@scc.nl>; from Marcel Moolenaar on Mon, Nov 15, 1999 at 11:09:38AM %2B0100 References: <382E8A1B.B7D9B7F0@scc.nl> <Pine.BSF.4.10.9911142303390.21828-100000@alphplex.bde.org> <19991115095729.A27507@cons.org> <382FDBE2.D075594@scc.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
Marcel, Bruce,
I moved this to -arch, since we have to make some long-term decisions
here:
1) Does FreeBSD prefer to pass information like this as unnamed array
of bytes or as structs with proper fields?
2) How big may we make struct sigcontext? The 512 bytes that is named
for now is quite a big step from the 100 bytes it had before FPU
state savings).
I have a strong preference to anser 1) with "yes".
Be assurend that my comments aren't in a any way an attack on you, but
since we both committed to the signal code lately and I did my work
with 1 == YES and you with 1 == NO (partly removing type-save
constructions of my former code), we should resolve this now.
In <382FDBE2.D075594@scc.nl>, Marcel Moolenaar wrote:
> Martin Cracauer wrote:
>
> > > Martin implemented saving it before you complicated things by changing
> > > the signal handling :-). I didn't like it because it moved the definitions
> > > of i386 FP structs to the wrong place (<machine/signal.h>). It's not
> > > clear that there is a right place.
> >
> > The reasoning is like this:
> >
> > 0) If the state of the FPU is passed to a signal handler, it is
> > preferrable to do so in a struct with proper names, not just an
> > array of bytes.
>
> I disagree. Signal handlers don't use context information for (wild
> guess) 99.9% of the time. Bloating signalling definitions with details
> of the many different hardware configurations (even within a single
> architecture) is not the way to go.
Maybe we have a different definition of "bloat", but since the size is
the same, this doesn't count as "bloat for me.
> For every FP emulator with it's own
> state information, we are forced to have unions and the like.
The non-GPL emulator can't support a production FreeBSD system
anymore. The GPL emulator is reasonably compatible even when it comes
to its stored state.
> No, it's
> better to be vague in struct sigcontext and let the program use a cast
> when it wants to dig in the gory details of the machine state.
As I said, I disagree. FreeBSD/alpha also exposes its FPU state in
struct sigcontext (although the FPU is simpler).
> > 1) Applications that use signal handlers with extended parameters
> > (SA_SIGINFO or old BSD-style three arguments) are supposed to
> > include <signal.h>, but nothing else.
>
> SUSv2 defines sa_sigaction as:
> void (*) (int, siginfo_t*, void*) sa_sigaction;
>
> Notice the `void*' where the `ucontext_t*' is supposed to be. SUSv2
> simply says that the handler may cast the `void*' to get to the gory
> details and ucontext_t itself contains an abstract type (mcontext_t)
> that represents machine specific state.
When I implemented SA_SIGINFO, you could get it in a type-save way as
a member of the second argument. This capability has been removed by
your newer signal changes.
> Point: You don't want low-level details embedded in high-level
> definitions.
We pass the FPU state some way or another. If we pass it, we should do
it as type-save as possible.
A proper description of the i386 FPU is already present in the kernel
sources and used at other places, I just want to reuse the same
definition in a ny place where is FPU is represented in a stored
state.
> > struct save87 is now 176 bytes, Marcel currently reserves 180. struct
> > sigcontext without FPU state is 100 bytes, so we are at 276 bytes at
> > least.
> >
> > Would it be unreasonable to bump it to 512 bytes to be done with it
> > once and for all? Might have positive effects on cache behaviour as
> > well.
>
> In that case, make ucontext_t 512 bytes and scale struct sigcontext
> accordingly.
>
> > We may also need an way to tell an application at runtime how far the
> > struct is filled.
>
> You need exactly 1 bit to tell the application if the FPU state is valid
> or not, right?
I don't think that is sufficient in the general case.
We don't want to know whether a kernel that is capable of storing the
FPU state did so (it does so in any case). We want to know whether the
kernel is new enough that is even knows that the FPU state might be
stored here. If it doesn't it can't set this bit.
Longer explanation:
In a kernel version 1 'struct sigcontext' has a number of fields
making 100 bytes and padding to 512 bytes.
struct sigcontext {
char bla[100];
char padding[412];
};
In version 2, the FPU state is added and padding adjusted:
struct sigcontext {
char bla[100];
char fpu[200]
char padding[212];
};
In version 3, some other state is added:
struct sigcontext {
char bla[100];
char fpu[200]
cht otherstate[12]
char padding[200];
};
Now, how can an application tell what is filled and what is not? A
#define in the header file isn't suitable, since a binary compiled on
one version might be moved to a didfferent kernel.
An application might assume that a given state is always at the same
place, i.e. it might not happen that otherstate is stored where fpu
was assumed. Newer kernels add fields, they never move or delete
fields.
I think it is best to add a field n_extensions:
struct sigcontext {
char bla[100];
int n_extensions; /* 3 in this case */
char fpu[200]
chat otherstate[12]
char padding[200];
};
If an application wants to access one of the extensions, it must know
its number (via a #define) and then test for n_extensions >= N.
Martin
--
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Martin Cracauer <cracauer@cons.org> http://www.cons.org/cracauer/
Tel.: (private) +4940 5221829 Fax.: (private) +4940 5228536
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19991115115552.A27870>