qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Ming Lei <ming.lei@canonical.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 2/3] linux-aio: handling -EAGAIN for !s->io_q.plugged case
Date: Mon, 24 Nov 2014 12:54:58 +0100	[thread overview]
Message-ID: <54731C92.9070402@redhat.com> (raw)
In-Reply-To: <CACVXFVOOctpmf=Haq296oP-N6K9e5YKeJN4xBmTuFDp1jAB=hA@mail.gmail.com>



On 24/11/2014 12:49, Ming Lei wrote:
> On Mon, Nov 24, 2014 at 7:38 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 24/11/2014 12:31, Ming Lei wrote:
>>> +    /* don't submit until next completion for -EAGAIN of non plug case */
>>> +    if (unlikely(!s->io_q.plugged)) {
>>> +        return 0;
>>> +    }
>>> +
>>>      /* submit immediately if queue depth is above 2/3 */
>>>      if (idx > s->io_q.size * 2 / 3) {
>>>          return ioq_submit(s);
>>       }
>>
>>       return 0;
>>
>> so I fail to see why my proposal:
>>
>>
>>     /* This is reached in two cases: queue not plugged but io_submit
>>      * returned -EAGAIN, or queue plugged.  In the latter case, start
>>      * submitting some I/O if the queue is getting too full.  In the
>>      * former case, instead, wait until an I/O operation is completed.
>>      */
>>     if (likely(idx <= s->io_q.size * 2 / 3) || unlikely(!s->io_q.plugged) {
>>         return 0;
>>     }
>>
>>     return ioq_submit(s);
>>
>> was wrong.  Can you explain?
> 
> I didn't say your proposal is wrong, and this patch is correct too
> without fat comment.
> 
> The difference is only that this patch returns immediately in case
> of !s->io_q.plugged after putting the req into io queue.

There is no difference in the behavior of the code, right?

The maintainers can decide if they want a v5 of this patch.

Thanks,

Paolo

  reply	other threads:[~2014-11-24 11:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-24 11:31 [Qemu-devel] [PATCH v4 0/3] linux-aio: fix batch submission Ming Lei
2014-11-24 11:31 ` [Qemu-devel] [PATCH v4 1/3] linux-aio: fix submit aio as a batch Ming Lei
2014-11-24 11:39   ` Paolo Bonzini
2014-11-24 11:51     ` Ming Lei
2014-11-24 11:31 ` [Qemu-devel] [PATCH v4 2/3] linux-aio: handling -EAGAIN for !s->io_q.plugged case Ming Lei
2014-11-24 11:38   ` Paolo Bonzini
2014-11-24 11:49     ` Ming Lei
2014-11-24 11:54       ` Paolo Bonzini [this message]
2014-11-24 11:31 ` [Qemu-devel] [PATCH v4 3/3] linux-aio: remove 'node' from 'struct qemu_laiocb' Ming Lei
2014-11-24 11:40 ` [Qemu-devel] [PATCH v4 0/3] linux-aio: fix batch submission Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54731C92.9070402@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=ming.lei@canonical.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).