From nobody Sun Apr 28 08:51:22 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1623146927; cv=none; d=zohomail.com; s=zohoarc; b=fLubiICPH9zFYAt8bTZxHqyBOE41kL3cQp9939IuSrxz4i2czjgYqbSop9Faf3GD5QzIE14Xkdaq/Fef+9lGXA/BYfvL2BqulGbB+tvexe627blFQvSb9BQQyS+XpEs17amz9O4km4qfxzRgdQRfsTHpxnzEsuIapj9Hbld9/6A= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1623146927; h=Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:Message-ID:Sender:Subject:To; bh=WWx1c4xCo/kN7OvGQUgBOy+jKZqydcCPkw/e/l7Xqec=; b=hsuJWLvaPxS356CLPDFl8AcCkfgaAmI+CnliLVOjjFQTeaoLGUHwhczRLoOxUCViFiXecOPadZcYWDZJHPOh66OujhCepRRVmyQcb9kM6aoUPwPYPlc67GvZcARguE56uWGeqkOapNl0d1Q/A1Qwu5KWMeAubuCOK9NnK7424hc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1623146927014193.44762241340857; Tue, 8 Jun 2021 03:08:47 -0700 (PDT) Received: from list by lists.xenproject.org with outflank-mailman.138387.256186 (Exim 4.92) (envelope-from ) id 1lqYes-0006oa-Bi; Tue, 08 Jun 2021 10:08:30 +0000 Received: by outflank-mailman (output) from mailman id 138387.256186; Tue, 08 Jun 2021 10:08:30 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lqYes-0006oS-8K; Tue, 08 Jun 2021 10:08:30 +0000 Received: by outflank-mailman (input) for mailman id 138387; Tue, 08 Jun 2021 10:08:29 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lqYer-0006oM-Nm for xen-devel@lists.xenproject.org; Tue, 08 Jun 2021 10:08:29 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lqYeq-0005AG-GO; Tue, 08 Jun 2021 10:08:28 +0000 Received: from 54-240-197-235.amazon.com ([54.240.197.235] helo=ufe34d9ed68d054.ant.amazon.com) by xenbits.xenproject.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lqYeq-00027X-6a; Tue, 08 Jun 2021 10:08:28 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version 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" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Message-Id:Date:Subject:Cc:To:From; bh=WWx1c4xCo/kN7OvGQUgBOy+jKZqydcCPkw/e/l7Xqec=; b=6TXhwZyRyHMEsBiAemv6XBy1g5 v/xZ+oR3a4SaNDkSLlSMYHVeDF+kBz9eezBhEOQakZsace3vmHiXNloc7Zk9BMhY6aEjdrfZfj8yr ImUr4HhBl7lgcw1h0dHAxxUqCgIYhIrdQ9DLqw03yDh+qEK1AAqq07pkv/S/sTBhu+t8=; From: Julien Grall To: xen-devel@lists.xenproject.org Cc: julien@xen.org, Julien Grall , Andrew Cooper , George Dunlap , Ian Jackson , Jan Beulich , Stefano Stabellini , Wei Liu Subject: [PATCH v2] xen/grant-table: Simplify the update to the per-vCPU maptrack freelist Date: Tue, 8 Jun 2021 11:08:24 +0100 Message-Id: <20210608100824.25141-1-julien@xen.org> X-Mailer: git-send-email 2.17.1 X-ZohoMail-DKIM: pass (identity @xen.org) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Julien Grall Since XSA-228 (commit 02cbeeb62075 "gnttab: split maptrack lock to make it fulfill its purpose again"), v->maptrack_head, v->maptrack_tail and the content of the freelist are accessed with the lock v->maptrack_freelist_lock held. Therefore it is not necessary to update the fields using cmpxchg() and also read them atomically. Note that there are two cases where v->maptrack_tail is accessed without the lock. They both happen in get_maptrack_handle() when initializing the free list of the current vCPU. Therefore there is no possible race. The code is now reworked to remove any use of cmpxch() and read_atomic() when accessing the fields v->maptrack_{head, tail} as wel as the freelist. Take the opportunity to add a comment on top of the lock definition and explain what it protects. Signed-off-by: Julien Grall Reviewed-by: Jan Beulich ---- Changes in v2: - Fix typoes - Update the commit message - Don't use interchangeably MAPTRACK_TAIL and INVALID_MAPTRACK_HANDLE --- xen/common/grant_table.c | 66 ++++++++++++++++------------------------ xen/include/xen/sched.h | 8 ++++- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index ab30e2e8cfb6..fab77ab9ccb8 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -543,33 +543,27 @@ double_gt_unlock(struct grant_table *lgt, struct gran= t_table *rgt) static inline grant_handle_t _get_maptrack_handle(struct grant_table *t, struct vcpu *v) { - unsigned int head, next, prev_head; + unsigned int head, next; =20 spin_lock(&v->maptrack_freelist_lock); =20 - do { - /* No maptrack pages allocated for this VCPU yet? */ - head =3D read_atomic(&v->maptrack_head); - if ( unlikely(head =3D=3D MAPTRACK_TAIL) ) - { - spin_unlock(&v->maptrack_freelist_lock); - return INVALID_MAPTRACK_HANDLE; - } - - /* - * Always keep one entry in the free list to make it easier to - * add free entries to the tail. - */ - next =3D read_atomic(&maptrack_entry(t, head).ref); - if ( unlikely(next =3D=3D MAPTRACK_TAIL) ) - { - spin_unlock(&v->maptrack_freelist_lock); - return INVALID_MAPTRACK_HANDLE; - } + /* No maptrack pages allocated for this VCPU yet? */ + head =3D v->maptrack_head; + if ( unlikely(head =3D=3D MAPTRACK_TAIL) ) + { + spin_unlock(&v->maptrack_freelist_lock); + return INVALID_MAPTRACK_HANDLE; + } =20 - prev_head =3D head; - head =3D cmpxchg(&v->maptrack_head, prev_head, next); - } while ( head !=3D prev_head ); + /* + * Always keep one entry in the free list to make it easier to + * add free entries to the tail. + */ + next =3D maptrack_entry(t, head).ref; + if ( unlikely(next =3D=3D MAPTRACK_TAIL) ) + head =3D INVALID_MAPTRACK_HANDLE; + else + v->maptrack_head =3D next; =20 spin_unlock(&v->maptrack_freelist_lock); =20 @@ -623,7 +617,7 @@ put_maptrack_handle( { struct domain *currd =3D current->domain; struct vcpu *v; - unsigned int prev_tail, cur_tail; + unsigned int tail; =20 /* 1. Set entry to be a tail. */ maptrack_entry(t, handle).ref =3D MAPTRACK_TAIL; @@ -633,14 +627,11 @@ put_maptrack_handle( =20 spin_lock(&v->maptrack_freelist_lock); =20 - cur_tail =3D read_atomic(&v->maptrack_tail); - do { - prev_tail =3D cur_tail; - cur_tail =3D cmpxchg(&v->maptrack_tail, prev_tail, handle); - } while ( cur_tail !=3D prev_tail ); + tail =3D v->maptrack_tail; + v->maptrack_tail =3D handle; =20 /* 3. Update the old tail entry to point to the new entry. */ - write_atomic(&maptrack_entry(t, prev_tail).ref, handle); + maptrack_entry(t, tail).ref =3D handle; =20 spin_unlock(&v->maptrack_freelist_lock); } @@ -650,7 +641,7 @@ get_maptrack_handle( struct grant_table *lgt) { struct vcpu *curr =3D current; - unsigned int i, head; + unsigned int i; grant_handle_t handle; struct grant_mapping *new_mt =3D NULL; =20 @@ -686,7 +677,7 @@ get_maptrack_handle( maptrack_entry(lgt, handle).ref =3D MAPTRACK_TAIL; curr->maptrack_tail =3D handle; if ( curr->maptrack_head =3D=3D MAPTRACK_TAIL ) - write_atomic(&curr->maptrack_head, handle); + curr->maptrack_head =3D handle; spin_unlock(&curr->maptrack_freelist_lock); } return steal_maptrack_handle(lgt, curr); @@ -707,7 +698,7 @@ get_maptrack_handle( new_mt[i].vcpu =3D curr->vcpu_id; } =20 - /* Set tail directly if this is the first page for this VCPU. */ + /* Set tail directly if this is the first page for the local vCPU. */ if ( curr->maptrack_tail =3D=3D MAPTRACK_TAIL ) curr->maptrack_tail =3D handle + MAPTRACK_PER_PAGE - 1; =20 @@ -716,13 +707,10 @@ get_maptrack_handle( lgt->maptrack_limit +=3D MAPTRACK_PER_PAGE; =20 spin_unlock(&lgt->maptrack_lock); - spin_lock(&curr->maptrack_freelist_lock); - - do { - new_mt[i - 1].ref =3D read_atomic(&curr->maptrack_head); - head =3D cmpxchg(&curr->maptrack_head, new_mt[i - 1].ref, handle += 1); - } while ( head !=3D new_mt[i - 1].ref ); =20 + spin_lock(&curr->maptrack_freelist_lock); + new_mt[i - 1].ref =3D curr->maptrack_head; + curr->maptrack_head =3D handle + 1; spin_unlock(&curr->maptrack_freelist_lock); =20 return handle; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 3982167144c6..6c52ba2af019 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -255,7 +255,13 @@ struct vcpu /* VCPU paused by system controller. */ int controller_pause_count; =20 - /* Grant table map tracking. */ + /* + * Grant table map tracking. The lock maptrack_freelist_lock + * protects to: + * - The entries in the freelist + * - maptrack_head + * - maptrack_tail + */ spinlock_t maptrack_freelist_lock; unsigned int maptrack_head; unsigned int maptrack_tail; --=20 2.17.1