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 <install> 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 0x04000000
#include <kernel_sigaction.h>
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 <sysdeps/unix/sysv/linux/sigaction.c>
---
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
