[PATCH v2 1/2] Posix asynchronous I/O support: fhandler files

[Corinna Vinschen]

On Apr 19 01:04, Mark Geisert wrote:
> This code is where the AIO implementation is wired into existing Cygwin
> mechanisms for file and device I/O: the fhandler* functions.  It makes
> use of an existing internal routine prw_open to supply a "shadow fd"
> that permits asynchronous operations on a file the user app accesses
> via its own fd.  This allows AIO to read or write at arbitrary locations
> within a file without disturbing the app's file pointer.  (This was
> already the case with normal pread|pwrite; we're just adding "async"
> to the mix.)
> 
> 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.
> ---
>  winsup/cygwin/fhandler.cc           |  4 +-
>  winsup/cygwin/fhandler.h            | 10 ++---
>  winsup/cygwin/fhandler_disk_file.cc | 89 +++++++++++++++++++++++++------------
>  3 files changed, 67 insertions(+), 36 deletions(-)
> [...]
> diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
> index fc87d91c1..c9b231a31 100644
> --- a/winsup/cygwin/fhandler_disk_file.cc
> +++ b/winsup/cygwin/fhandler_disk_file.cc
> @@ -24,6 +24,7 @@ details. */
>  #include "tls_pbuf.h"
>  #include "devices.h"
>  #include "ldap.h"
> +#include <aio.h>
>  
>  #define _COMPILING_NEWLIB
>  #include <dirent.h>
> @@ -1511,28 +1512,34 @@ fhandler_base::open_fs (int flags, mode_t mode)
>     parameter to the latter. */
>  
>  int
> -fhandler_disk_file::prw_open (bool write)
> +fhandler_disk_file::prw_open (bool write, void *aio)
>  {
>    NTSTATUS status;
>    IO_STATUS_BLOCK io;
>    OBJECT_ATTRIBUTES attr;
> +  ULONG options = get_options ();
> +
> +  /* If async i/o is intended, turn off default synchronous operation option */
> +  //XXX prw_open() only called for first pread|pwrite on an fd. options persist!

Ouch, yes.  It's probably unlikely that anybody uses the same descriptor
for synchronous and asynchronous operations but it's not impossible.
Another (third) handle?  Sounds ugly.  Maybe it makes sense to just note
the usage of the handle and close and reopen it if the usage changes...

The rest of the patch looks good, but it just occured to me that there's
a problem I really ignored so far.  If we use pread/pwrite like this for
aio, aio is restricted to on-disk files.  No sockets, no pipes, no
nothing works with aio.

It seems we need top provide pread/pwrite API for other fhandlers as
well in the long run, evn if only for aio.  In the non-aio pread/pwrite
case, returning an ESPIPE error would still be the right thing to do.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20180419/12dca2ee/attachment.sig>