Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mm: provide interface for retrieving kmem_cache name
@ 2019-11-07 11:54 Knut Omang
  2019-11-07 11:58 ` Michal Hocko
  0 siblings, 1 reply; 9+ messages in thread
From: Knut Omang @ 2019-11-07 11:54 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Christoph Lameter, Pekka Enberg, David Rientjes,
	Joonsoo Kim, Andrew Morton, Knut Omang

With the restructuring done in commit 9adeaa226988
("mm, slab: move memcg_cache_params structure to mm/slab.h")

it is no longer possible for code external to mm to access
the name of a kmem_cache as struct kmem_cache has effectively become
opaque. Having access to the cache name is helpful to kernel testing
infrastructure.

Expose a new function kmem_cache_name to mitigate that.

Signed-off-by: Knut Omang <knut.omang@oracle.com>
---
 include/linux/slab.h | 1 +
 mm/slab_common.c     | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 4d2a2fa55ed5..3298c9db6e46 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -702,6 +702,7 @@ static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
 }
 
 unsigned int kmem_cache_size(struct kmem_cache *s);
+const char *kmem_cache_name(struct kmem_cache *s);
 void __init kmem_cache_init_late(void);
 
 #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index f9fb27b4c843..269a99dc8214 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -82,6 +82,15 @@ unsigned int kmem_cache_size(struct kmem_cache *s)
 }
 EXPORT_SYMBOL(kmem_cache_size);
 
+/*
+ * Get the name of a slab object
+ */
+const char *kmem_cache_name(struct kmem_cache *s)
+{
+	return s->name;
+}
+EXPORT_SYMBOL(kmem_cache_name);
+
 #ifdef CONFIG_DEBUG_VM
 static int kmem_cache_sanity_check(const char *name, unsigned int size)
 {
-- 
2.20.1



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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-07 11:54 [PATCH] mm: provide interface for retrieving kmem_cache name Knut Omang
@ 2019-11-07 11:58 ` Michal Hocko
  2019-11-07 12:26   ` Knut Omang
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2019-11-07 11:58 UTC (permalink / raw)
  To: Knut Omang
  Cc: linux-mm, linux-kernel, Christoph Lameter, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton

On Thu 07-11-19 12:54:04, Knut Omang wrote:
> With the restructuring done in commit 9adeaa226988
> ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> 
> it is no longer possible for code external to mm to access
> the name of a kmem_cache as struct kmem_cache has effectively become
> opaque. Having access to the cache name is helpful to kernel testing
> infrastructure.
> 
> Expose a new function kmem_cache_name to mitigate that.

Who is going to use that symbol? It is preferred that a user is added in
the same patch as the newly added symbol.

> Signed-off-by: Knut Omang <knut.omang@oracle.com>
> ---
>  include/linux/slab.h | 1 +
>  mm/slab_common.c     | 9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 4d2a2fa55ed5..3298c9db6e46 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -702,6 +702,7 @@ static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
>  }
>  
>  unsigned int kmem_cache_size(struct kmem_cache *s);
> +const char *kmem_cache_name(struct kmem_cache *s);
>  void __init kmem_cache_init_late(void);
>  
>  #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index f9fb27b4c843..269a99dc8214 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -82,6 +82,15 @@ unsigned int kmem_cache_size(struct kmem_cache *s)
>  }
>  EXPORT_SYMBOL(kmem_cache_size);
>  
> +/*
> + * Get the name of a slab object
> + */
> +const char *kmem_cache_name(struct kmem_cache *s)
> +{
> +	return s->name;
> +}
> +EXPORT_SYMBOL(kmem_cache_name);
> +
>  #ifdef CONFIG_DEBUG_VM
>  static int kmem_cache_sanity_check(const char *name, unsigned int size)
>  {
> -- 
> 2.20.1

-- 
Michal Hocko
SUSE Labs


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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-07 11:58 ` Michal Hocko
@ 2019-11-07 12:26   ` Knut Omang
  2019-11-07 13:13     ` Michal Hocko
  0 siblings, 1 reply; 9+ messages in thread
From: Knut Omang @ 2019-11-07 12:26 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-mm, linux-kernel, Christoph Lameter, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton

