| Message ID | 1455300361-13092-22-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12.02.2016 19:05, Konrad Rzeszutek Wilk wrote: > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > 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. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > --- > xen/common/Makefile | 1 + > xen/common/xsplice_shadow.c | 105 ++++++++++++++++++++++++++++++++++++++++ > xen/include/xen/xsplice_patch.h | 39 +++++++++++++++ > 3 files changed, 145 insertions(+) > create mode 100644 xen/common/xsplice_shadow.c > create mode 100644 xen/include/xen/xsplice_patch.h > > diff --git a/xen/common/Makefile b/xen/common/Makefile > index a8ceaff..f4d54ad 100644 > --- a/xen/common/Makefile > +++ b/xen/common/Makefile > @@ -75,3 +75,4 @@ subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt > > obj-$(CONFIG_XSPLICE) += xsplice.o > obj-$(CONFIG_XSPLICE) += xsplice_elf.o > +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o > diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c > new file mode 100644 > index 0000000..619cdee > --- /dev/null > +++ b/xen/common/xsplice_shadow.c > @@ -0,0 +1,105 @@ > +#include <xen/init.h> > +#include <xen/kernel.h> > +#include <xen/lib.h> > +#include <xen/list.h> > +#include <xen/spinlock.h> > +#include <xen/xsplice_patch.h> > + > +#define SHADOW_SLOTS 256 Using something very round here will give you lot's of hash collisions at the price of a very fast hash computation as compilers, linkers, and memory allocators tend to align starting addresses. I would suggest to use a small prime here, e.g., 257 or 251, to have a first approximation of a simple hash function. Or use existing hash infrastructure (see below). > +struct hlist_head shadow_tbl[SHADOW_SLOTS]; > +static DEFINE_SPINLOCK(shadow_lock); > + > +struct shadow_var { > + struct hlist_node list; /* Linked to 'shadow_tbl' */ > + void *data; > + const void *obj; > + char var[16]; > +}; > + > +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size) > +{ > + struct shadow_var *shadow; > + unsigned int slot; > + > + shadow = xmalloc(struct shadow_var); > + if ( !shadow ) > + return NULL; > + > + shadow->obj = obj; > + strlcpy(shadow->var, var, sizeof shadow->var); > + shadow->data = xmalloc_bytes(size); > + if ( !shadow->data ) > + { > + xfree(shadow); > + return NULL; > + } > + > + slot = (unsigned long)obj % SHADOW_SLOTS; hash.h has an earlier import from Linux and provides hash_long(). That looks like it would not suffer from direct hash collisions. (also for all other occurrences of "obj % SHADOW_SLOTS" below) > + spin_lock(&shadow_lock); > + hlist_add_head(&shadow->list, &shadow_tbl[slot]); > + spin_unlock(&shadow_lock); > + > + return shadow->data; > +} > + > +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) > + { > + hlist_del(&shadow->list); > + xfree(shadow->data); > + xfree(shadow); > + } > + spin_unlock(&shadow_lock); > +} > + > +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; > +} > + > +static int __init xsplice_shadow_init(void) > +{ > + int i; > + > + for ( i = 0; i < SHADOW_SLOTS; i++ ) > + INIT_HLIST_HEAD(&shadow_tbl[i]); > + > + return 0; > +} > +__initcall(xsplice_shadow_init); > diff --git a/xen/include/xen/xsplice_patch.h b/xen/include/xen/xsplice_patch.h > new file mode 100644 > index 0000000..e3f344b > --- /dev/null > +++ b/xen/include/xen/xsplice_patch.h > @@ -0,0 +1,39 @@ > +#ifndef __XEN_XSPLICE_PATCH_H__ > +#define __XEN_XSPLICE_PATCH_H__ > + > +/* > + * 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"); > + */ > + > +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size); > +void xsplice_shadow_free(const void *obj, const char *var); > +void *xsplice_shadow_get(const void *obj, const char *var); > + > +#endif /* __XEN_XSPLICE_PATCH_H__ */ > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
On 12.02.2016 19:05, Konrad Rzeszutek Wilk wrote: > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > 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. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > --- > xen/common/Makefile | 1 + > xen/common/xsplice_shadow.c | 105 ++++++++++++++++++++++++++++++++++++++++ > xen/include/xen/xsplice_patch.h | 39 +++++++++++++++ > 3 files changed, 145 insertions(+) > create mode 100644 xen/common/xsplice_shadow.c > create mode 100644 xen/include/xen/xsplice_patch.h > > diff --git a/xen/common/Makefile b/xen/common/Makefile > index a8ceaff..f4d54ad 100644 > --- a/xen/common/Makefile > +++ b/xen/common/Makefile > @@ -75,3 +75,4 @@ subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt > > obj-$(CONFIG_XSPLICE) += xsplice.o > obj-$(CONFIG_XSPLICE) += xsplice_elf.o > +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o > diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c > new file mode 100644 > index 0000000..619cdee > --- /dev/null > +++ b/xen/common/xsplice_shadow.c > @@ -0,0 +1,105 @@ > +#include <xen/init.h> > +#include <xen/kernel.h> > +#include <xen/lib.h> > +#include <xen/list.h> > +#include <xen/spinlock.h> > +#include <xen/xsplice_patch.h> > + > +#define SHADOW_SLOTS 256 > +struct hlist_head shadow_tbl[SHADOW_SLOTS]; Thinking about this more, how would a module using this global hash ever be unloadable again without leaking memory? For unloading you would need some iterator that walks all the dynamically created shadow elements and frees them. The simplest approach would be if each hotpatch would bring its own instance of the hash table (if it needs it). That would allow it to fully walk and release the hash content on its unload path. Martin Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
On Mon, Mar 07, 2016 at 08:40:47AM +0100, Martin Pohlack wrote: > On 12.02.2016 19:05, Konrad Rzeszutek Wilk wrote: > > From: Ross Lagerwall <ross.lagerwall@citrix.com> > > > > 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. > > > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > > --- > > xen/common/Makefile | 1 + > > xen/common/xsplice_shadow.c | 105 ++++++++++++++++++++++++++++++++++++++++ > > xen/include/xen/xsplice_patch.h | 39 +++++++++++++++ > > 3 files changed, 145 insertions(+) > > create mode 100644 xen/common/xsplice_shadow.c > > create mode 100644 xen/include/xen/xsplice_patch.h > > > > diff --git a/xen/common/Makefile b/xen/common/Makefile > > index a8ceaff..f4d54ad 100644 > > --- a/xen/common/Makefile > > +++ b/xen/common/Makefile > > @@ -75,3 +75,4 @@ subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt > > > > obj-$(CONFIG_XSPLICE) += xsplice.o > > obj-$(CONFIG_XSPLICE) += xsplice_elf.o > > +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o > > diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c > > new file mode 100644 > > index 0000000..619cdee > > --- /dev/null > > +++ b/xen/common/xsplice_shadow.c > > @@ -0,0 +1,105 @@ > > +#include <xen/init.h> > > +#include <xen/kernel.h> > > +#include <xen/lib.h> > > +#include <xen/list.h> > > +#include <xen/spinlock.h> > > +#include <xen/xsplice_patch.h> > > + > > +#define SHADOW_SLOTS 256 > > Using something very round here will give you lot's of hash collisions > at the price of a very fast hash computation as compilers, linkers, and > memory allocators tend to align starting addresses. I would suggest to > use a small prime here, e.g., 257 or 251, to have a first approximation > of a simple hash function. Hey! I totally missed this comment. Let me spoll this up in my TODO to change this to 257 for v5.
diff --git a/xen/common/Makefile b/xen/common/Makefile index a8ceaff..f4d54ad 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -75,3 +75,4 @@ subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt obj-$(CONFIG_XSPLICE) += xsplice.o obj-$(CONFIG_XSPLICE) += xsplice_elf.o +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c new file mode 100644 index 0000000..619cdee --- /dev/null +++ b/xen/common/xsplice_shadow.c @@ -0,0 +1,105 @@ +#include <xen/init.h> +#include <xen/kernel.h> +#include <xen/lib.h> +#include <xen/list.h> +#include <xen/spinlock.h> +#include <xen/xsplice_patch.h> + +#define SHADOW_SLOTS 256 +struct hlist_head shadow_tbl[SHADOW_SLOTS]; +static DEFINE_SPINLOCK(shadow_lock); + +struct shadow_var { + struct hlist_node list; /* Linked to 'shadow_tbl' */ + void *data; + const void *obj; + char var[16]; +}; + +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size) +{ + struct shadow_var *shadow; + unsigned int slot; + + shadow = xmalloc(struct shadow_var); + if ( !shadow ) + return NULL; + + shadow->obj = obj; + strlcpy(shadow->var, var, sizeof shadow->var); + shadow->data = xmalloc_bytes(size); + if ( !shadow->data ) + { + xfree(shadow); + return NULL; + } + + slot = (unsigned long)obj % SHADOW_SLOTS; + spin_lock(&shadow_lock); + hlist_add_head(&shadow->list, &shadow_tbl[slot]); + spin_unlock(&shadow_lock); + + return shadow->data; +} + +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) + { + hlist_del(&shadow->list); + xfree(shadow->data); + xfree(shadow); + } + spin_unlock(&shadow_lock); +} + +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; +} + +static int __init xsplice_shadow_init(void) +{ + int i; + + for ( i = 0; i < SHADOW_SLOTS; i++ ) + INIT_HLIST_HEAD(&shadow_tbl[i]); + + return 0; +} +__initcall(xsplice_shadow_init); diff --git a/xen/include/xen/xsplice_patch.h b/xen/include/xen/xsplice_patch.h new file mode 100644 index 0000000..e3f344b --- /dev/null +++ b/xen/include/xen/xsplice_patch.h @@ -0,0 +1,39 @@ +#ifndef __XEN_XSPLICE_PATCH_H__ +#define __XEN_XSPLICE_PATCH_H__ + +/* + * 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"); + */ + +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size); +void xsplice_shadow_free(const void *obj, const char *var); +void *xsplice_shadow_get(const void *obj, const char *var); + +#endif /* __XEN_XSPLICE_PATCH_H__ */