From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2HZf-0006fW-Td for qemu-devel@nongnu.org; Fri, 27 Nov 2015 06:52:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2HZa-0002ER-TI for qemu-devel@nongnu.org; Fri, 27 Nov 2015 06:52:23 -0500 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]:36891) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2HZa-0002EI-Mn for qemu-devel@nongnu.org; Fri, 27 Nov 2015 06:52:18 -0500 Received: by wmww144 with SMTP id w144so55328060wmw.0 for ; Fri, 27 Nov 2015 03:52:18 -0800 (PST) Sender: Paolo Bonzini References: <1448592497-2462-1-git-send-email-peterx@redhat.com> <1448592497-2462-4-git-send-email-peterx@redhat.com> <565830E4.90301@redhat.com> <20151127112643.GB10191@pxdev.xzpeter.org> From: Paolo Bonzini Message-ID: <565843F1.50003@redhat.com> Date: Fri, 27 Nov 2015 12:52:17 +0100 MIME-Version: 1.0 In-Reply-To: <20151127112643.GB10191@pxdev.xzpeter.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: qemu-devel@nongnu.org On 27/11/2015 12:26, Peter Xu wrote: > On Fri, Nov 27, 2015 at 11:31:00AM +0100, Paolo Bonzini wrote: >> > > [snip] > >>> + >>> +static GlobalDumpState *dump_state_get_global(void) >>> +{ >>> + static bool init = false; >>> + static GlobalDumpState global_dump_state; >>> + if (unlikely(!init)) { >>> + qemu_mutex_init(&global_dump_state.gds_mutex); >> >> The mutex is not necessary. The structure is always created in the main >> thread and released by the dump thread (of which there is just one). > > [1] > >> >> You can then just make a global DumpState (not a pointer!), and separate >> the fields between: >> >> - those that are protected by a mutex (most likely the DumpResult and >> written_size, possibly others) > > Hi, Paolo, > > So mutex is still necessary, right? (refer to [1]) Since > "dump-query" will read several fields of it, while the dump thread > might be modifying them as well? Right, initially I meant a mutex around the allocation. But then I read the other patches where you add more fields, and came back to add more content. Strictly speaking it's likely that you can do everything without a mutex, but it's easier to use one. > Ok, I think I can remove the global state and make a static > DumpState. When I was drafting the patch, I just tried to keep the > old logic (malloc/free) and avoid introducing bugs. Maybe I was > wrong. I should better not introduce new struct if not necessary. It's okay. The only confusing bit was that you had to add more state to GlobalDumpState, and in the end it was split between DumpState and GlobalDumpState. As far as this patch is concerned, using malloc/free would have been okay, but then the subsequent changes suggest a different approach. Thanks! Paolo > Will try to follow this example in v3. > > Thanks. > Peter > >> >> Paolo >> > >