Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/21/18 4:05 AM, Adhemerval Zanella wrote: >> Thing is ARC signal return depends on a restorer. Meaning !SA_RESTORER >> doesn't >> effectively exist (between libc and kernel). We currently honor user provided >> value, instead we could simply overwrite it with default_rt_restorer and on >> the >> way out, zero it out to avoid leaking the glibc guts to curious users. >> > Yes, this is what I suggest ARC should do. So I was able to (1) switch ARC to __sigset_t [2] vs. [128] (2) create a generic patch to breakout struct sigaction into bits/sigaction-arch.h (3) update generic __libc_sigaction to avoid itemized copy if sa_mask, sa_flags et al offsets matched for struct kernel_signal and struct sigaction (4) create arc/bits/sigaction-arch.h However it turned out to be futile as the passthru won't work anyways :-( @act in sigaction(sig, act, oact) is a const and can't be modified in place for sticking in sa_restorer for kernel, thus needs to be copied over anyways. This means we have [1] but have to drop [2-4]. Oh well ! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
* Vineet Gupta: > On 12/20/18 4:40 AM, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> The only advantage of using a larger sigset_t from glibc standpoint is if >>> kernel ever change it maximum number of supported signals it would not be >>> a ABI change (it would be if glibc provided sigset_t need to be extended). >> It's not just the kernel. We might want to restore additional state in >> sigsetjmp, and historically, the excess signal space in sigset_t has >> provided a way to do that even if there is no other space left in the >> jump buffer. > > And that additional state is architectural state (more FPU regs etc > etc) or something libc generic. On both counts, that doesn't sound > like a clean interface design ! Sure, setjmp/longjmp is quite horrible as an interface, considering that register files do evolve. But it's what we have in C, and it's good to have at least some extension space there (but it could be expressed in a better way, definitely). Thanks, Florian ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 20/12/2018 18:46, Vineet Gupta wrote:
> On 12/20/18 12:06 PM, Adhemerval Zanella wrote:
>>
>>
>> On 20/12/2018 17:23, Vineet Gupta wrote:
>>> On 12/20/18 3:19 AM, Adhemerval Zanella wrote:
>> #define SET_SA_RESTORER(kact, act) \
>> (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
>> (kact)->sa_restorer = &__default_rt_sa_restorer
> +#define SET_SA_RESTORER(kact, act) \
> + ({ \
> + if (!((kact)->sa_flags & SA_RESTORER))\
> + { \
> + (kact)->sa_restorer = __default_rt_sa_restorer; \
> + (kact)->sa_flags |= SA_RESTORER; \
> + } \
> + else \
> + (kact)->sa_restorer = (act)->sa_restorer; \
> + })
What is so special about ARC sa_restorer that an application should provide
an specialized one?
>>>
>>> Its the other way around. Only if application provides it's own restorer,
>>> we honor
>>> it, otherwise default restorer is used. This logic goes back many many
>>> years ago
>>> to how ARC uClibc did this and likely inherited it from the port it was
>>> copied
>>> from. But I don't know if say POSIX allows apps to provide their own
>>> restorer or
>>> not. We could very well remove it; although per other discussion if we
>>> intend to
>>> use the same struct sigaction for kernel/userland, the placeholder would
>>> exist and
>>> we could choose to just ignore it.
>>>
>>
>> The 'should' should be indeed 'might' in my question. And SA_RESTORER is a
>> Linux
>> specific ABI not intended to be used by applications,
>
> What do u mean here. It is ABI between userspace and kernel, but not
> necessarily
> between applications and glibc, although the same "carrier" sa_flags is in
> play ?
> IOW, SA_RESTORER is not intended to be set by applications, but only by glibc
> ?
AFAIK SA_RESTORER is meant to be used by libc or alike environments to setup
the required code to handle signal handling, it should not be up to programs
to mess up with sa_restorer to provide custom routines.
>
>> so my question is in fact
>> what kind of specialized sa_restorer applications might provided that would
>> require the libc to honour it.
>
> Indeed, applications should not be allowed to change it. The exact signature
> of
> default sigreturn is hardwired in signal unwinding etc etc and any deviation
> from
> that is recipe for issues: but like I mentioned I didn't invent that
> interface for
> ARC.
So my suggestion is not allow user code to mess with sa_restore regardless
if sa_flags contain SA_RESTORER. If kernel allows it to be zero in case of
!SA_RESTORER, set it to zero as other architectures. If it requires an
special routine, add an arch-specific one within glibc.
>
>> The placeholder existence is not an issue itself (POSIX states the minimum
>> fields sigaction should provide, not its specific fields neither their
>> layout).
>
> OK !
>
Can't it follow other abi where they issue __NR_sigreturn
for !SA_SIGINFO or __NR_rt_sigreturn otherwise?
>>>
>>> With Linux UAPI ABI, __NR_sigreturn doesn't exist, but your concern is about
>>> restorer ?
>>
>> Right, so ARC at least is not pulling old compat stuff. Is it safe to just
>> zero
>> the sa_restorer for sa_flags without SA_RESTORER?
>
> Thing is ARC signal return depends on a restorer. Meaning !SA_RESTORER doesn't
> effectively exist (between libc and kernel). We currently honor user provided
> value, instead we could simply overwrite it with default_rt_restorer and on
> the
> way out, zero it out to avoid leaking the glibc guts to curious users.
>
Yes, this is what I suggest ARC should do.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/20/18 12:06 PM, Adhemerval Zanella wrote:
>
>
> On 20/12/2018 17:23, Vineet Gupta wrote:
>> On 12/20/18 3:19 AM, Adhemerval Zanella wrote:
> #define SET_SA_RESTORER(kact, act) \
> (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
> (kact)->sa_restorer = &__default_rt_sa_restorer
+#define SET_SA_RESTORER(kact, act)\
+ ({ \
+ if (!((kact)->sa_flags & SA_RESTORER)) \
+ {\
+ (kact)->sa_restorer = __default_rt_sa_restorer;\
+ (kact)->sa_flags |= SA_RESTORER; \
+ }\
+ else \
+ (kact)->sa_restorer = (act)->sa_restorer;\
+ })
>>> What is so special about ARC sa_restorer that an application should provide
>>> an specialized one?
>>
>> Its the other way around. Only if application provides it's own restorer, we
>> honor
>> it, otherwise default restorer is used. This logic goes back many many years
>> ago
>> to how ARC uClibc did this and likely inherited it from the port it was
>> copied
>> from. But I don't know if say POSIX allows apps to provide their own
>> restorer or
>> not. We could very well remove it; although per other discussion if we
>> intend to
>> use the same struct sigaction for kernel/userland, the placeholder would
>> exist and
>> we could choose to just ignore it.
>>
>
> The 'should' should be indeed 'might' in my question. And SA_RESTORER is a
> Linux
> specific ABI not intended to be used by applications,
What do u mean here. It is ABI between userspace and kernel, but not necessarily
between applications and glibc, although the same "carrier" sa_flags is in play
?
IOW, SA_RESTORER is not intended to be set by applications, but only by glibc ?
> so my question is in fact
> what kind of specialized sa_restorer applications might provided that would
> require the libc to honour it.
Indeed, applications should not be allowed to change it. The exact signature of
default sigreturn is hardwired in signal unwinding etc etc and any deviation
from
that is recipe for issues: but like I mentioned I didn't invent that interface
for
ARC.
> The placeholder existence is not an issue itself (POSIX states the minimum
> fields sigaction should provide, not its specific fields neither their
> layout).
OK !
>>> Can't it follow other abi where they issue __NR_sigreturn
>>> for !SA_SIGINFO or __NR_rt_sigreturn otherwise?
>>
>> With Linux UAPI ABI, __NR_sigreturn doesn't exist, but your concern is about
>> restorer ?
>
> Right, so ARC at least is not pulling old compat stuff. Is it safe to just
> zero
> the sa_restorer for sa_flags without SA_RESTORER?
Thing is ARC signal return depends on a restorer. Meaning !SA_RESTORER doesn't
effectively exist (between libc and kernel). We currently honor user provided
value, instead we could simply overwrite it with default_rt_restorer and on the
way out, zero it out to avoid leaking the glibc guts to curious users.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 20/12/2018 17:23, Vineet Gupta wrote:
> On 12/20/18 3:19 AM, Adhemerval Zanella wrote:
#define SET_SA_RESTORER(kact, act) \
(kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
(kact)->sa_restorer = &__default_rt_sa_restorer
>>> +#define SET_SA_RESTORER(kact, act) \
>>> + ({\
>>> + if (!((kact)->sa_flags & SA_RESTORER)) \
>>> + { \
>>> + (kact)->sa_restorer = __default_rt_sa_restorer; \
>>> + (kact)->sa_flags |= SA_RESTORER;\
>>> + } \
>>> + else\
>>> + (kact)->sa_restorer = (act)->sa_restorer; \
>>> + })
>> What is so special about ARC sa_restorer that an application should provide
>> an specialized one?
>
> Its the other way around. Only if application provides it's own restorer, we
> honor
> it, otherwise default restorer is used. This logic goes back many many years
> ago
> to how ARC uClibc did this and likely inherited it from the port it was copied
> from. But I don't know if say POSIX allows apps to provide their own restorer
> or
> not. We could very well remove it; although per other discussion if we intend
> to
> use the same struct sigaction for kernel/userland, the placeholder would
> exist and
> we could choose to just ignore it.
>
The 'should' should be indeed 'might' in my question. And SA_RESTORER is a Linux
specific ABI not intended to be used by applications, so my question is in fact
what kind of specialized sa_restorer applications might provided that would
require the libc to honour it.
The placeholder existence is not an issue itself (POSIX states the minimum
fields sigaction should provide, not its specific fields neither their
layout).
>
>> Can't it follow other abi where they issue __NR_sigreturn
>> for !SA_SIGINFO or __NR_rt_sigreturn otherwise?
>
> With Linux UAPI ABI, __NR_sigreturn doesn't exist, but your concern is about
> restorer ?
Right, so ARC at least is not pulling old compat stuff. Is it safe to just zero
the sa_restorer for sa_flags without SA_RESTORER?
>
>> As for other architecture I do think we should hide the sa_restorer usage
>> from application.
>
> As mentioned above, the placeholder could exist, we can choose to ignore it.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/20/18 4:07 AM, Arnd Bergmann wrote: > All users of sigset_t that I could find in the kernel just check > the size argument to ensure it matches _NSIG, and return > EINVAL otherwise. Indeed that was one of the very first problem ARC glibc port ran into with stock sizeof(sigset_t) passed in sigaction syscall. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/20/18 3:19 AM, Adhemerval Zanella wrote:
>>> #define SET_SA_RESTORER(kact, act) \
>>> (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
>>> (kact)->sa_restorer = &__default_rt_sa_restorer
>> +#define SET_SA_RESTORER(kact, act) \
>> + ({ \
>> + if (!((kact)->sa_flags & SA_RESTORER)) \
>> + { \
>> + (kact)->sa_restorer = __default_rt_sa_restorer; \
>> + (kact)->sa_flags |= SA_RESTORER; \
>> + } \
>> + else \
>> + (kact)->sa_restorer = (act)->sa_restorer; \
>> + })
> What is so special about ARC sa_restorer that an application should provide
> an specialized one?
Its the other way around. Only if application provides it's own restorer, we
honor
it, otherwise default restorer is used. This logic goes back many many years ago
to how ARC uClibc did this and likely inherited it from the port it was copied
from. But I don't know if say POSIX allows apps to provide their own restorer or
not. We could very well remove it; although per other discussion if we intend to
use the same struct sigaction for kernel/userland, the placeholder would exist
and
we could choose to just ignore it.
> Can't it follow other abi where they issue __NR_sigreturn
> for !SA_SIGINFO or __NR_rt_sigreturn otherwise?
With Linux UAPI ABI, __NR_sigreturn doesn't exist, but your concern is about
restorer ?
> As for other architecture I do think we should hide the sa_restorer usage
> from application.
As mentioned above, the placeholder could exist, we can choose to ignore it.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/20/18 4:40 AM, Florian Weimer wrote: > * Adhemerval Zanella: > >> The only advantage of using a larger sigset_t from glibc standpoint is if >> kernel ever change it maximum number of supported signals it would not be >> a ABI change (it would be if glibc provided sigset_t need to be extended). > It's not just the kernel. We might want to restore additional state in > sigsetjmp, and historically, the excess signal space in sigset_t has > provided a way to do that even if there is no other space left in the > jump buffer. And that additional state is architectural state (more FPU regs etc etc) or something libc generic. On both counts, that doesn't sound like a clean interface design ! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
* Adhemerval Zanella: > The only advantage of using a larger sigset_t from glibc standpoint is if > kernel ever change it maximum number of supported signals it would not be > a ABI change (it would be if glibc provided sigset_t need to be extended). It's not just the kernel. We might want to restore additional state in sigsetjmp, and historically, the excess signal space in sigset_t has provided a way to do that even if there is no other space left in the jump buffer. Thanks, Florian ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On Thu, Dec 20, 2018 at 12:19 PM Adhemerval Zanella
wrote:
> On 19/12/2018 20:23, Vineet Gupta wrote:
> > On 12/19/18 2:00 PM, Adhemerval Zanella wrote:
> >> One possibility is to define an arch-specific __sigset_t.h with a custom
> >> _SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
> >> if both kernel_sigaction and glibc sigaction share same size and internal
> >> layout to use the struct as-is for syscall instead of copy to a temporary
> >> value (similar to what we used to have on getdents). ARC would still have
> >> arch-specific code and would be the only ABI to have a different sigset_t
> >> though.
> >
> > I don't like ARC to single out either. But as Joseph suggests could this be
> > starting point for arches of future. Assuming it does, would rather see
> > this or
> > the approach Joseph alluded to earlier [1]
> >
> > [1]
> > http://lists.infradead.org/pipermail/linux-snps-arc/2018-December/005122.html
> >
> >>
> >> However I *hardly* think sigaction is a hotspot in real world cases,
> >> usually
> >> the signal action is defined once or in a very limited number of times. I
> >> am
> >> not considering synthetic benchmarks, specially lmbench which in most cases
> >> does not measure any useful metric.
> >
> > I tend to disagree. Coming from embedded linux background, I've found it
> > immensely
> > useful to compare 2 minimal systems: especially when distos, out-of-box
> > packages,
> > fancy benchmarks don't even exist.
> >
> > At any rate, my disagreement with status quo is not so much of optimize for
> > ARC,
> > but rather pointless spending of electrons. When we know that there are 64
> > signals
> > at max, which need 64-bits, why bother shuffling around 1k bits, specially
> > when
> > one is starting afresh and there's no legacy crap getting in the way.
> >
> > The case of adding more signals in future is indeed theoretically possible
> > but
> > that will be an ABI change anyways.
>
> The only advantage of using a larger sigset_t from glibc standpoint is if
> kernel ever change it maximum number of supported signals it would not be
> a ABI change (it would be if glibc provided sigset_t need to be extended).
>
> My point was that this change would hardly help in performance or memory
> utilization due the signal set common utilization in exported and internal
> API. But at the same time the signal set hasn't been changed for a *long*
> time
> and I don't see indication that it will any time soon. So a new architecture
> might indeed assume it won't change and set its default to follow Linux user
> ABI.
I just checked again what we actually use in the kernel.
With very few exceptions, all architectures in current kernels use
#define _NSIG 64
#define _NSIG_WORDS (_NSIG / _NSIG_BPW)
typedef struct {
/* this sucks for big-endian 32-bit compat mode */
unsigned long sig[_NSIG_WORDS];
} sigset_t;
The only two exceptions I see are:
- alpha uses a scalar 'unsigned long' instead of the struct+array, but
the effect is the same layout.
- For unknown reasons, all three MIPS ABIs now use _NSIG=128.
This changed during linux-2.1.x from 65, to 32 and then the current
value...
All users of sigset_t that I could find in the kernel just check
the size argument to ensure it matches _NSIG, and return
EINVAL otherwise.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 19/12/2018 20:23, Vineet Gupta wrote:
> On 12/19/18 2:00 PM, Adhemerval Zanella wrote:
>>
>>
>> One possibility is to define an arch-specific __sigset_t.h with a custom
>> _SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
>> if both kernel_sigaction and glibc sigaction share same size and internal
>> layout to use the struct as-is for syscall instead of copy to a temporary
>> value (similar to what we used to have on getdents). ARC would still have
>> arch-specific code and would be the only ABI to have a different sigset_t
>> though.
>
> I don't like ARC to single out either. But as Joseph suggests could this be
> starting point for arches of future. Assuming it does, would rather see this
> or
> the approach Joseph alluded to earlier [1]
>
> [1]
> http://lists.infradead.org/pipermail/linux-snps-arc/2018-December/005122.html
>
>>
>> However I *hardly* think sigaction is a hotspot in real world cases, usually
>> the signal action is defined once or in a very limited number of times. I am
>> not considering synthetic benchmarks, specially lmbench which in most cases
>> does not measure any useful metric.
>
> I tend to disagree. Coming from embedded linux background, I've found it
> immensely
> useful to compare 2 minimal systems: especially when distos, out-of-box
> packages,
> fancy benchmarks don't even exist.
>
> At any rate, my disagreement with status quo is not so much of optimize for
> ARC,
> but rather pointless spending of electrons. When we know that there are 64
> signals
> at max, which need 64-bits, why bother shuffling around 1k bits, specially
> when
> one is starting afresh and there's no legacy crap getting in the way.
>
> The case of adding more signals in future is indeed theoretically possible but
> that will be an ABI change anyways.
The only advantage of using a larger sigset_t from glibc standpoint is if
kernel ever change it maximum number of supported signals it would not be
a ABI change (it would be if glibc provided sigset_t need to be extended).
My point was that this change would hardly help in performance or memory
utilization due the signal set common utilization in exported and internal
API. But at the same time the signal set hasn't been changed for a *long* time
and I don't see indication that it will any time soon. So a new architecture
might indeed assume it won't change and set its default to follow Linux user
ABI.
>
>> Even for other sigset_t usage case I still
>> think an arch-specific definition should not make much difference:
>>
>> 1. setcontext: it would incur just in larger ucontext_t (kernel get/set
>> mask is done using kernel expected size). Also, taking in consideration
>> these interfaces were removed from POSIX.1-2008, the inherent
>> performance
>> issues (signal set/restore will most likely dominate the overhead), and
>> some implementation issues (BZ#18135 for instance), I would say to not
>> bother to optimize it.
>>
>> 2. pselect, ppoll, epoll_pwait, posix_spawn (posix_spawnattr_t), sig*:
>> for functions that accept sigset as an argument it would incur in just
>> larger memory utilization without much performance overhead. Again,
>> runtime for these calls would be mostly dominate by syscall overhead
>> or kernel wait-time for events.
>>
>> 3. raise, etc: for function that might allocate a sigset_t internally it
>> will similar to 2.
>
> I agree that that in libc, pretty much anything will be dominated by syscall
> overhead, but still...
>
>
>> Now, if ARC intention is just to follow generic glibc linux ABI definitions,
>> it could just define its sigaction as (not tested):
>
> Indeed the ABI is etched in stone and I have a very similar code now, with
> slight
> difference.
>
>> * sysdeps/unix/sysv/linux/arc/sigaction.c
>>
>> ---
>> #define SA_RESTORER 0x0400
>> #include
>>
>> extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
>>
>> #define SET_SA_RESTORER(kact, act) \
>> (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
>> (kact)->sa_restorer = &__default_rt_sa_restorer
>
> +#define SET_SA_RESTORER(kact, act) \
> + ({ \
> + if (!((kact)->sa_flags & SA_RESTORER))\
> + { \
> + (kact)->sa_restorer = __default_rt_sa_restorer; \
> + (kact)->sa_flags |= SA_RESTORER; \
> + } \
> + else \
> + (kact)->sa_restorer = (act)->sa_restorer; \
> + })
What is so special about ARC sa_restorer that an application should provide
an specialized one? Can't it follow other abi where they issue __NR_sigreturn
for !SA_SIGINFO or __NR_rt_sigret
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 12/19/18 2:00 PM, Adhemerval Zanella wrote:
>
>
> One possibility is to define an arch-specific __sigset_t.h with a custom
> _SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
> if both kernel_sigaction and glibc sigaction share same size and internal
> layout to use the struct as-is for syscall instead of copy to a temporary
> value (similar to what we used to have on getdents). ARC would still have
> arch-specific code and would be the only ABI to have a different sigset_t
> though.
I don't like ARC to single out either. But as Joseph suggests could this be
starting point for arches of future. Assuming it does, would rather see this or
the approach Joseph alluded to earlier [1]
[1]
http://lists.infradead.org/pipermail/linux-snps-arc/2018-December/005122.html
>
> However I *hardly* think sigaction is a hotspot in real world cases, usually
> the signal action is defined once or in a very limited number of times. I am
> not considering synthetic benchmarks, specially lmbench which in most cases
> does not measure any useful metric.
I tend to disagree. Coming from embedded linux background, I've found it
immensely
useful to compare 2 minimal systems: especially when distos, out-of-box
packages,
fancy benchmarks don't even exist.
At any rate, my disagreement with status quo is not so much of optimize for ARC,
but rather pointless spending of electrons. When we know that there are 64
signals
at max, which need 64-bits, why bother shuffling around 1k bits, specially when
one is starting afresh and there's no legacy crap getting in the way.
The case of adding more signals in future is indeed theoretically possible but
that will be an ABI change anyways.
> Even for other sigset_t usage case I still
> think an arch-specific definition should not make much difference:
>
> 1. setcontext: it would incur just in larger ucontext_t (kernel get/set
> mask is done using kernel expected size). Also, taking in consideration
> these interfaces were removed from POSIX.1-2008, the inherent performance
> issues (signal set/restore will most likely dominate the overhead), and
> some implementation issues (BZ#18135 for instance), I would say to not
> bother to optimize it.
>
> 2. pselect, ppoll, epoll_pwait, posix_spawn (posix_spawnattr_t), sig*:
> for functions that accept sigset as an argument it would incur in just
> larger memory utilization without much performance overhead. Again,
> runtime for these calls would be mostly dominate by syscall overhead
> or kernel wait-time for events.
>
> 3. raise, etc: for function that might allocate a sigset_t internally it
> will similar to 2.
I agree that that in libc, pretty much anything will be dominated by syscall
overhead, but still...
> Now, if ARC intention is just to follow generic glibc linux ABI definitions,
> it could just define its sigaction as (not tested):
Indeed the ABI is etched in stone and I have a very similar code now, with
slight
difference.
> * sysdeps/unix/sysv/linux/arc/sigaction.c
>
> ---
> #define SA_RESTORER 0x0400
> #include
>
> extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
>
> #define SET_SA_RESTORER(kact, act) \
> (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
> (kact)->sa_restorer = &__default_rt_sa_restorer
+#define SET_SA_RESTORER(kact, act) \
+ ({\
+ if (!((kact)->sa_flags & SA_RESTORER)) \
+ { \
+ (kact)->sa_restorer = __default_rt_sa_restorer; \
+ (kact)->sa_flags |= SA_RESTORER;\
+ } \
+ else\
+ (kact)->sa_restorer = (act)->sa_restorer; \
+ })
>
> #define RESET_SA_RESTORER(act, kact)\
> (act)->sa_restorer = (kact)->sa_restorer
>
> static void __default_rt_sa_restorer(void)
> {
> INTERNAL_SYSCALL_DECL (err);
> INTERNAL_SYSCALL_CALL (__NR_rt_sigreturn, err);
> }
>
> #include
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On 19/12/2018 15:58, Vineet Gupta wrote:
> On 12/18/18 6:39 PM, Vineet Gupta wrote:
diff --git a/sysdeps/unix/sysv/linux/arc/sigaction.c
b/sysdeps/unix/sysv/linux/arc/sigaction.c
>>> Why do you need this, rather than using the unified version (possibly with
>>> a few macros defined first)?
>>
>> The only syscall ABI requirement is that we pass our our own SA_RESTORER stub
>> (rather than inject one in kernel, and deal with cache sync etc etc). Indeed
>> the
>> common code can be used - likely was not the case when I started with ARC
>> port, or
>> more likely the port that I started ARC port from ;-)
>>
>> I'll update this.
>
> I took a stab at this but not really happy with taking this approach.
>
> (1). Common code assumes disparate kernel and userland sigaction struct even
> though there's no reason for a *new* port to be: its not like all glibc code
> is
> shared/common although I agree it is best to do so as much as possible
> So this requires explicit copy over of 1 struct into other, when it could have
> been avoided altogether for some cases atleast (!SA_RESTORER).
>
> (2) Linux kernel asm-generic syscall ABI expects sigset_t to be 2 words
>
> | kernel: include/uapi/asm-generic/signal.h
> |
> | #define _NSIG 64
> | typedef struct {
> | unsigned long sig[_NSIG_WORDS];
> | } sigset_t;
>
> And that is what we pretend at the time of syscall itself, e.g. snippet below
> from
> generic sigaction()
>
> | /* XXX The size argument hopefully will have to be changed to the
> | real size of the user-level sigset_t. */
> | result = INLINE_SYSCALL_CALL (rt_sigaction, sig,
> | act ? &kact : NULL,
> | oact ? &koact : NULL, STUB(act) _NSIG / 8);
> ^
>
> However glibc assumes sigset_to to be 128 words which is fine, however the
> memcpy
> for 256 words seems pointless when kernel is not supposed to be even looking
> at
> beyond 2nd word (although I realize my SA_RESTORE case was doing the implicit
> copy
> as well)
>
> (3) Consider me a micro-optimization freak :-) but this memcpy seems waste of
> cycles and will atleast show up LMBench lat_sig micro-benchmarks.
>
>
> I don't have strong feelings either ways, but wanted to express my concerns
> anyways.
>
One possibility is to define an arch-specific __sigset_t.h with a custom
_SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
if both kernel_sigaction and glibc sigaction share same size and internal
layout to use the struct as-is for syscall instead of copy to a temporary
value (similar to what we used to have on getdents). ARC would still have
arch-specific code and would be the only ABI to have a different sigset_t
though.
However I *hardly* think sigaction is a hotspot in real world cases, usually
the signal action is defined once or in a very limited number of times. I am
not considering synthetic benchmarks, specially lmbench which in most cases
does not measure any useful metric. Even for other sigset_t usage case I still
think an arch-specific definition should not make much difference:
1. setcontext: it would incur just in larger ucontext_t (kernel get/set
mask is done using kernel expected size). Also, taking in consideration
these interfaces were removed from POSIX.1-2008, the inherent performance
issues (signal set/restore will most likely dominate the overhead), and
some implementation issues (BZ#18135 for instance), I would say to not
bother to optimize it.
2. pselect, ppoll, epoll_pwait, posix_spawn (posix_spawnattr_t), sig*:
for functions that accept sigset as an argument it would incur in just
larger memory utilization without much performance overhead. Again,
runtime for these calls would be mostly dominate by syscall overhead
or kernel wait-time for events.
3. raise, etc: for function that might allocate a sigset_t internally it
will similar to 2.
Now, if ARC intention is just to follow generic glibc linux ABI definitions,
it could just define its sigaction as (not tested):
* sysdeps/unix/sysv/linux/arc/sigaction.c
---
#define SA_RESTORER 0x0400
#include
extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
#define SET_SA_RESTORER(kact, act) \
(kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
(kact)->sa_restorer = &__default_rt_sa_restorer
#define RESET_SA_RESTORER(act, kact)\
(act)->sa_restorer = (kact)->sa_restorer
static void __default_rt_sa_restorer(void)
{
INTERNAL_SYSCALL_DECL (err);
INTERNAL_SYSCALL_CALL (__NR_rt_sigreturn, err);
}
#include
---
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On Wed, 19 Dec 2018, Vineet Gupta wrote: > I took a stab at this but not really happy with taking this approach. > > (1). Common code assumes disparate kernel and userland sigaction struct even > though there's no reason for a *new* port to be: its not like all glibc code > is > shared/common although I agree it is best to do so as much as possible > So this requires explicit copy over of 1 struct into other, when it could have > been avoided altogether for some cases atleast (!SA_RESTORER). So make the generic code optimize those cases based on appropriate conditionals (making sure to verify those conditionals are right for every architecture in glibc). Any new architecture having much architecture-specific code for the kernel interface in glibc, beyond the basic definitions of how to call a syscall, is suspect, given that the kernel structures should be consistent across asm-generic architectures; we ought to make the defaults work so they are genuinely suitable as defaults for new architectures. This may require changes to the sysdeps/unix/sysv/linux/ code if it's currently "generic" to old architectures but not so good for asm-generic ones. -- Joseph S. Myers jos...@codesourcery.com ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
