From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52806) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adYrt-0002vd-Bf for qemu-devel@nongnu.org; Wed, 09 Mar 2016 02:49:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adYrs-0004Oi-EC for qemu-devel@nongnu.org; Wed, 09 Mar 2016 02:49:17 -0500 Sender: Paolo Bonzini References: <1455645388-32401-1-git-send-email-pbonzini@redhat.com> <1455645388-32401-8-git-send-email-pbonzini@redhat.com> <20160309034126.GF17947@ad.usersys.redhat.com> From: Paolo Bonzini Message-ID: <56DFD573.608@redhat.com> Date: Wed, 9 Mar 2016 08:49:07 +0100 MIME-Version: 1.0 In-Reply-To: <20160309034126.GF17947@ad.usersys.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 07/16] block: change drain to look only at one child at a time List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com On 09/03/2016 04:41, Fam Zheng wrote: > > bdrv_requests_pending is checking children to also wait until internal > > requests (such as metadata writes) have completed. However, checking > > children is in general overkill because, apart from this special case, > > the parent's in_flight count will always be incremented by at least one > > for every request in the child. > > While the patch looks good, I'm not sure I understand this: aren't children's > requests generated by the child itself, how is parent's in_flight incremented? What I mean is that there are two cases of bs generating I/O on bs->file->bs: 1) writes caused by an operation on bs, e.g. a bdrv_aio_write to bs. In this case, the bdrv_aio_write increments bs's in_flight count 2) asynchronous metadata writes or flushes. Such writes can be started even if bs's in_flight count is zero, but not after the .bdrv_drain callback has been invoked. So the correct sequence is: - finish current I/O - call bdrv_drain callback - recurse on children This is what is needed for QED's callback to work correctly, and I'll implement it this way in v2. Paolo