| Message ID | 20210428153542.2814175-22-Liam.Howlett@Oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing the Maple Tree | expand |
On Wed, Apr 28, 2021 at 8:36 AM Liam Howlett <liam.howlett@oracle.com> wrote: > I know you have v2 for the first part of this patchset, I'm just going over the whole thing... There should be some description here of what the new struct member and new function are for. Ideally you would also split it in two because it introduces two seemingly independent additions: non_kernel and kmem_cache_get_alloc. > Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > tools/testing/radix-tree/linux.c | 16 +++++++++++++++- > tools/testing/radix-tree/linux/kernel.h | 1 + > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/radix-tree/linux.c b/tools/testing/radix-tree/linux.c > index 2d9c59df60de..93f7de81fbe8 100644 > --- a/tools/testing/radix-tree/linux.c > +++ b/tools/testing/radix-tree/linux.c > @@ -24,15 +24,28 @@ struct kmem_cache { > int nr_objs; > void *objs; > void (*ctor)(void *); > + unsigned int non_kernel; > }; > > +void kmem_cache_set_non_kernel(struct kmem_cache *cachep, unsigned int val) > +{ > + cachep->non_kernel = val; > +} > + > +unsigned long kmem_cache_get_alloc(struct kmem_cache *cachep) > +{ > + return cachep->size * nr_allocated; IIUC nr_allocated is incremented/decremented every time memory is allocated/freed from *any* kmem_cache. Each kmem_cache has its own size. So, nr_allocated counts allocated objects of potentially different sizes. If that is so then I'm unclear what the result of this multiplication would represent. > +} > void *kmem_cache_alloc(struct kmem_cache *cachep, int gfp) > { > void *p; > > - if (!(gfp & __GFP_DIRECT_RECLAIM)) > + if (!(gfp & __GFP_DIRECT_RECLAIM) && !cachep->non_kernel) > return NULL; > > + if (!(gfp & __GFP_DIRECT_RECLAIM)) > + cachep->non_kernel--; > + > pthread_mutex_lock(&cachep->lock); > if (cachep->nr_objs) { > struct radix_tree_node *node = cachep->objs; > @@ -116,5 +129,6 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align, > ret->nr_objs = 0; > ret->objs = NULL; > ret->ctor = ctor; > + ret->non_kernel = 0; > return ret; > } > diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h > index 39867fd80c8f..c5c9d05f29da 100644 > --- a/tools/testing/radix-tree/linux/kernel.h > +++ b/tools/testing/radix-tree/linux/kernel.h > @@ -14,6 +14,7 @@ > #include "../../../include/linux/kconfig.h" > > #define printk printf > +#define pr_err printk > #define pr_info printk > #define pr_debug printk > #define pr_cont printk > -- > 2.30.2
* Suren Baghdasaryan <surenb@google.com> [210528 13:31]: > On Wed, Apr 28, 2021 at 8:36 AM Liam Howlett <liam.howlett@oracle.com> wrote: > > > > I know you have v2 for the first part of this patchset, I'm just going > over the whole thing... There should be some description here of what > the new struct member and new function are for. Ideally you would also > split it in two because it introduces two seemingly independent > additions: non_kernel and kmem_cache_get_alloc. Your comments are still valid and appreciated. I did add a description to the patch: -------------------------------- radix tree test suite: Add kmem_cache enhancements and pr_err Add kmem_cache_set_non_kernel(), a mechanism to allow a certain number of kmem_cache_alloc requests to succeed even when GFP_KERNEL is not set in the flags. Add kmem_cache_get_alloc() to see the size of the allocated kmem_cache. Add a define of pr_err to printk. -------------------------------- I did group these two changes together as they were both affecting kmem_cache. I will reorganize them into separate commits. > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com> > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > tools/testing/radix-tree/linux.c | 16 +++++++++++++++- > > tools/testing/radix-tree/linux/kernel.h | 1 + > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/radix-tree/linux.c b/tools/testing/radix-tree/linux.c > > index 2d9c59df60de..93f7de81fbe8 100644 > > --- a/tools/testing/radix-tree/linux.c > > +++ b/tools/testing/radix-tree/linux.c > > @@ -24,15 +24,28 @@ struct kmem_cache { > > int nr_objs; > > void *objs; > > void (*ctor)(void *); > > + unsigned int non_kernel; > > }; > > > > +void kmem_cache_set_non_kernel(struct kmem_cache *cachep, unsigned int val) > > +{ > > + cachep->non_kernel = val; > > +} > > + > > +unsigned long kmem_cache_get_alloc(struct kmem_cache *cachep) > > +{ > > + return cachep->size * nr_allocated; > > IIUC nr_allocated is incremented/decremented every time memory is > allocated/freed from *any* kmem_cache. Each kmem_cache has its own > size. So, nr_allocated counts allocated objects of potentially > different sizes. If that is so then I'm unclear what the result of > this multiplication would represent. This is intended to only be used for testing with one kmem_cache, so it hasn't been an issue. Having this variable exist external to the kmem_cache struct allows for checking if any allocations remain outside the scope of the kmem_cache (ie: threads). I think putting it in the struct would cause issues with the IDR testing. I could make this a new variable and increment them together but this variable existed for the IDR testing and I didn't need to support that additional functionality for my testing. I should at least add a comment about this limitation though. > > > +} > > void *kmem_cache_alloc(struct kmem_cache *cachep, int gfp) > > { > > void *p; > > > > - if (!(gfp & __GFP_DIRECT_RECLAIM)) > > + if (!(gfp & __GFP_DIRECT_RECLAIM) && !cachep->non_kernel) > > return NULL; > > > > + if (!(gfp & __GFP_DIRECT_RECLAIM)) > > + cachep->non_kernel--; > > + > > pthread_mutex_lock(&cachep->lock); > > if (cachep->nr_objs) { > > struct radix_tree_node *node = cachep->objs; > > @@ -116,5 +129,6 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align, > > ret->nr_objs = 0; > > ret->objs = NULL; > > ret->ctor = ctor; > > + ret->non_kernel = 0; > > return ret; > > } > > diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h > > index 39867fd80c8f..c5c9d05f29da 100644 > > --- a/tools/testing/radix-tree/linux/kernel.h > > +++ b/tools/testing/radix-tree/linux/kernel.h > > @@ -14,6 +14,7 @@ > > #include "../../../include/linux/kconfig.h" > > > > #define printk printf > > +#define pr_err printk > > #define pr_info printk > > #define pr_debug printk > > #define pr_cont printk > > -- > > 2.30.2
diff --git a/tools/testing/radix-tree/linux.c b/tools/testing/radix-tree/linux.c index 2d9c59df60de..93f7de81fbe8 100644 --- a/tools/testing/radix-tree/linux.c +++ b/tools/testing/radix-tree/linux.c @@ -24,15 +24,28 @@ struct kmem_cache { int nr_objs; void *objs; void (*ctor)(void *); + unsigned int non_kernel; }; +void kmem_cache_set_non_kernel(struct kmem_cache *cachep, unsigned int val) +{ + cachep->non_kernel = val; +} + +unsigned long kmem_cache_get_alloc(struct kmem_cache *cachep) +{ + return cachep->size * nr_allocated; +} void *kmem_cache_alloc(struct kmem_cache *cachep, int gfp) { void *p; - if (!(gfp & __GFP_DIRECT_RECLAIM)) + if (!(gfp & __GFP_DIRECT_RECLAIM) && !cachep->non_kernel) return NULL; + if (!(gfp & __GFP_DIRECT_RECLAIM)) + cachep->non_kernel--; + pthread_mutex_lock(&cachep->lock); if (cachep->nr_objs) { struct radix_tree_node *node = cachep->objs; @@ -116,5 +129,6 @@ kmem_cache_create(const char *name, unsigned int size, unsigned int align, ret->nr_objs = 0; ret->objs = NULL; ret->ctor = ctor; + ret->non_kernel = 0; return ret; } diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h index 39867fd80c8f..c5c9d05f29da 100644 --- a/tools/testing/radix-tree/linux/kernel.h +++ b/tools/testing/radix-tree/linux/kernel.h @@ -14,6 +14,7 @@ #include "../../../include/linux/kconfig.h" #define printk printf +#define pr_err printk #define pr_info printk #define pr_debug printk #define pr_cont printk