Re: [PATCH v2 3/3] Posix asynchronous I/O support: other files

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Apr 19 01:04, Mark Geisert wrote:
> Updates to misc files to integrate AIO into the Cygwin source tree.
> Much of it has to be done when adding any new syscalls.  There are
> some updates to limits.h for AIO-specific limits.  And some doc mods.
> 
> This is the 2nd WIP patch set for AIO.  The string XXX marks issues
> I'm specifically requesting comments on, but feel free to comment or
> suggest changes on any of this code.

A few notes:


> ---
>  winsup/cygwin/Makefile.in              |  1 +
>  winsup/cygwin/common.din               |  8 ++++++++
>  winsup/cygwin/include/cygwin/version.h |  4 +++-
>  winsup/cygwin/include/limits.h         | 12 ++++++------
>  winsup/cygwin/thread.cc                |  4 ++--
>  winsup/doc/posix.xml                   | 16 ++++++++--------
>  6 files changed, 28 insertions(+), 17 deletions(-)
> [...]
> diff --git a/winsup/cygwin/include/limits.h b/winsup/cygwin/include/limits.h
> index fe1b8b493..b52ca11f2 100644
> --- a/winsup/cygwin/include/limits.h
> +++ b/winsup/cygwin/include/limits.h
> @@ -147,7 +147,7 @@ details. */
>  
>  /* Runtime Invariant Values */
>  
> -/* Please note that symbolic names shall be ommited, on specific
> +/* Please note that symbolic names shall be omitted, on specific
>     implementations where the corresponding value is equal to or greater
>     than the stated minimum, but is unspecified.  This indetermination
>     might depend on the amount of available memory space on a specific
> @@ -155,17 +155,17 @@ details. */
>     a specific instance shall be provided by the sysconf() function. */
>  
>  /* Maximum number of I/O operations in a single list I/O call supported by
> -   the implementation.  Not yet implemented. */
> -#undef AIO_LISTIO_MAX
> +   the implementation. */
> +#define AIO_LISTIO_MAX 32
>  /* #define AIO_LISTIO_MAX >= _POSIX_AIO_LISTIO_MAX */
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   Please remove this comment, it's as outdated as the one preceeding the
   macro.

>  /* Maximum number of outstanding asynchronous I/O operations supported by
> -   the implementation.  Not yet implemented. */
> -#undef AIO_MAX
> +   the implementation. */
> +#define AIO_MAX 8
>  /*  #define AIO_MAX >= _POSIX_AIO_MAX */
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   Ditto.

Btw., this isn't quite enough to be POSIX compliant.  You'll have to
fix the return value of sysconf(2) for _SC_AIO_LISTIO_MAX and
_SC_AIO_MAX as well.

>  /* The maximum amount by which a process can decrease its asynchronous I/O
> -   priority level from its own scheduling priority. */
> +   priority level from its own scheduling priority. Not yet implemented. */
>  #undef AIO_PRIO_DELTA_MAX
>  /* #define AIO_PRIO_DELTA_MAX >= 0 */

Wouldn't it make sense to set AIO_PRIO_DELTA_MAX (_SC_AIO_PRIO_DELTA_MAX)
to 0 now?  As soon as aio is implemented, we have a aio_reqprio field
and that may be filled with some value.  The value is supposed to be
between 0 and AIO_PRIO_DELTA_MAX.  Given there's no POSIX requirement
for a minimum value, 0 is as much valid as any value > 0.

This could be revisited later, if we ever allow different prios.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature