Re: [PATCH v2] Adding userspace_libaio_reap option

[Jens Axboe <axboe@kernel.dk>]

On 2011-08-30 19:09, Daniel Ehrenberg wrote:
> On Tue, Aug 30, 2011 at 5:57 PM, Jens Axboe <axboe@kernel.dk> wrote:
>> On 2011-08-30 18:50, Dan Ehrenberg wrote:
>>> @@ -66,7 +107,16 @@ static int fio_libaio_getevents(struct thread_data *td, unsigned int min,
>>>       int r, events = 0;
>>>
>>>       do {
>>> -             r = io_getevents(ld->aio_ctx, actual_min, max, ld->aio_events + events, t);
>>> +             if (td->o.userspace_libaio_reap == 1
>>> +                 && actual_min == 0
>>> +                 && ((struct aio_ring *)(ld->aio_ctx))->magic
>>> +                             == AIO_RING_MAGIC) {
>>> +                     r = user_io_getevents(ld->aio_ctx, max,
>>> +                             ld->aio_events + events);
>>> +             } else {
>>> +                     r = io_getevents(ld->aio_ctx, actual_min,
>>> +                             max, ld->aio_events + events, t);
>>> +             }
>>
>> One question here - why depend on actual_min == 0? The check is
>> practically free, would not hurt to do the user_io_getevents() first and
>> punt to io_getevents() afterwards if need be.
> 
> I guess I don't need to depend on that, since you'll never be calling
> both user_io_getevents() and sys_io_getevents() at the same time
> still. It was just paranoia, really--if I'm not grabbing the ring_lock
> spinlock that exists in the kernel, then I'll be really safe if I
> never run code that wants to grab it. But I guess the condition is not
> really necessary since the whole thing is single-threaded.

I think it should be safe.

FWIW, I committed this version, so just base further improves on fio
git. I also half-assed a patch to add the sub-option support, and
converted this one. So you should use

ioengine=libaio:userspace_reap

to set it now. I say half-assed, since it's only really supported for
ioengine at the moment, and it's not included in --cmdhelp output. But
it should work, hopefully.

Please give it a spin :-)

-- 
Jens Axboe