From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D7BC1C433F5 for ; Tue, 28 Sep 2021 16:58:28 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 73A9761266 for ; Tue, 28 Sep 2021 16:58:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 73A9761266 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:32988 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mVGR1-0000tS-Fj for qemu-devel@archiver.kernel.org; Tue, 28 Sep 2021 12:58:27 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:42534) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mVGQ0-0007hN-Ij for qemu-devel@nongnu.org; Tue, 28 Sep 2021 12:57:24 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:23999) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mVGPy-0007E2-5U for qemu-devel@nongnu.org; Tue, 28 Sep 2021 12:57:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632848240; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references; bh=8995sLfyPilVCGx1LFPD1dmOhK1Lb1elOqW/RzJxJBA=; b=E17LDAnNo5juoJ8W7EnbTegKAHK71HNRpB3W5MM+RYQI4TIwjyYrfjq73NXxSB4HYfxuzJ GzhP3sGQoZY+FkDg9G5gjxjJyFVlkih6+95iwEHLCIqBfR+zUCaMz6jkwipD/vVPLLOHhf oaUTy69UxU7UdsNKiLQkxKBTQ8J2cVk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-490-VZhuPFnnOACRdIH7dfRBUA-1; Tue, 28 Sep 2021 12:57:19 -0400 X-MC-Unique: VZhuPFnnOACRdIH7dfRBUA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 11D606A7; Tue, 28 Sep 2021 16:57:18 +0000 (UTC) Received: from redhat.com (unknown [10.39.192.82]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E1CE719736; Tue, 28 Sep 2021 16:56:44 +0000 (UTC) Date: Tue, 28 Sep 2021 17:56:41 +0100 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: David Hildenbrand Subject: Re: [PATCH v3 6/7] util/oslib-posix: Support concurrent os_mem_prealloc() invocation Message-ID: References: <20210816094739.21970-1-david@redhat.com> <20210816094739.21970-7-david@redhat.com> MIME-Version: 1.0 In-Reply-To: <20210816094739.21970-7-david@redhat.com> User-Agent: Mutt/2.0.7 (2021-05-04) X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=berrange@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Received-SPF: pass client-ip=170.10.133.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Cc: Pankaj Gupta , Eduardo Habkost , "Michael S. Tsirkin" , "Dr . David Alan Gilbert" , qemu-devel@nongnu.org, Pankaj Gupta , Igor Mammedov , Paolo Bonzini Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Mon, Aug 16, 2021 at 11:47:38AM +0200, David Hildenbrand wrote: > Add a mutex to protect the SIGBUS case, as we cannot mess concurrently > with the sigbus handler and we have to manage the global variable > sigbus_memset_context. The MADV_POPULATE_WRITE path can run > concurrently. > > Note that page_mutex and page_cond are shared between concurrent > invocations, which shouldn't be a problem. > > This is a preparation for future virtio-mem prealloc code, which will call > os_mem_prealloc() asynchronously from an iothread when handling guest > requests. > > Add a comment that messing with the SIGBUS handler is frowned upon and > can result in problems we fortunately haven't seen so far. Note that > forwarding signals to the already installed SIGBUS handler isn't clean > either, as that one might modify the SIGBUS handler again. Even with the mutex, messing with SIGBUS post-startup still isn't safe as we're clashing with SIGBUS usage in softmmu/cpus.c IIUC, the virtio-mem prealloc code is something new that we've not shipped yet. With this in mind, how about we simply enforce that usage of this new feature is dependant on kernel support for MADV_POPULATE_WRITE ? If users want this feature they'll simply need to update to a modern kernel. This shouldn't break any existing deployed QEMU guests IIUC > > Reviewed-by: Pankaj Gupta > Signed-off-by: David Hildenbrand > --- > util/oslib-posix.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index efa4f96d56..9829149e4b 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -95,6 +95,7 @@ typedef struct MemsetThread MemsetThread; > > /* used by sigbus_handler() */ > static MemsetContext *sigbus_memset_context; > +static QemuMutex sigbus_mutex; > > static QemuMutex page_mutex; b> static QemuCond page_cond; > @@ -625,6 +626,7 @@ static bool madv_populate_write_possible(char *area, size_t pagesize) > void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus, > Error **errp) > { > + static gsize initialized; > int ret; > struct sigaction act, oldact; > size_t hpagesize = qemu_fd_getpagesize(fd); > @@ -638,6 +640,12 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus, > use_madv_populate_write = madv_populate_write_possible(area, hpagesize); > > if (!use_madv_populate_write) { > + if (g_once_init_enter(&initialized)) { > + qemu_mutex_init(&sigbus_mutex); > + g_once_init_leave(&initialized, 1); > + } > + > + qemu_mutex_lock(&sigbus_mutex); > memset(&act, 0, sizeof(act)); > act.sa_handler = &sigbus_handler; > act.sa_flags = 0; > @@ -665,6 +673,7 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus, > perror("os_mem_prealloc: failed to reinstall signal handler"); > exit(1); > } > + qemu_mutex_unlock(&sigbus_mutex); > } > } > > -- > 2.31.1 > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|