On Thu, Dec 4, 2014 at 10:37 PM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben:
>> When getting an error while submitting requests, we must be careful to
>> wake up only inactive coroutines. Therefore we must special-case the
>> currently active coroutine and communicate an error for that request
>> using the ordinary return value of ioq_submit().
>>
>> Signed-off-by: Kevin Wolf <kw...@redhat.com>
>> ---
>> block/linux-aio.c | 23 ++++++++++++++++-------
>> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> Ming, did you have a look at this patch specifically? Does it fix the
> issue that you tried to address with a much more complex callback-based
> patch?
I think your patch can't fix my issue.
As I explained, we have to handle -EAGAIN and partial submission,
which can be triggered quite easily in case of multi-queue, and other
case like NVME.
Thanks,
>
> I'd like to go forward with this as both Peter and I have measured
> considerable performance improvements with our optimisation proposals,
> and this series is an important part of it.
>
> Kevin
>
>
>> diff --git a/block/linux-aio.c b/block/linux-aio.c
>> index 99b259d..fd8f0e4 100644
>> --- a/block/linux-aio.c
>> +++ b/block/linux-aio.c
>> @@ -177,12 +177,19 @@ static int ioq_submit(struct qemu_laio_state *s)
>> container_of(s->io_q.iocbs[i], struct qemu_laiocb, iocb);
>>
>> laiocb->ret = (ret < 0) ? ret : -EIO;
>> - qemu_laio_process_completion(s, laiocb);
>> + if (laiocb->co != qemu_coroutine_self()) {
>> + qemu_coroutine_enter(laiocb->co, NULL);
>> + } else {
>> + /* The return value is used for the currently active coroutine.
>> + * We're always in ioq_enqueue() here, ioq_submit() never runs
>> from
>> + * a request's coroutine.*/
>> + ret = laiocb->ret;
>> + }
>> }
>> return ret;
>> }
>>
>> -static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
>> +static int ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
>> {
>> unsigned int idx = s->io_q.idx;
>>
>> @@ -191,7 +198,9 @@ static void ioq_enqueue(struct qemu_laio_state *s,
>> struct iocb *iocb)
>>
>> /* submit immediately if queue is full */
>> if (idx == s->io_q.size) {
>> - ioq_submit(s);
>> + return ioq_submit(s);
>> + } else {
>> + return 0;
>> }
>> }
>>
>> @@ -253,11 +262,11 @@ int laio_submit_co(BlockDriverState *bs, void
>> *aio_ctx, int fd,
>>
>> if (!s->io_q.plugged) {
>> ret = io_submit(s->ctx, 1, &iocbs);
>> - if (ret < 0) {
>> - return ret;
>> - }
>> } else {
>> - ioq_enqueue(s, iocbs);
>> + ret = ioq_enqueue(s, iocbs);
>> + }
>> + if (ret < 0) {
>> + return ret;
>> }
>>
>> qemu_coroutine_yield();
>> --
>> 1.8.3.1
>>
>
