On 06/01/2014 03:44 AM, johnlumby wrote:
> If you look at the new patch, you'll see that for the different-pid case,
> I still call aio_suspend with a timeout.
> As you or Claudio pointed out earlier, it could just as well sleep
> for the same timeout,
> but the small advantage of calling aio_suspend is if the io completed
> just between
> the aio_error returning EINPROGRESS and the aio_suspend call.
> Also it makes the code simpler. In fact this change is quite small,
> just a few lines
> in backend/storage/buffer/buf_async.c and backend/storage/file/fd.c
>
> Based on this, I think it is not necessary to get rid of the polling
> altogether
> (and in any case, as far as I can see, very difficult).
That's still just as wrong as it always has been. Just get rid of it.
Don't put aiocb structs in shared memory at all. They don't belong there.
>>>> Well, as mentioned earlier, it is not broken. Whether it is
>>>> efficient I am not sure.
>>>> I have looked at the mutex in aio_suspend that you mentioned and I am not
>>>> quite convinced that, if caller is not the original aio_read process,
>>>> it renders the suspend() into an instant timeout. I will see if I can
>>>> verify that.
>>>
>>> I don't see the point of pursuing this design further. Surely we don't want
>>> to use polling here, and you're relying on undefined behavior anyway. I'm
>>> pretty sure aio_return/aio_error won't work from a different process on all
>>> platforms, even if it happens to work on Linux. Even on Linux, it might stop
>>> working if the underlying implementation changes from the glibc pthread
>>> emulation to something kernel-based.
>
> Good point. I have included the guts of your little test program
> (modified to do polling) into the existing autoconf test program
> that decides on the
> #define USE_AIO_ATOMIC_BUILTIN_COMP_SWAP.
> See config/c-library.m4.
> I hope this goes some way to answer your concern about robustness,
> as at least now if the implementation changes in some way that
> renders the polling ineffective, it will be caught in configure.
No, that does not make it robust enough.
- Heikki
