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
signature.asc
Description: PGP signature
