diff mbox series

[3/4] xen/domctl: Introduce fault_ttl

Message ID 20201223163442.8840-4-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series xen: domain-tracked allocations, and fault injection | expand

Commit Message

Andrew Cooper Dec. 23, 2020, 4:34 p.m. UTC
To inject a simulated resource failure, for testing purposes.

Given a specific set of hypercall parameters, the failure is in a repeatable
position, for the currently booted Xen.  The exact position of failures is
highly dependent on the build of Xen, and hardware support.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>

RFC:
 * Probably wants to be Kconfig'd
 * I'm thinking of dropping handle from xen_domctl_createdomain because it's a
   waste of valuable space.
---
 xen/common/dmalloc.c        | 8 +++++++-
 xen/common/domain.c         | 8 ++++++--
 xen/include/public/domctl.h | 1 +
 xen/include/xen/sched.h     | 1 +
 4 files changed, 15 insertions(+), 3 deletions(-)

Comments

Jan Beulich Jan. 5, 2021, 4:39 p.m. UTC | #1
On 23.12.2020 17:34, Andrew Cooper wrote:
> To inject a simulated resource failure, for testing purposes.
> 
> Given a specific set of hypercall parameters, the failure is in a repeatable
> position, for the currently booted Xen.  The exact position of failures is
> highly dependent on the build of Xen, and hardware support.

What about other kinds of resources, or ones only indirectly
related to memory allocations (e.g. where we don't mean to
associate them with the domain)?

> RFC:
>  * Probably wants to be Kconfig'd

Yes.

>  * I'm thinking of dropping handle from xen_domctl_createdomain because it's a
>    waste of valuable space.

Looks entirely unrelated, but yes - as long as Xen itself has no
consumer of the field. The more that there already is
XEN_DOMCTL_setdomainhandle.

> --- a/xen/common/dmalloc.c
> +++ b/xen/common/dmalloc.c
> @@ -10,7 +10,13 @@ void dfree(struct domain *d, void *ptr)
>  
>  void *_dzalloc(struct domain *d, size_t size, size_t align)
>  {
> -    void *ptr = _xmalloc(size, align);
> +    void *ptr;
> +
> +    if ( atomic_read(&d->fault_ttl) &&
> +         atomic_dec_and_test(&d->fault_ttl) )
> +        return NULL;

Perhaps we want to introduce Linux'es atomic_add_unless()?

Jan
Andrew Cooper Jan. 13, 2021, 11:58 p.m. UTC | #2
On 05/01/2021 16:39, Jan Beulich wrote:
> On 23.12.2020 17:34, Andrew Cooper wrote:
>> To inject a simulated resource failure, for testing purposes.
>>
>> Given a specific set of hypercall parameters, the failure is in a repeatable
>> position, for the currently booted Xen.  The exact position of failures is
>> highly dependent on the build of Xen, and hardware support.
> What about other kinds of resources, or ones only indirectly
> related to memory allocations (e.g. where we don't mean to
> associate them with the domain)?

I intend this for "any fail-able operation", not just plain memory
allocation.

It's merely that memory allocation is the simple first resource to go
after.  And after all, these 4 patches alone have already identified a
real bug in ARM's dom0less logic.

Perhaps what we actually want is a general "bool simulate_failure(d)"
which dalloc() and others can use.

>>  * I'm thinking of dropping handle from xen_domctl_createdomain because it's a
>>    waste of valuable space.
> Looks entirely unrelated, but yes - as long as Xen itself has no
> consumer of the field. The more that there already is
> XEN_DOMCTL_setdomainhandle.

It's purely for pretty printing in 'q' and handing back to userspace for
xentop, et al.  Nothing in Xen acts meaningfully upon the value.

>> --- a/xen/common/dmalloc.c
>> +++ b/xen/common/dmalloc.c
>> @@ -10,7 +10,13 @@ void dfree(struct domain *d, void *ptr)
>>  
>>  void *_dzalloc(struct domain *d, size_t size, size_t align)
>>  {
>> -    void *ptr = _xmalloc(size, align);
>> +    void *ptr;
>> +
>> +    if ( atomic_read(&d->fault_ttl) &&
>> +         atomic_dec_and_test(&d->fault_ttl) )
>> +        return NULL;
> Perhaps we want to introduce Linux'es atomic_add_unless()?

Possibly.  I definitely got caught out by the semantics of
atomic_dec_and_test() seeing as it has an implicit "!= 0".

~Andrew
diff mbox series

Patch

diff --git a/xen/common/dmalloc.c b/xen/common/dmalloc.c
index e3a0e546c2..1f5d0f5627 100644
--- a/xen/common/dmalloc.c
+++ b/xen/common/dmalloc.c
@@ -10,7 +10,13 @@  void dfree(struct domain *d, void *ptr)
 
 void *_dzalloc(struct domain *d, size_t size, size_t align)
 {
-    void *ptr = _xmalloc(size, align);
+    void *ptr;
+
+    if ( atomic_read(&d->fault_ttl) &&
+         atomic_dec_and_test(&d->fault_ttl) )
+        return NULL;
+
+    ptr = _xmalloc(size, align);
 
     if ( ptr )
         atomic_inc(&d->dalloc_heap);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 1db1c0e70a..cd73321980 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -427,14 +427,18 @@  struct domain *domain_create(domid_t domid,
     if ( (d = alloc_domain_struct()) == NULL )
         return ERR_PTR(-ENOMEM);
 
-    d->options = config ? config->flags : 0;
-
     /* Sort out our idea of is_system_domain(). */
     d->domain_id = domid;
 
     /* Debug sanity. */
     ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
 
+    if ( config )
+    {
+        d->options = config->flags;
+        atomic_set(&d->fault_ttl, config->fault_ttl);
+    }
+
     /* Sort out our idea of is_control_domain(). */
     d->is_privileged = is_priv;
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 666aeb71bf..aaa3d66616 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -48,6 +48,7 @@ 
 /* XEN_DOMCTL_createdomain */
 struct xen_domctl_createdomain {
     /* IN parameters */
+    uint32_t fault_ttl;
     uint32_t ssidref;
     xen_domain_handle_t handle;
  /* Is this an HVM guest (as opposed to a PV guest)? */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 8ed8b55a1e..620a9f20e5 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -349,6 +349,7 @@  struct domain
     atomic_t         shr_pages;         /* shared pages */
     atomic_t         paged_pages;       /* paged-out pages */
 
+    atomic_t         fault_ttl;         /* Time until a simulated resource failure. */
     atomic_t         dalloc_heap;       /* Number of xmalloc-like allocations. */
 
     /* Scheduling. */