This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
Re: [patch] aio_*() posix compliance
- From: Andreas Jaeger <aj at suse dot de>
- To: Amos Waterland <apw at us dot ibm dot com>
- Cc: libc-alpha at sources dot redhat dot com
- Date: Tue, 18 Jun 2002 14:52:11 +0200
- Subject: Re: [patch] aio_*() posix compliance
- References: <20020617123835.A362@kvasir.austin.ibm.com>
Amos Waterland <apw@us.ibm.com> writes:
> I have converted the code to the GNU coding standards, removed checks
> for null pointers, and removed checks for problems that the standard
> allows to be detected asynchronously. I grouped the remaining changes
> into a single patch, which does the following.
Thanks for the cleanup. For a perfect patch a ChangeLog entry is
missing that explains your changes. Could you come up with one,
please?
>
> aio_cancel()
> - detect bad file descriptor
> aio_error()
> - no changes
> aio_fsync()
> - detect bad file descriptor
> - detect fd not open for writing
> aio_read()
> - no changes
> aio_return()
> - no changes
> aio_suspend()
> - detect completed element(s) and do not suspend thread
> aio_write()
> - no changes
> lio_listio()
> - no changes (see comment below)
>
> I believe that glibc with this patch is fully compliant, with the
> possible exception of this issue: the standard says for lio_listio()
> (http://www.opengroup.org/onlinepubs/007904975/functions/lio_listio.html):
>
> The lio_listio() function shall fail if:
>
> [EAGAIN] The number of entries indicated by nent would cause the
> system-wide limit {AIO_MAX} to be exceeded.
>
> [EINVAL] The mode argument is not a proper value, or the value of
> nent was greater than {AIO_LISTIO_MAX}.
>
> Glibc does not seem to define AIO_MAX and AIO_LISTIO_MAX, and because of
> the way sysconf() conditionally compiles, it returns -1 for both (see
> below for test0024.c):
>
> % ./test0024
> + _POSIX_ASYNCHRONOUS_IO
> + _POSIX_ASYNC_IO
> - AIO_LISTIO_MAX
> - AIO_MAX
> (-1) _SC_AIO_LISTIO_MAX
> (-1) _SC_AIO_MAX
>
> The lio_listio() function does not check for nent exceeding the
> constants of concern. I know that there might be a simple answer to
> this, so I wanted to ask you all about it before I added definitions to
> unistd.h and patched lio_listio.c.
>
> Here is the patch.
>
> ---- Begin patch ----
>
> Index: sysdeps/pthread/aio_cancel.c
> ===================================================================
> RCS file: /cvs/glibc/libc/sysdeps/pthread/aio_cancel.c,v
> retrieving revision 1.2
> diff -u -r1.2 aio_cancel.c
> --- sysdeps/pthread/aio_cancel.c 6 Jul 2001 04:56:02 -0000 1.2
> +++ sysdeps/pthread/aio_cancel.c 16 Jun 2002 01:16:34 -0000
> @@ -43,6 +43,13 @@
> struct requestlist *req = NULL;
> int result = AIO_ALLDONE;
>
> + /* If fildes is invalid, error. */
> + if (fcntl (fildes, F_GETFL) < 0)
> + {
> + __set_errno (EBADF);
> + return -1;
> + }
> +
> /* Request the mutex. */
> pthread_mutex_lock (&__aio_requests_mutex);
>
> Index: sysdeps/pthread/aio_fsync.c
> ===================================================================
> RCS file: /cvs/glibc/libc/sysdeps/pthread/aio_fsync.c,v
> retrieving revision 1.2
> diff -u -r1.2 aio_fsync.c
> --- sysdeps/pthread/aio_fsync.c 6 Jul 2001 04:56:02 -0000 1.2
> +++ sysdeps/pthread/aio_fsync.c 16 Jun 2002 01:16:34 -0000
> @@ -36,15 +36,23 @@
> int
> aio_fsync (int op, struct aiocb *aiocbp)
> {
> - if (op != O_DSYNC && op != O_SYNC)
> + int flags;
> +
> + if ((flags = fcntl (aiocbp->aio_fildes, F_GETFL)) >= 0
> + && (flags & (O_RDWR | O_WRONLY)))
> {
> - __set_errno (EINVAL);
> - return -1;
> + if (op != O_DSYNC && op != O_SYNC)
> + {
> + __set_errno (EINVAL);
> + return -1;
> + }
> +
> + return (__aio_enqueue_request ((aiocb_union *) aiocbp,
> + op == O_SYNC ? LIO_SYNC : LIO_DSYNC) == NULL ? -1 : 0);
> }
>
> - return (__aio_enqueue_request ((aiocb_union *) aiocbp,
> - op == O_SYNC ? LIO_SYNC : LIO_DSYNC) == NULL
> - ? -1 : 0);
> + __set_errno (EBADF);
> + return -1;
> }
>
> weak_alias (aio_fsync, aio_fsync64)
> Index: sysdeps/pthread/aio_suspend.c
> ===================================================================
> RCS file: /cvs/glibc/libc/sysdeps/pthread/aio_suspend.c,v
> retrieving revision 1.2
> diff -u -r1.2 aio_suspend.c
> --- sysdeps/pthread/aio_suspend.c 6 Jul 2001 04:56:02 -0000 1.2
> +++ sysdeps/pthread/aio_suspend.c 16 Jun 2002 01:16:34 -0000
> @@ -49,6 +49,7 @@
> int result = 0;
> int dummy;
> int none = 1;
> + int do_not_suspend = 0;
>
> /* Request the mutex. */
> pthread_mutex_lock (&__aio_requests_mutex);
> @@ -56,8 +57,15 @@
> /* There is not yet a finished request. Signal the request that
> we are working for it. */
> for (cnt = 0; cnt < nent; ++cnt)
> - if (list[cnt] != NULL && list[cnt]->__error_code == EINPROGRESS)
> + if (list[cnt] != NULL)
> {
> + /* This element has already completed, so we must not suspend thread. */
> + if (list[cnt]->__error_code != EINPROGRESS)
> + {
> + do_not_suspend = 1;
> + continue;
> + }
> +
> requestlist[cnt] = __aio_find_req ((aiocb_union *) list[cnt]);
>
> if (requestlist[cnt] != NULL)
> @@ -83,7 +91,10 @@
> pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &oldstate);
>
> if (timeout == NULL)
> - result = pthread_cond_wait (&cond, &__aio_requests_mutex);
> + if (do_not_suspend)
> + result = 0;
> + else
> + result = pthread_cond_wait (&cond, &__aio_requests_mutex);
> else
> {
> /* We have to convert the relative timeout value into an
> @@ -100,8 +111,11 @@
> abstime.tv_sec += 1;
> }
>
> - result = pthread_cond_timedwait (&cond, &__aio_requests_mutex,
> - &abstime);
> + if (do_not_suspend)
> + result = 0;
> + else
> + result = pthread_cond_timedwait (&cond, &__aio_requests_mutex,
> + &abstime);
> }
>
> /* Now remove the entry in the waiting list for all requests
>
> ---- End patch ----
>
Last time you had a test program for all of these. I'd like to have
this program as part of glibc's regression suite. This means it
should live in rt as e.g. tst-aio-7.c and should return 0 in case of
success and 1 in case of an error. Can you reformat it and change the
program taking your patches in account and send it?
Thanks,
Andreas
--
Andreas Jaeger
SuSE Labs aj@suse.de
private aj@arthur.inka.de
http://www.suse.de/~aj