aio/unix: Use signal.sival which is standard

[Sergey Kandaurov]

> On 17 Jan 2019, at 16:22, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Thu, Jan 17, 2019 at 01:29:56PM +0300, Sergey Kandaurov wrote:
> 
>> 
>> 
>>> On 17 Jan 2019, at 08:43, Sepherosa Ziehau <sepherosa at gmail.com> wrote:
>>> 
>>> What's the preferred way to handle this?  I am not sure whether you
>>> guys allow __FreeBSD_version testing etc.
>>> 
>> 
>> This could be solved with autotests.
> 
> As long as we only care about different FreeBSD versions, this 
> might as well be an auto/os/freebsd test based on $version, or 
> just a define based on __FreeBSD_version in 
> src/os/unix/ngx_freebsd_config.h.
> 
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet at nginx.com>
>> # Date 1547720890 0
>> #      Thu Jan 17 10:28:10 2019 +0000
>> # Node ID d28513cd71bce227b4e159b7a3f518aa504232f0
>> # Parent  6d15e452fa2eaf19408e24a0d0fcc3a31344a289
>> Fixed portability issues with union sigval.
>> 
>> The sival_ptr field is now preferably used.
>> 
>> diff --git a/auto/unix b/auto/unix
>> --- a/auto/unix
>> +++ b/auto/unix
>> @@ -523,6 +523,7 @@ if [ $NGX_FILE_AIO = YES ]; then
>>     ngx_feature_libs=
>>     ngx_feature_test="struct aiocb  iocb;
>>                       iocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
>> +                      iocb.aio_sigevent.sigev_value.sival_ptr = NULL;
>>                       (void) aio_read(&iocb)"
>>     . auto/feature
>> 
>> @@ -532,6 +533,22 @@ if [ $NGX_FILE_AIO = YES ]; then
>> 
>>     if [ $ngx_found = no ]; then
>> 
>> +        ngx_feature="kqueue AIO support (legacy)"
>> +        ngx_feature_name="NGX_HAVE_FILE_AIO_LEGACY"
>> +        ngx_feature_test="struct aiocb  iocb;
>> +                          iocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
>> +                          iocb.aio_sigevent.sigev_value.sigval_ptr = NULL;
>> +                          (void) aio_read(&iocb)"
>> +        . auto/feature
> 
> Note that this will test for "kqueue AIO support (legacy)" on each 
> Linux build with aio.

That is unfortunate.

>> +
>> +        if [ $ngx_found = yes ]; then
>> +            CORE_SRCS="$CORE_SRCS $FILE_AIO_SRCS"
>> +            have=NGX_HAVE_FILE_AIO . auto/have
>> +        fi
>> +    fi
>> +
>> +    if [ $ngx_found = no ]; then
>> +
>>         ngx_feature="Linux AIO support"
>>         ngx_feature_name="NGX_HAVE_FILE_AIO"
>>         ngx_feature_run=no
>> diff --git a/src/os/unix/ngx_file_aio_read.c b/src/os/unix/ngx_file_aio_read.c
>> --- a/src/os/unix/ngx_file_aio_read.c
>> +++ b/src/os/unix/ngx_file_aio_read.c
>> @@ -110,8 +110,12 @@ ngx_file_aio_read(ngx_file_t *file, u_ch
>> #if (NGX_HAVE_KQUEUE)
>>     aio->aiocb.aio_sigevent.sigev_notify_kqueue = ngx_kqueue;
>>     aio->aiocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
>> +#if !(NGX_HAVE_FILE_AIO_LEGACY)
>> +    aio->aiocb.aio_sigevent.sigev_value.sival_ptr = ev;
>> +#else
>>     aio->aiocb.aio_sigevent.sigev_value.sigval_ptr = ev;
>> #endif
>> +#endif
>>     ev->handler = ngx_file_aio_event_handler;
>> 
>>     n = aio_read(&aio->aiocb);
> 
> A simplier solution might be to always use sival_ptr, and define 
> it to sigval_ptr on old FreeBSDs.

This looks simpler indeed.

> Alternatively, we can consider dropping file AIO support for these 
> old FreeBSD versions.  This shouldn't be a big deal as this is an 
> optional feature which is not enabled by default.

And that's what I'd prefer for --test-build-eventport workaround.

What about these patches?
For DragonFly this should effectively match initial submission.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1547734251 0
#      Thu Jan 17 14:10:51 2019 +0000
# Node ID baab2b35e8cc79cfdf9924d1752348e97c1da13e
# Parent  6d15e452fa2eaf19408e24a0d0fcc3a31344a289
Fixed portability issues with union sigval.

diff --git a/src/os/unix/ngx_file_aio_read.c b/src/os/unix/ngx_file_aio_read.c
--- a/src/os/unix/ngx_file_aio_read.c
+++ b/src/os/unix/ngx_file_aio_read.c
@@ -110,7 +110,7 @@ ngx_file_aio_read(ngx_file_t *file, u_ch
 #if (NGX_HAVE_KQUEUE)
     aio->aiocb.aio_sigevent.sigev_notify_kqueue = ngx_kqueue;
     aio->aiocb.aio_sigevent.sigev_notify = SIGEV_KEVENT;
-    aio->aiocb.aio_sigevent.sigev_value.sigval_ptr = ev;
+    aio->aiocb.aio_sigevent.sigev_value.sival_ptr = ev;
 #endif
     ev->handler = ngx_file_aio_event_handler;
 
diff --git a/src/os/unix/ngx_freebsd_config.h b/src/os/unix/ngx_freebsd_config.h
--- a/src/os/unix/ngx_freebsd_config.h
+++ b/src/os/unix/ngx_freebsd_config.h
@@ -91,6 +91,10 @@
 #if (NGX_HAVE_FILE_AIO)
 #include <aio.h>
 typedef struct aiocb  ngx_aiocb_t;
+
+#if (__FreeBSD_version < 700005 && !defined __DragonFly__)
+#define sival_ptr                 sigval_ptr
+#endif
 #endif
 
 
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1547736673 0
#      Thu Jan 17 14:51:13 2019 +0000
# Node ID c66911fc9924a60bb5d691ca00bc2fb1c3032866
# Parent  baab2b35e8cc79cfdf9924d1752348e97c1da13e
Removed --test-build-eventport workaround for old FreeBSD versions.

diff --git a/src/event/modules/ngx_eventport_module.c b/src/event/modules/ngx_eventport_module.c
--- a/src/event/modules/ngx_eventport_module.c
+++ b/src/event/modules/ngx_eventport_module.c
@@ -250,9 +250,7 @@ ngx_eventport_init(ngx_cycle_t *cycle, n
 
         ngx_memzero(&sev, sizeof(struct sigevent));
         sev.sigev_notify = SIGEV_PORT;
-#if !(NGX_TEST_BUILD_EVENTPORT)
         sev.sigev_value.sival_ptr = &pn;
-#endif
 
         if (timer_create(CLOCK_REALTIME, &sev, &event_timer) == -1) {
             ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_errno,


-- 
Sergey Kandaurov