[systemd-devel] [PATCH 2/2] domain: grab the domain's parent lock only when needed

[Daniel Mack]

On 03/19/2014 09:24 PM, Djalal Harouni wrote:
> Signed-off-by: Djalal Harouni <tixxdz at opendz.org>
> ---
>  domain.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/domain.c b/domain.c
> index d27cad2..554b4fe 100644
> --- a/domain.c
> +++ b/domain.c
> @@ -183,12 +183,13 @@ struct kdbus_domain *kdbus_domain_unref(struct kdbus_domain *domain)
>  	return NULL;
>  }
>  
> -static struct kdbus_domain *kdbus_domain_find(struct kdbus_domain const *parent,
> +static struct kdbus_domain *kdbus_domain_find(struct kdbus_domain *parent,
>  				      const char *name)
>  {
>  	struct kdbus_domain *domain = NULL;
>  	struct kdbus_domain *n;
>  
> +	mutex_lock(&parent->lock);
>  	list_for_each_entry(n, &parent->domain_list, domain_entry) {
>  		if (strcmp(n->name, name))
>  			continue;
> @@ -196,6 +197,7 @@ static struct kdbus_domain *kdbus_domain_find(struct kdbus_domain const *parent,
>  		domain = kdbus_domain_ref(n);
>  		break;
>  	}
> +	mutex_unlock(&parent->lock);
>  
>  	return domain;
>  }
> @@ -288,9 +290,6 @@ int kdbus_domain_new(struct kdbus_domain *parent, const char *name,
>  	atomic64_set(&d->msg_seq_last, 0);
>  	idr_init(&d->user_idr);
>  
> -	if (parent)
> -		mutex_lock(&parent->lock);
> -
>  	mutex_lock(&kdbus_subsys_lock);
>  
>  	/* compose name and path of base directory in /dev */
> @@ -340,21 +339,19 @@ int kdbus_domain_new(struct kdbus_domain *parent, const char *name,
>  
>  	/* link into parent domain */
>  	if (parent) {

What if 'parent' was already unrefed at this point? That can happen any
time as soon we give up the lock.

kdbus_domain_unref() calls kdbus_domain_disconnect(), wihch grabs
domain->lock, so we're only safe as long as we hold the lock across all
operations on the object, right?


Daniel