* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
[not found] <20190114161950.114763.qmail@sourceware.org>
@ 2019-01-15 9:51 ` Václav Haisman
2019-01-15 11:22 ` Corinna Vinschen
0 siblings, 1 reply; 6+ messages in thread
From: Václav Haisman @ 2019-01-15 9:51 UTC (permalink / raw)
To: cygwin-developers
On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
>
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=f42776fa781de858a927bc03aa966a0f3096b581
>
> commit f42776fa781de858a927bc03aa966a0f3096b581
> Author: Corinna Vinschen <corinna@vinschen.de>
> Date: Mon Jan 14 17:19:37 2019 +0100
>
> Cygwin: signalfd: implement non-polling select
>
> Allow the signal thread to recognize we're called in consequence of
> select on a signalfd. If the signal is part of the wait mask, don't
> call any signal handler and don't remove the signal from the queue,
> so a subsequent read (or sigwaitinfo/sigtimedwait) still gets the
> signal. Instead, just signal the event object at
> _cygtls::signalfd_select_wait for the thread running select.
>
> The addition of signalfd_select_wait to _cygtls unearthed the alignment
> problem of the context member again. To make sure this doesn't get lost,
> improve the related comment in the header file so that this (hopefully)
> doesn't get lost (again).
>
> Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> [...]
>
> diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
> index 39dba13..65a905c 100644
> --- a/winsup/cygwin/cygtls.h
> +++ b/winsup/cygwin/cygtls.h
> @@ -189,8 +189,11 @@ public:
> stack_t altstack;
> siginfo_t *sigwait_info;
> HANDLE signal_arrived;
> + HANDLE signalfd_select_wait;
> bool will_wait_for_signal;
> - long __align; /* Needed to align context to 16 byte. */
> + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
> + If you prepend cygtls members here, make sure context stays 16 byte
> + aligned. */
> ucontext_t context;
>[...]
Would it not better to do this with `ucontext_t context
__attribute__((__aligned__(16)));` instead?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
2019-01-15 9:51 ` [newlib-cygwin] Cygwin: signalfd: implement non-polling select Václav Haisman
@ 2019-01-15 11:22 ` Corinna Vinschen
2019-01-15 11:53 ` Corinna Vinschen
2019-01-16 14:09 ` Ryan Johnson
0 siblings, 2 replies; 6+ messages in thread
From: Corinna Vinschen @ 2019-01-15 11:22 UTC (permalink / raw)
To: Václav Haisman; +Cc: cygwin-developers
[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]
On Jan 15 10:50, Václav Haisman wrote:
> On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
> >
> > https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=f42776fa781de858a927bc03aa966a0f3096b581
> >
> > commit f42776fa781de858a927bc03aa966a0f3096b581
> > Author: Corinna Vinschen <corinna@vinschen.de>
> > Date: Mon Jan 14 17:19:37 2019 +0100
> >
> > Cygwin: signalfd: implement non-polling select
> >
> > Allow the signal thread to recognize we're called in consequence of
> > select on a signalfd. If the signal is part of the wait mask, don't
> > call any signal handler and don't remove the signal from the queue,
> > so a subsequent read (or sigwaitinfo/sigtimedwait) still gets the
> > signal. Instead, just signal the event object at
> > _cygtls::signalfd_select_wait for the thread running select.
> >
> > The addition of signalfd_select_wait to _cygtls unearthed the alignment
> > problem of the context member again. To make sure this doesn't get lost,
> > improve the related comment in the header file so that this (hopefully)
> > doesn't get lost (again).
> >
> > Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> > [...]
> >
> > diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
> > index 39dba13..65a905c 100644
> > --- a/winsup/cygwin/cygtls.h
> > +++ b/winsup/cygwin/cygtls.h
> > @@ -189,8 +189,11 @@ public:
> > stack_t altstack;
> > siginfo_t *sigwait_info;
> > HANDLE signal_arrived;
> > + HANDLE signalfd_select_wait;
> > bool will_wait_for_signal;
> > - long __align; /* Needed to align context to 16 byte. */
> > + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
> > + If you prepend cygtls members here, make sure context stays 16 byte
> > + aligned. */
> > ucontext_t context;
> >[...]
> Would it not better to do this with `ucontext_t context
> __attribute__((__aligned__(16)));` instead?
In theory yes, but it doesn't work. The simple perl parser in
gentls_offsets doesn't understand __attribute__ and screws up,
so generating tlsoffsets{64}.h fails.
I'm not good at perl. My attempts to fix the parser to let
__attribute__ statements slip through unchanged failed so far.
Right now what it does is:
ucontext_t context __attribute__((__aligned__(16)));
in the input is mangled to
ucontext_t context__attribute____aligned__16;
in the output. Any help appreciated.
Corinna
--
Corinna Vinschen
Cygwin Maintainer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
2019-01-15 11:22 ` Corinna Vinschen
@ 2019-01-15 11:53 ` Corinna Vinschen
2019-01-15 11:57 ` Corinna Vinschen
2019-01-16 14:09 ` Ryan Johnson
1 sibling, 1 reply; 6+ messages in thread
From: Corinna Vinschen @ 2019-01-15 11:53 UTC (permalink / raw)
To: Václav Haisman; +Cc: cygwin-developers
[-- Attachment #1: Type: text/plain, Size: 1615 bytes --]
On Jan 15 12:22, Corinna Vinschen wrote:
> On Jan 15 10:50, Václav Haisman wrote:
> > On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
> > > + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
> > > + If you prepend cygtls members here, make sure context stays 16 byte
> > > + aligned. */
> > > ucontext_t context;
> > >[...]
> > Would it not better to do this with `ucontext_t context
> > __attribute__((__aligned__(16)));` instead?
>
> In theory yes, but it doesn't work. The simple perl parser in
> gentls_offsets doesn't understand __attribute__ and screws up,
> so generating tlsoffsets{64}.h fails.
>
> I'm not good at perl. My attempts to fix the parser to let
> __attribute__ statements slip through unchanged failed so far.
> Right now what it does is:
>
> ucontext_t context __attribute__((__aligned__(16)));
>
> in the input is mangled to
>
> ucontext_t context__attribute____aligned__16;
>
> in the output. Any help appreciated.
Uhm... I managed to find a solution to keep __attribute__ expressions
in and to generate the code to compute the offsets. However, it has
no effect at all. I added a long variable which moves the address
by 8 bytes. The alignment should have moved context by 16 bytes, but
it doesn't.
Before:
//; $tls::context = -8624;
//; $tls::pcontext = 4176;
after:
//; $tls::foo = -8624;
//; $tls::pfoo = 4176;
//; $tls::context = -8616;
//; $tls::pcontext = 4184;
/scratching head/
Corinna
--
Corinna Vinschen
Cygwin Maintainer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
2019-01-15 11:53 ` Corinna Vinschen
@ 2019-01-15 11:57 ` Corinna Vinschen
0 siblings, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2019-01-15 11:57 UTC (permalink / raw)
To: Václav Haisman; +Cc: cygwin-developers
[-- Attachment #1: Type: text/plain, Size: 3884 bytes --]
On Jan 15 12:52, Corinna Vinschen wrote:
> On Jan 15 12:22, Corinna Vinschen wrote:
> > On Jan 15 10:50, Václav Haisman wrote:
> > > On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
> > > > + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
> > > > + If you prepend cygtls members here, make sure context stays 16 byte
> > > > + aligned. */
> > > > ucontext_t context;
> > > >[...]
> > > Would it not better to do this with `ucontext_t context
> > > __attribute__((__aligned__(16)));` instead?
> >
> > In theory yes, but it doesn't work. The simple perl parser in
> > gentls_offsets doesn't understand __attribute__ and screws up,
> > so generating tlsoffsets{64}.h fails.
> >
> > I'm not good at perl. My attempts to fix the parser to let
> > __attribute__ statements slip through unchanged failed so far.
> > Right now what it does is:
> >
> > ucontext_t context __attribute__((__aligned__(16)));
> >
> > in the input is mangled to
> >
> > ucontext_t context__attribute____aligned__16;
> >
> > in the output. Any help appreciated.
>
> Uhm... I managed to find a solution to keep __attribute__ expressions
> in and to generate the code to compute the offsets. However, it has
> no effect at all. I added a long variable which moves the address
> by 8 bytes. The alignment should have moved context by 16 bytes, but
> it doesn't.
>
> Before:
>
> //; $tls::context = -8624;
> //; $tls::pcontext = 4176;
>
> after:
>
> //; $tls::foo = -8624;
> //; $tls::pfoo = 4176;
> //; $tls::context = -8616;
> //; $tls::pcontext = 4184;
>
>
> /scratching head/
> Corinna
In the interest of full disclosure, here's the patch:
diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
index 65a905c32078..c60507231ae0 100644
--- a/winsup/cygwin/cygtls.h
+++ b/winsup/cygwin/cygtls.h
@@ -191,10 +191,11 @@ public:
HANDLE signal_arrived;
HANDLE signalfd_select_wait;
bool will_wait_for_signal;
+ long foo;
/* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
If you prepend cygtls members here, make sure context stays 16 byte
aligned. */
- ucontext_t context;
+ ucontext_t context __attribute__ ((aligned (16)));
DWORD thread_id;
siginfo_t infodata;
struct pthread *tid;
diff --git a/winsup/cygwin/gentls_offsets b/winsup/cygwin/gentls_offsets
index 745ea27a035c..1b42b6ceb9cd 100755
--- a/winsup/cygwin/gentls_offsets
+++ b/winsup/cygwin/gentls_offsets
@@ -26,22 +26,29 @@ substr($tls, 0, length($pre)) = '';
$pre .= "\n//*/";
$tls =~ s%/\*\s*gentls_offsets.*?/\*\s*gentls_offsets\s*\*/%%ogs;
foreach ($tls =~ /^.*\n/mg) {
- /^}|\s*(?:typedef|const)/o and do {
+ /^}|\s*(?:typedef|const)/ and do {
$def .= $_ ;
next;
};
$def .= $_ if $struct;
- if (!s/;.*$//o) {
- if (!$struct && /^\s*(?:struct|class)\s*([a-z_0-9]+)/o) {
+ if (!s/;.*$//) {
+ if (!$struct && /^\s*(?:struct|class)\s*([a-z_0-9]+)/) {
$def .= $_;
$struct = $1
}
next;
}
- s/(?:\[[^\]]*\]|struct|class)//o;
- s/^\s+\S+\s+//o;
- s/[\*\s()]+//go;
+ s/(?:\[[^\]]*\]|struct|class)//;
+ s/^\s+\S+\s+//;
+ if (! /__attribute__/) {
+ s/[\*\s()]+//g;
+ } else {
+ s/\s*$//g;
+ }
for my $f (split(/,/)) {
+ if ( $f =~ m/__attribute__/ ) {
+ $f =~ s/\s*__attribute__\s*\(\([^)]+\)\)+//
+ }
push(@fields, $f);
}
}
@@ -99,5 +106,5 @@ open OFFS, '-|', "/tmp/$$.a.out" or die "$0: couldn't run \"/tmp/$$.a.out\" - $!
print TLS_OUT <OFFS>;
close OFFS;
close TLS_OUT;
-unlink "/tmp/$$.cc", "/tmp/$$-1.cc", "/tmp/$$-1.d", "/tmp/$$.a.out";
+#unlink "/tmp/$$.cc", "/tmp/$$-1.cc", "/tmp/$$-1.d", "/tmp/$$.a.out";
exit(0);
Corinna
--
Corinna Vinschen
Cygwin Maintainer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
2019-01-15 11:22 ` Corinna Vinschen
2019-01-15 11:53 ` Corinna Vinschen
@ 2019-01-16 14:09 ` Ryan Johnson
2019-01-16 14:42 ` Corinna Vinschen
1 sibling, 1 reply; 6+ messages in thread
From: Ryan Johnson @ 2019-01-16 14:09 UTC (permalink / raw)
To: cygwin-developers
On 1/15/2019 4:22 AM, Corinna Vinschen wrote:
> On Jan 15 10:50, Václav Haisman wrote:
>> On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
>>> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=f42776fa781de858a927bc03aa966a0f3096b581
>>>
>>> commit f42776fa781de858a927bc03aa966a0f3096b581
>>> Author: Corinna Vinschen <corinna@vinschen.de>
>>> Date: Mon Jan 14 17:19:37 2019 +0100
>>>
>>> Cygwin: signalfd: implement non-polling select
>>>
>>> Allow the signal thread to recognize we're called in consequence of
>>> select on a signalfd. If the signal is part of the wait mask, don't
>>> call any signal handler and don't remove the signal from the queue,
>>> so a subsequent read (or sigwaitinfo/sigtimedwait) still gets the
>>> signal. Instead, just signal the event object at
>>> _cygtls::signalfd_select_wait for the thread running select.
>>>
>>> The addition of signalfd_select_wait to _cygtls unearthed the alignment
>>> problem of the context member again. To make sure this doesn't get lost,
>>> improve the related comment in the header file so that this (hopefully)
>>> doesn't get lost (again).
>>>
>>> Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
>>> [...]
>>>
>>> diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
>>> index 39dba13..65a905c 100644
>>> --- a/winsup/cygwin/cygtls.h
>>> +++ b/winsup/cygwin/cygtls.h
>>> @@ -189,8 +189,11 @@ public:
>>> stack_t altstack;
>>> siginfo_t *sigwait_info;
>>> HANDLE signal_arrived;
>>> + HANDLE signalfd_select_wait;
>>> bool will_wait_for_signal;
>>> - long __align; /* Needed to align context to 16 byte. */
>>> + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
>>> + If you prepend cygtls members here, make sure context stays 16 byte
>>> + aligned. */
>>> ucontext_t context;
>>> [...]
>> Would it not better to do this with `ucontext_t context
>> __attribute__((__aligned__(16)));` instead?
> In theory yes, but it doesn't work. The simple perl parser in
> gentls_offsets doesn't understand __attribute__ and screws up,
> so generating tlsoffsets{64}.h fails.
Why not introduce a typedef in some convenient location the perl script
doesn't examine, e.g.
typedef ucontext_t __attribute__((aligned(16))) aligned_ucontext_t;
And then use that in the struct?
Ryan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [newlib-cygwin] Cygwin: signalfd: implement non-polling select
2019-01-16 14:09 ` Ryan Johnson
@ 2019-01-16 14:42 ` Corinna Vinschen
0 siblings, 0 replies; 6+ messages in thread
From: Corinna Vinschen @ 2019-01-16 14:42 UTC (permalink / raw)
To: Ryan Johnson; +Cc: cygwin-developers
[-- Attachment #1: Type: text/plain, Size: 2901 bytes --]
On Jan 16 07:09, Ryan Johnson wrote:
> On 1/15/2019 4:22 AM, Corinna Vinschen wrote:
> > On Jan 15 10:50, Václav Haisman wrote:
> > > On Mon, 14 Jan 2019 at 17:19, Corinna Vinschen <corinna@sourceware.org> wrote:
> > > > https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=f42776fa781de858a927bc03aa966a0f3096b581
> > > >
> > > > commit f42776fa781de858a927bc03aa966a0f3096b581
> > > > Author: Corinna Vinschen <corinna@vinschen.de>
> > > > Date: Mon Jan 14 17:19:37 2019 +0100
> > > >
> > > > Cygwin: signalfd: implement non-polling select
> > > >
> > > > Allow the signal thread to recognize we're called in consequence of
> > > > select on a signalfd. If the signal is part of the wait mask, don't
> > > > call any signal handler and don't remove the signal from the queue,
> > > > so a subsequent read (or sigwaitinfo/sigtimedwait) still gets the
> > > > signal. Instead, just signal the event object at
> > > > _cygtls::signalfd_select_wait for the thread running select.
> > > >
> > > > The addition of signalfd_select_wait to _cygtls unearthed the alignment
> > > > problem of the context member again. To make sure this doesn't get lost,
> > > > improve the related comment in the header file so that this (hopefully)
> > > > doesn't get lost (again).
> > > >
> > > > Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> > > > [...]
> > > >
> > > > diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
> > > > index 39dba13..65a905c 100644
> > > > --- a/winsup/cygwin/cygtls.h
> > > > +++ b/winsup/cygwin/cygtls.h
> > > > @@ -189,8 +189,11 @@ public:
> > > > stack_t altstack;
> > > > siginfo_t *sigwait_info;
> > > > HANDLE signal_arrived;
> > > > + HANDLE signalfd_select_wait;
> > > > bool will_wait_for_signal;
> > > > - long __align; /* Needed to align context to 16 byte. */
> > > > + /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
> > > > + If you prepend cygtls members here, make sure context stays 16 byte
> > > > + aligned. */
> > > > ucontext_t context;
> > > > [...]
> > > Would it not better to do this with `ucontext_t context
> > > __attribute__((__aligned__(16)));` instead?
> > In theory yes, but it doesn't work. The simple perl parser in
> > gentls_offsets doesn't understand __attribute__ and screws up,
> > so generating tlsoffsets{64}.h fails.
>
> Why not introduce a typedef in some convenient location the perl script
> doesn't examine, e.g.
>
> typedef ucontext_t __attribute__((aligned(16))) aligned_ucontext_t;
>
> And then use that in the struct?
>
> Ryan
Great idea... it just doesn't work either, same as
https://cygwin.com/ml/cygwin-developers/2019-01/msg00006.html
Corinna
--
Corinna Vinschen
Cygwin Maintainer
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-16 14:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190114161950.114763.qmail@sourceware.org>
2019-01-15 9:51 ` [newlib-cygwin] Cygwin: signalfd: implement non-polling select Václav Haisman
2019-01-15 11:22 ` Corinna Vinschen
2019-01-15 11:53 ` Corinna Vinschen
2019-01-15 11:57 ` Corinna Vinschen
2019-01-16 14:09 ` Ryan Johnson
2019-01-16 14:42 ` Corinna Vinschen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).