Re: [Xen-devel] [PATCH v5 27/28] xsplice: Add support for shadow variables.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.03.16 at 21:00, <konrad.wilk@xxxxxxxxxx> wrote:
> Shadow variables are a piece of infrastructure to be used by xsplice
> modules. They are used to attach a new piece of data to an existing
> structure in memory.

An already known question again: Out of recent XSAs, how many
needed such? I.e. can#t we put this off for now?

> Martin Pohlack also mentions that using the SHADOW_SLOTS of small
> prime numbers has the advantage of having a simple hash function.

This reads kind of backwards.

> +void xsplice_shadow_free(const void *obj, const char *var)
> +{
> +    struct shadow_var *entry, *shadow = NULL;
> +    unsigned int slot;
> +    struct hlist_node *next;
> +
> +    slot = (unsigned long)obj % SHADOW_SLOTS;
> +
> +    spin_lock(&shadow_lock);
> +    hlist_for_each_entry(entry, next, &shadow_tbl[slot], list)
> +    {
> +        if ( entry->obj == obj &&
> +             !strcmp(entry->var, var) )
> +        {
> +            shadow = entry;
> +            break;
> +        }
> +    }
> +    if (shadow)

Coding style.

> +    {
> +        hlist_del(&shadow->list);
> +        xfree(shadow->data);
> +        xfree(shadow);
> +    }
> +    spin_unlock(&shadow_lock);
> +}

Uniqueness not being guaranteed by the allocation function, how
can you free the first hit here ...

> +void *xsplice_shadow_get(const void *obj, const char *var)
> +{
> +    struct shadow_var *entry;
> +    unsigned int slot;
> +    struct hlist_node *next;
> +    void *ret = NULL;
> +
> +    slot = (unsigned long)obj % SHADOW_SLOTS;
> +
> +    spin_lock(&shadow_lock);
> +    hlist_for_each_entry(entry, next, &shadow_tbl[slot], list)
> +    {
> +        if ( entry->obj == obj &&
> +             !strcmp(entry->var, var) )
> +        {
> +            ret = entry->data;
> +            break;
> +        }
> +    }
> +
> +    spin_unlock(&shadow_lock);
> +    return ret;
> +}

.. or return the first match here?

> +static int __init xsplice_shadow_init(void)
> +{
> +    int i;

unsigned int

> --- a/xen/include/xen/xsplice_patch.h
> +++ b/xen/include/xen/xsplice_patch.h
> @@ -56,4 +56,40 @@ typedef void (*xsplice_unloadcall_t)(void);
>  #define XSPLICE_UNLOAD_HOOK(_fn) \
>       xsplice_unloadcall_t __attribute__((weak)) xsplice_unload_data 
> __section(".xsplice.hooks.unload") = _fn;
>  
> +
> +/*
> + * The following definitions are to be used in patches. They are taken
> + * from kpatch.
> + */
> +
> +/*
> + * xsplice shadow variables
> + *
> + * These functions can be used to add new "shadow" fields to existing data
> + * structures.  For example, to allocate a "newpid" variable associated with 
> an
> + * instance of task_struct, and assign it a value of 1000:
> + *
> + * struct task_struct *tsk = current;
> + * int *newpid;
> + * newpid = xsplice_shadow_alloc(tsk, "newpid", sizeof(int));
> + * if (newpid)
> + *   *newpid = 1000;
> + *
> + * To retrieve a pointer to the variable:
> + *
> + * struct task_struct *tsk = current;
> + * int *newpid;
> + * newpid = xsplice_shadow_get(tsk, "newpid");
> + * if (newpid)
> + *   printk("task newpid = %d\n", *newpid); // prints "task newpid = 1000"
> + *
> + * To free it:
> + *
> + * xsplice_shadow_free(tsk, "newpid");
> + */

While this indeed provides some basic understanding, I think this
should be using a more Xen-affine example, and I fail to see how
with particularly the example given this is going to be useful: How
would the patch module sanely and within reasonable time get
hold of all instances of a particular type?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel