All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] domctl: improve locking during domain destruction
@ 2017-12-06 15:31 Jan Beulich
  2017-12-06 15:34 ` Andrew Cooper
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2017-12-06 15:31 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
	Ian Jackson, Tim Deegan

There is no need to hold the global domctl lock across domain_kill() -
the domain lock is fully sufficient here, and parallel cleanup after
multiple domains performs quite a bit better this way.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Changes since RFC: Comment added.
---
Obviously other domctl-s could benefit from similar adjustments, so
this is meant to be just a start.

--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -615,13 +615,21 @@ int domain_kill(struct domain *d)
     if ( d == current->domain )
         return -EINVAL;
 
-    /* Protected by domctl_lock. */
+    /* Protected by d->domain_lock. */
     switch ( d->is_dying )
     {
     case DOMDYING_alive:
+        domain_unlock(d);
         domain_pause(d);
+        domain_lock(d);
+        /*
+         * With the domain lock dropped, d->is_dying may have changed. Call
+         * ourselves recursively if so, which is safe as then we won't come
+         * back here.
+         */
+        if ( d->is_dying != DOMDYING_alive )
+            return domain_kill(d);
         d->is_dying = DOMDYING_dying;
-        spin_barrier(&d->domain_lock);
         evtchn_destroy(d);
         gnttab_release_mappings(d);
         tmem_destroy(d->tmem_client);
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -665,11 +665,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         break;
 
     case XEN_DOMCTL_destroydomain:
+        domctl_lock_release();
+        domain_lock(d);
         ret = domain_kill(d);
+        domain_unlock(d);
         if ( ret == -ERESTART )
             ret = hypercall_create_continuation(
                 __HYPERVISOR_domctl, "h", u_domctl);
-        break;
+        goto domctl_out_unlock_domonly;
 
     case XEN_DOMCTL_setnodeaffinity:
     {




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] domctl: improve locking during domain destruction
  2017-12-06 15:31 [PATCH] domctl: improve locking during domain destruction Jan Beulich
@ 2017-12-06 15:34 ` Andrew Cooper
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2017-12-06 15:34 UTC (permalink / raw)
  To: Jan Beulich, xen-devel
  Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan, Ian Jackson

On 06/12/17 15:31, Jan Beulich wrote:
> There is no need to hold the global domctl lock across domain_kill() -
> the domain lock is fully sufficient here, and parallel cleanup after
> multiple domains performs quite a bit better this way.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This is clearer.  Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
> Changes since RFC: Comment added.
> ---
> Obviously other domctl-s could benefit from similar adjustments, so
> this is meant to be just a start.
>
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -615,13 +615,21 @@ int domain_kill(struct domain *d)
>      if ( d == current->domain )
>          return -EINVAL;
>  
> -    /* Protected by domctl_lock. */
> +    /* Protected by d->domain_lock. */
>      switch ( d->is_dying )
>      {
>      case DOMDYING_alive:
> +        domain_unlock(d);
>          domain_pause(d);
> +        domain_lock(d);
> +        /*
> +         * With the domain lock dropped, d->is_dying may have changed. Call
> +         * ourselves recursively if so, which is safe as then we won't come
> +         * back here.
> +         */
> +        if ( d->is_dying != DOMDYING_alive )
> +            return domain_kill(d);
>          d->is_dying = DOMDYING_dying;
> -        spin_barrier(&d->domain_lock);
>          evtchn_destroy(d);
>          gnttab_release_mappings(d);
>          tmem_destroy(d->tmem_client);
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -665,11 +665,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>          break;
>  
>      case XEN_DOMCTL_destroydomain:
> +        domctl_lock_release();
> +        domain_lock(d);
>          ret = domain_kill(d);
> +        domain_unlock(d);
>          if ( ret == -ERESTART )
>              ret = hypercall_create_continuation(
>                  __HYPERVISOR_domctl, "h", u_domctl);
> -        break;
> +        goto domctl_out_unlock_domonly;
>  
>      case XEN_DOMCTL_setnodeaffinity:
>      {
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-12-06 15:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-06 15:31 [PATCH] domctl: improve locking during domain destruction Jan Beulich
2017-12-06 15:34 ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.