This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Initialize IO_STATUS_BLOCK for pread, pwrite


On Nov 29 02:25, Mark Geisert wrote:
> Corinna Vinschen wrote:
> > On Nov 28 02:28, Mark Geisert wrote:
> > > Corinna Vinschen wrote:
> > > > On Nov 28 00:03, Mark Geisert wrote:
> > > > > Oops, I neglected to include an explanatory comment. Issuing simultaneous
> > > > > pwrite(s) on one file descriptor from multiple threads, as one might do in a
> > > > > forthcoming POSIX aio implementation, sometimes results in garbage status in
> > > > > the IO_STATUS_BLOCK on return from NtWriteFile(). Zeroing beforehand made
> > > > > the issue go away.
> > > > > [...]
> > That doesn't mean it has been returned by NtWriteFile.  Random values
> > suggest NtWriteFile didn *set* a value in the first place, so what
> > you see is the random value from the stack position io is in.  And
> > that in turn suggests the status code should indicate why io wasn't
> > written by NtWriteFile.  If you're playing with async IO, is it possible
> > the status code indicates something like STATUS_TIMEOUT or STATUS_PENDING,
> > both of which are treated as NT_SUCCESS?
> > [...]
> I added the printf()s and, what do you know, it shows all the NtWriteFile()s
> are returning STATUS_PENDING.  On return some of the IO_STATUS_BLOCKS have the
> correct byte count but most of them have the same trash as before the call.

This is just a timing problem, see below.

> [...]
> Does this mean pwrite() should be waiting for the status to change from
> STATUS_PENDING to something else before returning?

MSDN has the following to say about this situation:

  The operating system implements support routines that write
  IO_STATUS_BLOCK values to caller-supplied output buffers. For example,
  see ZwOpenFile or NtOpenFile. These routines return status codes that
  might not match the status codes in the IO_STATUS_BLOCK structures. If
  one of these routines returns STATUS_PENDING, the caller should wait
  for the I/O operation to complete, and then check the status code in
  the IO_STATUS_BLOCK structure to determine the final status of the
  operation. If the routine returns a status code other than
  STATUS_PENDING, the caller should rely on this status code instead of
  the status code in the IO_STATUS_BLOCK structure.

STATUS_PENDING means the operation hasn't been finished yet, but that
only means that the IO_STATUS_BLOCK wasn't filled with correct information
when NtWriteFile returned.
^^^^^^^^^^^^^^^^^^^^^^^^^

So, after NtWriteFile returns with STATUS_PENDING it will still write
the completion status into the IO_STATUS_BLOCK parameter, as soon as it
finished its job, even if that takes a long time.

debug_printf of course takes time itself, so what happened in the
io.Infomation == 0x1000000 case was that NtWriteFile finished its job
while debug_printf was still collecting its wits.

The other NtWriteFile calls just didn't finish their job in time for
debug_printf to already get the completion status.

So what does that mean for us?

- If you don't do async IO on files, you're completely off the hook,
  because NtWriteFile will never return with STATUS_PENDING.

- If you do async IO, you have to handle STATUS_PENDING gracefully:

  - The IO_STATUS_BLOCK given to NtWriteFile *must* exist for the
    entire time the operation takes after NtWriteFile returned
    STATUS_PENDING.  An IO_STATUS_BLOCK fhandler member comes to mind,
    maybe fhandler_base_overlapped::io_status can be reused.

  - You *must* call NtWriteFile with an event object as 2nd parameter,
    which will be signalled when NtWriteFile completes (and then the
    io.Status member indicates success or failure).  Otherwise you have
    no chance to implement aio_error or any aio_sigevent method.

  - And of course, you don't wait in pwrite for completion.  That
    would be counter to the idea of async I/O.  Rather, STATUS_PENDING
    means returning -1 with errno set to EAGAIN.

    On the other hand, aio_write would return 0 in this case, so calling
    pwrite for the purpose of implementing aio_write shouldn't set
    errno.  I guess changing pread/pwrite to return a negative error
    value may be the better approach.  The caller would then turn this
    into setting errno and a valid return value to user space.

I'd like to suggest the Freenode IRC channel #cygwin-developers for
discussion if something's unclear.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]