On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > With the restructuring done in commit 9adeaa226988
> > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > 
> > it is no longer possible for code external to mm to access
> > the name of a kmem_cache as struct kmem_cache has effectively become
> > opaque. Having access to the cache name is helpful to kernel testing
> > infrastructure.
> > 
> > Expose a new function kmem_cache_name to mitigate that.
> 
> Who is going to use that symbol? It is preferred that a user is added in
> the same patch as the newly added symbol.

Yes, I am aware that that's the normal practice, 
we're currently using cache->name directly in the kernel 
unit test framework KTF (https://github.com/oracle/ktf/)
which we are working (https://lkml.org/lkml/2019/8/13/111) to get 
into the kernel in one form or another.

To me this seems like a natural part of an API for the kmem_cache
data structure now that it has in effect become opaque, so it seemed 
appropriate to get it in close in time to the patch that no longer 
makes this possible, instead of someone else hitting this down the road.

Thanks,
Knut

> > Signed-off-by: Knut Omang <knut.omang@oracle.com>
> > ---
> >  include/linux/slab.h | 1 +
> >  mm/slab_common.c     | 9 +++++++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 4d2a2fa55ed5..3298c9db6e46 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -702,6 +702,7 @@ static inline void *kzalloc_node(size_t size, gfp_t flags, int node)
> >  }
> >  
> >  unsigned int kmem_cache_size(struct kmem_cache *s);
> > +const char *kmem_cache_name(struct kmem_cache *s);
> >  void __init kmem_cache_init_late(void);
> >  
> >  #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index f9fb27b4c843..269a99dc8214 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -82,6 +82,15 @@ unsigned int kmem_cache_size(struct kmem_cache *s)
> >  }
> >  EXPORT_SYMBOL(kmem_cache_size);
> >  
> > +/*
> > + * Get the name of a slab object
> > + */
> > +const char *kmem_cache_name(struct kmem_cache *s)
> > +{
> > +	return s->name;
> > +}
> > +EXPORT_SYMBOL(kmem_cache_name);
> > +
> >  #ifdef CONFIG_DEBUG_VM
> >  static int kmem_cache_sanity_check(const char *name, unsigned int size)
> >  {
> > -- 
> > 2.20.1



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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-07 12:26   ` Knut Omang
@ 2019-11-07 13:13     ` Michal Hocko
  2019-11-07 20:50       ` David Rientjes
  2019-11-08 15:37       ` Christopher Lameter
  0 siblings, 2 replies; 9+ messages in thread
From: Michal Hocko @ 2019-11-07 13:13 UTC (permalink / raw)
  To: Knut Omang
  Cc: linux-mm, linux-kernel, Christoph Lameter, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton

On Thu 07-11-19 13:26:09, Knut Omang wrote:
> On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > With the restructuring done in commit 9adeaa226988
> > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > 
> > > it is no longer possible for code external to mm to access
> > > the name of a kmem_cache as struct kmem_cache has effectively become
> > > opaque. Having access to the cache name is helpful to kernel testing
> > > infrastructure.
> > > 
> > > Expose a new function kmem_cache_name to mitigate that.
> > 
> > Who is going to use that symbol? It is preferred that a user is added in
> > the same patch as the newly added symbol.
> 
> Yes, I am aware that that's the normal practice, 
> we're currently using cache->name directly in the kernel 
> unit test framework KTF (https://github.com/oracle/ktf/)
> which we are working (https://lkml.org/lkml/2019/8/13/111) to get 
> into the kernel in one form or another.

Please add the export with a patch that really needs it.

> To me this seems like a natural part of an API for the kmem_cache
> data structure now that it has in effect become opaque, so it seemed 
> appropriate to get it in close in time to the patch that no longer 
> makes this possible, instead of someone else hitting this down the road.

Well, this is something for SLAB maintainers but I do not really think
the name is something the in kernel code should care about. It is solely
for presenting reasonable statistics to the userspace and that code
workds just fine AFAIK.
-- 
Michal Hocko
SUSE Labs


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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-07 13:13     ` Michal Hocko
@ 2019-11-07 20:50       ` David Rientjes
  2019-11-08 15:37       ` Christopher Lameter
  1 sibling, 0 replies; 9+ messages in thread
From: David Rientjes @ 2019-11-07 20:50 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Knut Omang, linux-mm, linux-kernel, Christoph Lameter,
	Pekka Enberg, Joonsoo Kim, Andrew Morton

On Thu, 7 Nov 2019, Michal Hocko wrote:

> On Thu 07-11-19 13:26:09, Knut Omang wrote:
> > On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > > With the restructuring done in commit 9adeaa226988
> > > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > > 
> > > > it is no longer possible for code external to mm to access
> > > > the name of a kmem_cache as struct kmem_cache has effectively become
> > > > opaque. Having access to the cache name is helpful to kernel testing
> > > > infrastructure.
> > > > 
> > > > Expose a new function kmem_cache_name to mitigate that.
> > > 
> > > Who is going to use that symbol? It is preferred that a user is added in
> > > the same patch as the newly added symbol.
> > 
> > Yes, I am aware that that's the normal practice, 
> > we're currently using cache->name directly in the kernel 
> > unit test framework KTF (https://github.com/oracle/ktf/)
> > which we are working (https://lkml.org/lkml/2019/8/13/111) to get 
> > into the kernel in one form or another.
> 
> Please add the export with a patch that really needs it.
> 

Agree, this patch could be added as a predecessor to the series at
https://lkml.org/lkml/2019/8/13/111 which would use it.  If it's obvious 
why using the kmem cache name is useful in that series, acking this will 
be a no brainer.


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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-07 13:13     ` Michal Hocko
  2019-11-07 20:50       ` David Rientjes
@ 2019-11-08 15:37       ` Christopher Lameter
  2019-11-08 16:44         ` Knut Omang
  1 sibling, 1 reply; 9+ messages in thread
From: Christopher Lameter @ 2019-11-08 15:37 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Knut Omang, linux-mm, linux-kernel, Pekka Enberg, David Rientjes,
	Joonsoo Kim, Andrew Morton

On Thu, 7 Nov 2019, Michal Hocko wrote:

> On Thu 07-11-19 13:26:09, Knut Omang wrote:
> > On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > > With the restructuring done in commit 9adeaa226988
> > > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > >
> > > > it is no longer possible for code external to mm to access

That patch only affected the memcg_cache_params structure and not
kmem_cache.

And I do not see any references to the memcg_cache_param?

The fields that all allocators need to expose are listed in
the struct kmme_cache definition in linux/mm/slab.h.



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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-08 15:37       ` Christopher Lameter
@ 2019-11-08 16:44         ` Knut Omang
  2019-11-08 16:49           ` Christopher Lameter
  0 siblings, 1 reply; 9+ messages in thread
From: Knut Omang @ 2019-11-08 16:44 UTC (permalink / raw)
  To: Christopher Lameter, Michal Hocko
  Cc: linux-mm, linux-kernel, Pekka Enberg, David Rientjes,
	Joonsoo Kim, Andrew Morton

On Fri, 2019-11-08 at 15:37 +0000, Christopher Lameter wrote:
> On Thu, 7 Nov 2019, Michal Hocko wrote:
> 
> > On Thu 07-11-19 13:26:09, Knut Omang wrote:
> > > On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > > > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > > > With the restructuring done in commit 9adeaa226988
> > > > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > > >
> > > > > it is no longer possible for code external to mm to access
> 
> That patch only affected the memcg_cache_params structure and not
> kmem_cache.
> 
> And I do not see any references to the memcg_cache_param?

Good point, I should have made explicit reference to it. 

It gets inlined into kmem_cache with CONFIG_SLUB if CONFIG_MEMCG is set
(include/linux/slub_def.h, line 112)

> The fields that all allocators need to expose are listed in
> the struct kmme_cache definition in linux/mm/slab.h.

So I take that kmem_cache::name was still intended to be public, 
just that that broke due to the inlining of struct memcg_cache_param 
in slub_def.h?

Knut



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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-08 16:44         ` Knut Omang
@ 2019-11-08 16:49           ` Christopher Lameter
  2019-11-08 20:40             ` Knut Omang
  0 siblings, 1 reply; 9+ messages in thread
From: Christopher Lameter @ 2019-11-08 16:49 UTC (permalink / raw)
  To: Knut Omang
  Cc: Michal Hocko, linux-mm, linux-kernel, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton

On Fri, 8 Nov 2019, Knut Omang wrote:

> On Fri, 2019-11-08 at 15:37 +0000, Christopher Lameter wrote:
> > On Thu, 7 Nov 2019, Michal Hocko wrote:
> >
> > > On Thu 07-11-19 13:26:09, Knut Omang wrote:
> > > > On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > > > > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > > > > With the restructuring done in commit 9adeaa226988
> > > > > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > > > >
> > > > > > it is no longer possible for code external to mm to access
> >
> > That patch only affected the memcg_cache_params structure and not
> > kmem_cache.
> >
> > And I do not see any references to the memcg_cache_param?
>
> Good point, I should have made explicit reference to it.
>
> It gets inlined into kmem_cache with CONFIG_SLUB if CONFIG_MEMCG is set
> (include/linux/slub_def.h, line 112)

Yes but that does not affect the "name" field on line 105

> > The fields that all allocators need to expose are listed in
> > the struct kmme_cache definition in linux/mm/slab.h.
>
> So I take that kmem_cache::name was still intended to be public,
> just that that broke due to the inlining of struct memcg_cache_param
> in slub_def.h?

The patch did not change the name field. I am not sure what is broken?

Maybe you need memcg_params to compose a name of the memcg with the slab
name?



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

* Re: [PATCH] mm: provide interface for retrieving kmem_cache name
  2019-11-08 16:49           ` Christopher Lameter
@ 2019-11-08 20:40             ` Knut Omang
  0 siblings, 0 replies; 9+ messages in thread
From: Knut Omang @ 2019-11-08 20:40 UTC (permalink / raw)
  To: Christopher Lameter
  Cc: Michal Hocko, linux-mm, linux-kernel, Pekka Enberg,
	David Rientjes, Joonsoo Kim, Andrew Morton

On Fri, 2019-11-08 at 16:49 +0000, Christopher Lameter wrote:
> On Fri, 8 Nov 2019, Knut Omang wrote:
> 
> > On Fri, 2019-11-08 at 15:37 +0000, Christopher Lameter wrote:
> > > On Thu, 7 Nov 2019, Michal Hocko wrote:
> > >
> > > > On Thu 07-11-19 13:26:09, Knut Omang wrote:
> > > > > On Thu, 2019-11-07 at 12:58 +0100, Michal Hocko wrote:
> > > > > > On Thu 07-11-19 12:54:04, Knut Omang wrote:
> > > > > > > With the restructuring done in commit 9adeaa226988
> > > > > > > ("mm, slab: move memcg_cache_params structure to mm/slab.h")
> > > > > > >
> > > > > > > it is no longer possible for code external to mm to access
> > >
> > > That patch only affected the memcg_cache_params structure and not
> > > kmem_cache.
> > >
> > > And I do not see any references to the memcg_cache_param?
> >
> > Good point, I should have made explicit reference to it.
> >
> > It gets inlined into kmem_cache with CONFIG_SLUB if CONFIG_MEMCG is set
> > (include/linux/slub_def.h, line 112)
> 
> Yes but that does not affect the "name" field on line 105
> 
> > > The fields that all allocators need to expose are listed in
> > > the struct kmme_cache definition in linux/mm/slab.h.
> >
> > So I take that kmem_cache::name was still intended to be public,
> > just that that broke due to the inlining of struct memcg_cache_param
> > in slub_def.h?
> 
> The patch did not change the name field. I am not sure what is broken?
> 
> Maybe you need memcg_params to compose a name of the memcg with the slab
> name?

No, it's just that the slub definition of struct kmem_cache won't compile because
the definition of struct memcg_cache_params is no longer available.

Knut



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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 11:54 [PATCH] mm: provide interface for retrieving kmem_cache name Knut Omang
2019-11-07 11:58 ` Michal Hocko
2019-11-07 12:26   ` Knut Omang
2019-11-07 13:13     ` Michal Hocko
2019-11-07 20:50       ` David Rientjes
2019-11-08 15:37       ` Christopher Lameter
2019-11-08 16:44         ` Knut Omang
2019-11-08 16:49           ` Christopher Lameter
2019-11-08 20:40             ` Knut Omang

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git