All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: stefanha@redhat.com, qemu-devel@nongnu.org,
	qemu-block@nongnu.org, qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qed: fix bdrv_qed_drain
Date: Tue, 23 Feb 2016 11:43:55 +0100	[thread overview]
Message-ID: <56CC37EB.7050901@redhat.com> (raw)
In-Reply-To: <20160223055704.GC19080@ad.usersys.redhat.com>



On 23/02/2016 06:57, Fam Zheng wrote:
>>>> +        qed_cancel_need_check_timer(s);
>>>> +        qed_need_check_timer_cb(s);
>>>> +    }
>>>
>>> What if an allocating write is queued (the else branch case)? Its completion
>>> will be in bdrv_drain and it could arm the need_check_timer which is wrong.
>>>
>>> We need to drain the allocating_write_reqs queue before checking the timer.
>>
>> You're right, but how?  That's what bdrv_drain(bs) does, it's a
>> chicken-and-egg problem.
> 
> Maybe use an aio_poll loop before the if?

That would not change the fact that you're reimplementing bdrv_drain
inside bdrv_qed_drain.

Perhaps for now it's simplest to just remove the QED .bdrv_drain
callback, if you think this patch is not a good stopgap measure to avoid
the segmentation faults.

Once the bdrv_drain rework is in, we can move the callback _after_ I/O
is drained on bs and before it is drained on bs->file->bs.

Paolo

  reply	other threads:[~2016-02-23 10:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-16 15:53 [Qemu-devel] [PATCH] qed: fix bdrv_qed_drain Paolo Bonzini
2016-02-17  2:57 ` Fam Zheng
2016-02-17 11:28   ` Paolo Bonzini
2016-02-23  5:57     ` Fam Zheng
2016-02-23 10:43       ` Paolo Bonzini [this message]
2016-02-23 12:49         ` Fam Zheng
2016-02-23 13:54           ` Paolo Bonzini
2016-03-07 16:57             ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2016-03-07 20:56               ` Stefan Hajnoczi
2016-03-07 21:22                 ` [Qemu-devel] " Paolo Bonzini
2016-03-08  9:52                   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-03-08  9:59                     ` Kevin Wolf
2016-03-08  9:58                 ` Kevin Wolf
2016-03-09 15:37                   ` Stefan Hajnoczi
2016-03-07 20:57               ` Stefan Hajnoczi

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=56CC37EB.7050901@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=famz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.