Date: Mon, 15 Nov 1999 13:08:54 +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: <19991115130854.A28445@cons.org> In-Reply-To: <382FF2AF.545C8C81@scc.nl>; from Marcel Moolenaar on Mon, Nov 15, 1999 at 12:46:55PM %2B0100 References: <382E8A1B.B7D9B7F0@scc.nl> <Pine.BSF.4.10.9911142303390.21828-100000@alphplex.bde.org> <19991115095729.A27507@cons.org> <382FDBE2.D075594@scc.nl> <19991115115552.A27870@cons.org> <382FF2AF.545C8C81@scc.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
In <382FF2AF.545C8C81@scc.nl>, Marcel Moolenaar wrote:
> Martin Cracauer wrote:
> >
> > 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".
>
> Do you want A or B? ... yes.
> Euh, what is it? :-)
> I'd say that we should find the balance. If more detail requires us to
> rearrange a lot of the headers in /sys/<ARCH>/include and /sys/sys, then
> I'd say we should use arrays.
Rearrange is the wrong word.
We move struct definitions that describe state that is passed to
signal handlers to include files included from <signal.h>.
The hole FPU include file structure cries for restructuring anyway, as
bruce will probably point out :-)
> > 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.
>
> The bloat is in what you get when you include <signal.h>
OK, you're speaking compile-time bloat here.
What I'm trying to do is to access members of a data struct by reusing
a struct definition that is already in our tree, and not requireing
the user to access the members by couting bytes and then cast.
That not bloat, that's moving struct definitions to where they are
needed.
> > > 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).
>
> Sort of:
> long sc_xxx2[3]; /* sc_fp_trap_pc, sc_fp_trigger_sum,
> sc_fp_trigger_inst */
>
> > > > 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.
>
> Yes, because ucontext_t is not part of siginfo_t and adding it there
> only creates unnecessary dependencies and pitfalls. Not to mention the
> size increase and the shear unportability of the approach.
Sorry, you are wrong here. It doesn't increase the amount of data that
is stored or copies for signal handlers.
Befor my changes, the struct sigcontext was pushed on the signal
handler argument stack as if it was a fith (hidden) argument, then the
(visible, documented) pointer passed as the thrird argument pointed to
that location.
After my changes, the struct was put into the struct sigcontext and
the third argument points back there.
That's just an rearrangement, not bloating anything.
As for unportable, it is of course unportable to use struct
sigcontext.
That doesn't mean it isn't useful to let the user access it without a
manual cast when he uses it.
> You don't want to implement RT signals with a bloated siginfo_t...
If the RT signals don't use a struct sigcontext at all (and I mean,
the kernel implementation doesn't need one when sending the signal and
returing from it, no matter whether the application's signal handler
can access it), then you can omit it.
> > > 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.
>
> As is *reasonably* 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.
>
> A change in how the state is saved would then automaticly result in an
> interface change while this should not be the case (I should know, I did
> exactly the same thing :-)
I don't understand why the fact the the description might not useful
in the emulator case prevents us from securing the case for the
hardware FPU case.
> > > > 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.
>
> If you refer to the general case here, then also do it for the saved
> state. In the general case, that may be very incompatible for each
> different FPU and or emulator, which is not limited to what we have now.
We're talking different levels of generality here. I'm talking about
more extensions to the "things" that a signal handler gets passed.
> > 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.
>
> Thus, a new kernel sets a bit where an old kernel does not set it...
>
> > 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.
>
> If a new kernel breaks old binaries, then it's not backwards compatible
> :-)
A newer kernel can't even break old binaries that fail to use this
scheme.
The scheme I propose is to support newer binaries on older kernels.
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?19991115130854.A28445>