From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3k7M-0000lT-HN for qemu-devel@nongnu.org; Tue, 01 Dec 2015 07:33:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3k7J-0003II-5y for qemu-devel@nongnu.org; Tue, 01 Dec 2015 07:33:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54014) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3k7J-0003IB-0d for qemu-devel@nongnu.org; Tue, 01 Dec 2015 07:33:09 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 1453F8E375 for ; Tue, 1 Dec 2015 12:33:07 +0000 (UTC) Date: Tue, 1 Dec 2015 20:32:52 +0800 From: Peter Xu Message-ID: <20151201123250.GA31109@pxdev.xzpeter.org> References: <1448883140-20249-1-git-send-email-peterx@redhat.com> <1448883140-20249-11-git-send-email-peterx@redhat.com> <565C478A.8080106@redhat.com> <20151201035755.GI21032@pxdev.xzpeter.org> <565D6E68.1060303@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <565D6E68.1060303@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 10/12] Dump: add qmp command "query-dump" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: drjones@redhat.com, lersek@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, famz@redhat.com, lcapitulino@redhat.com On Tue, Dec 01, 2015 at 10:54:48AM +0100, Paolo Bonzini wrote: > > > On 01/12/2015 04:57, Peter Xu wrote: > >> > You need a mutex around the reads of ->status and ->written_size. > > Could I avoid using mutex here? Let me try to explain what I > > thought. > > > > The concurrency of this should only happen when: > > > > - detached dump thread is working (dump thread) > > - user queries dump status (main thread) > > > > What the dump thread is doing should be something like: > > > > - [start dumping] > > - inc written_size > > - inc written_size > > - ... > > - inc written_size > > - set ->status to COMPLETED|FAILED > > - [end dumping] > > Yes, it's possible but you need to use atomic_mb_read/atomic_mb_set to > write ->status. Otherwise a CPU can see the write to ->status before > some of the final writes to ->written_size. Hi, Paolo, Thanks to point out. However, would it be confusing to use atomic_mb_{read|set} rather than directly use smp_rmb() and smp_wmb()? Like: In dump thread: - inc written_size - inc written_size - ... - inc written_size - smp_wmb() - atomic_set(status, COMPLETED|FAILED) In main thread: - atomic_read(status) - smp_rmb() - read written_size What I understand from the doc (seems written by you, thanks :) ) is that: atomic_mb_{read|set} is the pair of helper functions for _one_ specific variable, to make sure its operations are always in order as long as we are using atomic_mb_* functions to access it all the time. However, in the dump thread case, it's related to read/write order of two variables (status and written_size). Thanks! Peter > > Paolo