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 X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B818C433F5 for ; Fri, 17 Sep 2021 14:22:41 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (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 4B77A60EB2 for ; Fri, 17 Sep 2021 14:22:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4B77A60EB2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=tklengyel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.189452.339160 (Exim 4.92) (envelope-from ) id 1mREkw-0004Oo-6O; Fri, 17 Sep 2021 14:22:22 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 189452.339160; Fri, 17 Sep 2021 14:22:22 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mREkw-0004Oh-34; Fri, 17 Sep 2021 14:22:22 +0000 Received: by outflank-mailman (input) for mailman id 189452; Fri, 17 Sep 2021 14:22:21 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mREku-0004Ob-VD for xen-devel@lists.xenproject.org; Fri, 17 Sep 2021 14:22:20 +0000 Received: from MTA-06-4.privateemail.com (unknown [198.54.122.56]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id aaa55fce-17c2-11ec-b6ba-12813bfff9fa; Fri, 17 Sep 2021 14:22:20 +0000 (UTC) Received: from mta-06.privateemail.com (localhost [127.0.0.1]) by mta-06.privateemail.com (Postfix) with ESMTP id 3E1401800209 for ; Fri, 17 Sep 2021 10:22:19 -0400 (EDT) Received: from mail-wr1-f51.google.com (unknown [10.20.151.230]) by mta-06.privateemail.com (Postfix) with ESMTPA id 254A41800200 for ; Fri, 17 Sep 2021 10:22:19 -0400 (EDT) Received: by mail-wr1-f51.google.com with SMTP id t18so15549490wrb.0 for ; Fri, 17 Sep 2021 07:22:19 -0700 (PDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: aaa55fce-17c2-11ec-b6ba-12813bfff9fa X-Gm-Message-State: AOAM53041YfH5HfKwI3+5hbkl5jUPFODplhy5jQeqyYLEWDqnNshk9sz z+ZAM0mZ7qKJSlRlifja4WkOxrYpfQRybI157r0= X-Google-Smtp-Source: ABdhPJwyY6jEs4fX6nDZzhwypKgH6lJhUFFf92sguZ1vRcBsb8Y/SI0mAz2NX0eMVkH5dpPS9rrXHpMQRS1GzNeocbc= X-Received: by 2002:adf:e6c9:: with SMTP id y9mr12616102wrm.429.1631888537841; Fri, 17 Sep 2021 07:22:17 -0700 (PDT) MIME-Version: 1.0 References: <52b28c4d-1cf1-6c98-e1dc-1e0f7b2f768c@suse.com> In-Reply-To: <52b28c4d-1cf1-6c98-e1dc-1e0f7b2f768c@suse.com> From: Tamas K Lengyel Date: Fri, 17 Sep 2021 10:21:41 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] x86/mem_sharing: don't lock parent during fork reset To: Jan Beulich Cc: Tamas K Lengyel , Andrew Cooper , George Dunlap , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Wei Liu , Xen-devel Content-Type: text/plain; charset="UTF-8" X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Sep 17, 2021 at 3:26 AM Jan Beulich wrote: > > On 16.09.2021 17:04, Tamas K Lengyel wrote: > > During fork reset operation the parent domain doesn't need to be gathered using > > rcu_lock_live_remote_domain_by_id as the fork reset doesn't modify anything on > > the parent. The parent is also guaranteed to be paused while forks are active. > > This patch reduces lock contention when performing resets in parallel. > > I'm a little in trouble following you here: RCU locks aren't really > locks in that sense, so "lock contention" seems misleading to me. I > can see that rcu_lock_domain_by_id()'s loop is extra overhead. > > Furthermore - does the parent being paused really mean the parent > can't go away behind the back of the fork reset? In fork() I see > > if ( rc && rc != -ERESTART ) > { > domain_unpause(d); > put_domain(d); > cd->parent = NULL; > } > > i.e. the ref gets dropped before the parent pointer gets cleared. If > the parent having a reference kept was indeed properly guaranteed, I > agree the code change itself is fine. > > (The sequence looks correct at the other put_domain() site [dealing > with the success case of fork(), when the reference gets retained] > in domain_relinquish_resources().) This code above you copied is when the fork() fails. Calling fork_reset() before fork() successfully returns is not a sane sequence and it is not "supported" by any means. If someone would try to do that it would be racy as-is already with or without this patch. Clearing the cd->parent pointer first here on the error path wouldn't guarantee that sequence to be safe or sane either. Adding an extra field to struct domain that signifies that "fork is complete" would be a way to make that safe. But since the toolstack using this interface is already sane (ie. never calls fork_reset before a successful fork) I really don't think that's necessary. It would just grow struct domain for very little benefit. Tamas