> On 17 Jan 2019, at 16:22, Maxim Dounin <mdounin@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@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@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@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@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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel