[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] xen: add domid_to_domain() helper


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 12 Sep 2022 10:19:33 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=5CpTGMLyLtXeWbDHr5OLNdMgVmEoioIN6kAE9108s98=; b=JiMZve/LLXa9Go1moKswPIHMORE+7FIBUgP7ws6FaQ/tJDf+IrQp6XRVIbF89L1mjMn5qzgcEo4RrqBrEFOFk0n5bX3JRFd0wFi4NzmuuZ1PUc1MLhZ8gbjiFbyXb3H44R6kAKX/oLG9hKKkwqIagHeRYirAmB3lByYa4C/9dZDrI8u1PfBtVW9AG8yqCojues0pA6DdWgOjm7viEClSCFYpWfb1UrxMdW/2n+rG2CEsBHSHKa+uC+3G+8s4kUfD57CTNx8DZvPN+Ka1tv+aV+ppYNvWzRUrNhavoRMRpLIzBToTXLRejHUMYWTmcyzGFJNPX6T8UvPeuRUubZQvMg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=j4m6Vz5ft/AVcWqRS1TszydO44aaAprXYDZ/BUwVj9q2VdH6OEYTmLvZ6YRo5LM+oqx+UE+8b9KaboKmj04b1j5kfiB745b3u+8O7x3AC4SUakV6KHdhs68fpSTTn3GsTr1LfrdqU8cMEE4F6BLdI+5hCrOnNqBF3T6LH1jWSU0O76TE9q4T4XvCHCdUt+V4ViwSGw9buUy2Ttc1rwOM6sIIKeHB7bvVU5Eqwn3GPGJVtSEbh/33Ipmbjc2MaduYPMaIxQKkcpj7fwcB1nEa1Z6mRbrA+WPM0HRBUzbFBN5+I7EwGjzYjqgRXz3a+nwGw+e1WRbtwUd9qyBb7OCN0A==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 12 Sep 2022 08:20:20 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12.09.2022 07:53, Juergen Gross wrote:
> Add a helper domid_to_domain() returning the struct domain pointer for
> a domain give by its domid and which is known not being able to be
> released (its reference count isn't incremented and no rcu_lock_domain()
> is called for it).
> 
> In order to simplify coding add an internal helper for doing the lookup
> and call that from the new function and similar functions.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

I don't see an issue with adding such a helper (responding to your concern
in the cover letter), but I think the constraints need to be empahsized
more: We already have get_knownalive_domain() and get_domain_by_id(), so
how about naming the new helper get_knownalive_domain_by_id()? And then ...

> @@ -859,20 +866,27 @@ struct domain *get_domain_by_id(domid_t dom)
>  
>  struct domain *rcu_lock_domain_by_id(domid_t dom)
>  {
> -    struct domain *d = NULL;
> +    struct domain *d;
>  
>      rcu_read_lock(&domlist_read_lock);
>  
> -    for ( d = rcu_dereference(domain_hash[DOMAIN_HASH(dom)]);
> -          d != NULL;
> -          d = rcu_dereference(d->next_in_hashbucket) )
> -    {
> -        if ( d->domain_id == dom )
> -        {
> -            rcu_lock_domain(d);
> -            break;
> -        }
> -    }
> +    d = domid_2_domain(dom);
> +    if ( d )
> +        rcu_lock_domain(d);
> +
> +    rcu_read_unlock(&domlist_read_lock);
> +
> +    return d;
> +}
> +
> +/* Use only if struct domain is known to stay allocated! */
> +struct domain *domid_to_domain(domid_t dom)
> +{
> +    struct domain *d;
> +
> +    rcu_read_lock(&domlist_read_lock);
> +
> +    d = domid_2_domain(dom);
>  
>      rcu_read_unlock(&domlist_read_lock);

... extend the comment here and in the header (or perhaps one in
the header would suffice and the definition here doesn't need any
further comment) to explicitly say "reference held or RCU-locked".

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.