From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60585) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDXAy-0000HU-Ml for qemu-devel@nongnu.org; Thu, 16 Jun 2016 09:17:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDXAw-0004mE-Mn for qemu-devel@nongnu.org; Thu, 16 Jun 2016 09:17:39 -0400 Received: from mail-vk0-x230.google.com ([2607:f8b0:400c:c05::230]:35047) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDXAw-0004m7-IC for qemu-devel@nongnu.org; Thu, 16 Jun 2016 09:17:38 -0400 Received: by mail-vk0-x230.google.com with SMTP id j2so72576676vkg.2 for ; Thu, 16 Jun 2016 06:17:38 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1465928228-1184-1-git-send-email-stefanha@redhat.com> <1465928228-1184-3-git-send-email-stefanha@redhat.com> From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 14:17:37 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v4 2/5] blockjob: add pause points List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Stefan Hajnoczi , qemu-devel , Kevin Wolf , Jeff Cody , Fam Zheng , "Jason J. Herne" , Max Reitz On Wed, Jun 15, 2016 at 9:53 AM, Paolo Bonzini wrote: > On 14/06/2016 20:17, Stefan Hajnoczi wrote: >> Block jobs are coroutines that usually perform I/O but sometimes also >> sleep or yield. Currently only sleeping or yielded block jobs can be >> paused. This means jobs that do not sleep or yield (using >> block_job_yield()) are unaffected by block_job_pause(). >> >> Add block_job_pause_point() so that block jobs can mark quiescent points >> that are suitable for pausing. This solves the problem that it can take >> a block job a long time to pause if it is performing a long series of >> I/O operations. >> >> Transitioning to paused state involves a .pause()/.resume() callback. >> These callbacks are used to ensure that I/O and event loop activity has >> ceased while the job is at a pause point. >> >> Note that this patch introduces a stricter pause state than previously. >> The job->busy flag was incorrectly documented as a quiescent state >> without I/O pending. This is violated by any job that has I/O pending >> across sleep or block_job_yield(), like the mirror block job. > > Right, we should document job->busy as a quiescent state where no one > will re-enter the coroutine. That statement doesn't correspond with how it's used: block_job_sleep_ns() leaves a timer pending and the job will re-enter when the timer expires. So "no one will re-enter the coroutine" is too strict. The important thing is it's safe to call block_job_enter(). In the block_job_sleep_ns() case the timer is cancelled to prevent doubly re-entry. The doc comment I have in v4 allows the block_job_sleep_ns() case: /* * Set to false by the job while the coroutine has yielded and may be * re-entered by block_job_enter(). There may still be I/O or event loop * activity pending. */ bool busy; Stefan