* SLUB: purpose of sysfs events on cache creation/removal
@ 2019-11-26 12:19 Michal Hocko
2019-11-26 16:32 ` Christopher Lameter
0 siblings, 1 reply; 26+ messages in thread
From: Michal Hocko @ 2019-11-26 12:19 UTC (permalink / raw)
To: Cristopher Lameter; +Cc: LKML, linux-mm
Hi,
I have just learnt about KOBJ_{ADD,REMOVE} sysfs events triggered on
kmem cache creation/removal when SLUB is configured. This functionality
goes all the way down to initial SLUB merge. I do not see any references
in the Documentation explaining what those events are used for and
whether there are any real users.
Could you shed some more light into this?
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-26 12:19 SLUB: purpose of sysfs events on cache creation/removal Michal Hocko @ 2019-11-26 16:32 ` Christopher Lameter 2019-11-26 16:54 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2019-11-26 16:32 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm On Tue, 26 Nov 2019, Michal Hocko wrote: > Hi, > I have just learnt about KOBJ_{ADD,REMOVE} sysfs events triggered on > kmem cache creation/removal when SLUB is configured. This functionality > goes all the way down to initial SLUB merge. I do not see any references > in the Documentation explaining what those events are used for and > whether there are any real users. > > Could you shed some more light into this? I have no idea about what this is. There have been many people who reworked the sysfs support and this has been the cause for a lot of breakage over the years. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-26 16:32 ` Christopher Lameter @ 2019-11-26 16:54 ` Michal Hocko 2019-11-27 15:40 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-11-26 16:54 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm On Tue 26-11-19 16:32:56, Cristopher Lameter wrote: > On Tue, 26 Nov 2019, Michal Hocko wrote: > > > Hi, > > I have just learnt about KOBJ_{ADD,REMOVE} sysfs events triggered on > > kmem cache creation/removal when SLUB is configured. This functionality > > goes all the way down to initial SLUB merge. I do not see any references > > in the Documentation explaining what those events are used for and > > whether there are any real users. > > > > Could you shed some more light into this? > > I have no idea about what this is. It seems to be there since the initial merge. I suspect this is just following a generic sysfs rule that each file has to provide those events? > There have been many people who > reworked the sysfs support and this has been the cause for a lot of > breakage over the years. Remember any specifics? I am mostly interested in potential users. In other words I am thinking to suppress those events. There is already ke knob to control existence of memcg caches but I do not see anything like this for root caches. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-26 16:54 ` Michal Hocko @ 2019-11-27 15:40 ` Christopher Lameter 2019-11-27 16:24 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2019-11-27 15:40 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm On Tue, 26 Nov 2019, Michal Hocko wrote: > > I have no idea about what this is. > > It seems to be there since the initial merge. I suspect this is just > following a generic sysfs rule that each file has to provide those > events? I have never heard of anyone using this. > > There have been many people who > > reworked the sysfs support and this has been the cause for a lot of > > breakage over the years. > > Remember any specifics? The sequencing of setup / teardown of sysfs entries has frequently been a problem and that caused numerous issues with slab initialization as well as kmem cache creation. Initially kmalloc DMA caches were created on demand which caused some issues. Then there was the back and forth with cache aliasing during kmem_cache_create() that caused another set of instabilities. > I am mostly interested in potential users. In other words I am thinking > to suppress those events. There is already ke knob to control existence > of memcg caches but I do not see anything like this for root caches. > I am not aware of any users but the deployments of Linux are so diverse these days that I am not sure that there are no users. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-27 15:40 ` Christopher Lameter @ 2019-11-27 16:24 ` Michal Hocko 2019-11-27 16:26 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-11-27 16:24 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm On Wed 27-11-19 15:40:19, Cristopher Lameter wrote: > On Tue, 26 Nov 2019, Michal Hocko wrote: > > > > I have no idea about what this is. > > > > It seems to be there since the initial merge. I suspect this is just > > following a generic sysfs rule that each file has to provide those > > events? > > I have never heard of anyone using this. > > > > There have been many people who > > > reworked the sysfs support and this has been the cause for a lot of > > > breakage over the years. > > > > Remember any specifics? > > The sequencing of setup / teardown of sysfs entries has frequently been > a problem and that caused numerous issues with slab initialization as well > as kmem cache creation. Initially kmalloc DMA caches were created on > demand which caused some issues. Then there was the back and forth with > cache aliasing during kmem_cache_create() that caused another set of > instabilities. > > > I am mostly interested in potential users. In other words I am thinking > > to suppress those events. There is already ke knob to control existence > > of memcg caches but I do not see anything like this for root caches. > > > > I am not aware of any users but the deployments of Linux are so diverse > these days that I am not sure that there are no users. Would you mind a patch that would add a kernel command line parameter that would work like memcg_sysfs_enabled? The default for the config would be on. Or it would be preferrable to simply drop only events? -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-27 16:24 ` Michal Hocko @ 2019-11-27 16:26 ` Christopher Lameter 2019-11-27 17:43 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2019-11-27 16:26 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm On Wed, 27 Nov 2019, Michal Hocko wrote: > Would you mind a patch that would add a kernel command line parameter > that would work like memcg_sysfs_enabled? The default for the config > would be on. Or it would be preferrable to simply drop only events? Just drop the events may be best. Then we know if someone is using it. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-27 16:26 ` Christopher Lameter @ 2019-11-27 17:43 ` Michal Hocko 2019-12-04 13:28 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-11-27 17:43 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm On Wed 27-11-19 16:26:11, Cristopher Lameter wrote: > On Wed, 27 Nov 2019, Michal Hocko wrote: > > > Would you mind a patch that would add a kernel command line parameter > > that would work like memcg_sysfs_enabled? The default for the config > > would be on. Or it would be preferrable to simply drop only events? > > Just drop the events may be best. Then we know if someone is using it. I would be worried that a lack of events might be surprising and a potential userspace wouldn't know that something has changed. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-11-27 17:43 ` Michal Hocko @ 2019-12-04 13:28 ` Michal Hocko 2019-12-04 15:25 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-12-04 13:28 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm On Wed 27-11-19 18:43:17, Michal Hocko wrote: > On Wed 27-11-19 16:26:11, Cristopher Lameter wrote: > > On Wed, 27 Nov 2019, Michal Hocko wrote: > > > > > Would you mind a patch that would add a kernel command line parameter > > > that would work like memcg_sysfs_enabled? The default for the config > > > would be on. Or it would be preferrable to simply drop only events? > > > > Just drop the events may be best. Then we know if someone is using it. > > I would be worried that a lack of events might be surprising and a > potential userspace wouldn't know that something has changed. It would be great to land with some decision here. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 13:28 ` Michal Hocko @ 2019-12-04 15:25 ` Christopher Lameter 2019-12-04 15:32 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2019-12-04 15:25 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm On Wed, 4 Dec 2019, Michal Hocko wrote: > > > Just drop the events may be best. Then we know if someone is using it. > > > > I would be worried that a lack of events might be surprising and a > > potential userspace wouldn't know that something has changed. > > It would be great to land with some decision here. Drop the events. These are internal kernel structures and supporting notifiers to userspace is a bit unusual. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 15:25 ` Christopher Lameter @ 2019-12-04 15:32 ` Michal Hocko 2019-12-04 16:53 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-12-04 15:32 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm On Wed 04-12-19 15:25:43, Cristopher Lameter wrote: > On Wed, 4 Dec 2019, Michal Hocko wrote: > > > > > Just drop the events may be best. Then we know if someone is using it. > > > > > > I would be worried that a lack of events might be surprising and a > > > potential userspace wouldn't know that something has changed. > > > > It would be great to land with some decision here. > > Drop the events. These are internal kernel structures and supporting > notifiers to userspace is a bit unusual. As I've said I believe it is quite risky. But if you as a maintainer believe this is the right thing to do I will not object. Care to send a patch? -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 15:32 ` Michal Hocko @ 2019-12-04 16:53 ` Christopher Lameter 2019-12-04 17:32 ` Michal Hocko 2020-01-09 14:07 ` Vlastimil Babka 0 siblings, 2 replies; 26+ messages in thread From: Christopher Lameter @ 2019-12-04 16:53 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm On Wed, 4 Dec 2019, Michal Hocko wrote: > As I've said I believe it is quite risky. But if you as a maintainer > believe this is the right thing to do I will not object. Care to send a > patch? From: Christoph Lameter <cl@linux.com> Subject: slub: Remove userspace notifier for cache add/remove Kmem caches are internal kernel structures so it is strange that userspace notifiers would be needed. And I am not aware of any use of these notifiers. These notifiers may just exist because in the initial slub release the sysfs code was copied from another subsystem. Signed-off-by: Christoph Lameter <cl@linux.com> Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c 2019-12-02 15:13:23.948312925 +0000 +++ linux/mm/slub.c 2019-12-04 16:32:34.648550310 +0000 @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { .release = kmem_cache_release, }; -static int uevent_filter(struct kset *kset, struct kobject *kobj) -{ - struct kobj_type *ktype = get_ktype(kobj); - - if (ktype == &slab_ktype) - return 1; - return 0; -} - -static const struct kset_uevent_ops slab_uevent_ops = { - .filter = uevent_filter, -}; - static struct kset *slab_kset; static inline struct kset *cache_kset(struct kmem_cache *s) @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str #ifdef CONFIG_MEMCG kset_unregister(s->memcg_kset); #endif - kobject_uevent(&s->kobj, KOBJ_REMOVE); out: kobject_put(&s->kobj); } @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca } #endif - kobject_uevent(&s->kobj, KOBJ_ADD); if (!unmergeable) { /* Setup first alias */ sysfs_slab_alias(s, s->name); @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) mutex_lock(&slab_mutex); - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); if (!slab_kset) { mutex_unlock(&slab_mutex); pr_err("Cannot register slab subsystem.\n"); ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 16:53 ` Christopher Lameter @ 2019-12-04 17:32 ` Michal Hocko 2020-01-06 11:57 ` Michal Hocko 2020-01-09 14:07 ` Vlastimil Babka 1 sibling, 1 reply; 26+ messages in thread From: Michal Hocko @ 2019-12-04 17:32 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm, Andrew Morton [Cc akpm - the email thread starts http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] On Wed 04-12-19 16:53:47, Cristopher Lameter wrote: > On Wed, 4 Dec 2019, Michal Hocko wrote: > > > As I've said I believe it is quite risky. But if you as a maintainer > > believe this is the right thing to do I will not object. Care to send a > > patch? > > From: Christoph Lameter <cl@linux.com> > Subject: slub: Remove userspace notifier for cache add/remove > > Kmem caches are internal kernel structures so it is strange that > userspace notifiers would be needed. And I am not aware of any use > of these notifiers. These notifiers may just exist because in the > initial slub release the sysfs code was copied from another > subsystem. > > Signed-off-by: Christoph Lameter <cl@linux.com> > > Index: linux/mm/slub.c > =================================================================== > --- linux.orig/mm/slub.c 2019-12-02 15:13:23.948312925 +0000 > +++ linux/mm/slub.c 2019-12-04 16:32:34.648550310 +0000 > @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { > .release = kmem_cache_release, > }; > > -static int uevent_filter(struct kset *kset, struct kobject *kobj) > -{ > - struct kobj_type *ktype = get_ktype(kobj); > - > - if (ktype == &slab_ktype) > - return 1; > - return 0; > -} > - > -static const struct kset_uevent_ops slab_uevent_ops = { > - .filter = uevent_filter, > -}; > - > static struct kset *slab_kset; > > static inline struct kset *cache_kset(struct kmem_cache *s) > @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str > #ifdef CONFIG_MEMCG > kset_unregister(s->memcg_kset); > #endif > - kobject_uevent(&s->kobj, KOBJ_REMOVE); > out: > kobject_put(&s->kobj); > } > @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca > } > #endif > > - kobject_uevent(&s->kobj, KOBJ_ADD); > if (!unmergeable) { > /* Setup first alias */ > sysfs_slab_alias(s, s->name); > @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) > > mutex_lock(&slab_mutex); > > - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); > + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); > if (!slab_kset) { > mutex_unlock(&slab_mutex); > pr_err("Cannot register slab subsystem.\n"); -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 17:32 ` Michal Hocko @ 2020-01-06 11:57 ` Michal Hocko 2020-01-06 15:51 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2020-01-06 11:57 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm, Andrew Morton On Wed 04-12-19 18:32:24, Michal Hocko wrote: > [Cc akpm - the email thread starts > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] ping. > On Wed 04-12-19 16:53:47, Cristopher Lameter wrote: > > On Wed, 4 Dec 2019, Michal Hocko wrote: > > > > > As I've said I believe it is quite risky. But if you as a maintainer > > > believe this is the right thing to do I will not object. Care to send a > > > patch? > > > > From: Christoph Lameter <cl@linux.com> > > Subject: slub: Remove userspace notifier for cache add/remove > > > > Kmem caches are internal kernel structures so it is strange that > > userspace notifiers would be needed. And I am not aware of any use > > of these notifiers. These notifiers may just exist because in the > > initial slub release the sysfs code was copied from another > > subsystem. > > > > Signed-off-by: Christoph Lameter <cl@linux.com> > > > > Index: linux/mm/slub.c > > =================================================================== > > --- linux.orig/mm/slub.c 2019-12-02 15:13:23.948312925 +0000 > > +++ linux/mm/slub.c 2019-12-04 16:32:34.648550310 +0000 > > @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { > > .release = kmem_cache_release, > > }; > > > > -static int uevent_filter(struct kset *kset, struct kobject *kobj) > > -{ > > - struct kobj_type *ktype = get_ktype(kobj); > > - > > - if (ktype == &slab_ktype) > > - return 1; > > - return 0; > > -} > > - > > -static const struct kset_uevent_ops slab_uevent_ops = { > > - .filter = uevent_filter, > > -}; > > - > > static struct kset *slab_kset; > > > > static inline struct kset *cache_kset(struct kmem_cache *s) > > @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str > > #ifdef CONFIG_MEMCG > > kset_unregister(s->memcg_kset); > > #endif > > - kobject_uevent(&s->kobj, KOBJ_REMOVE); > > out: > > kobject_put(&s->kobj); > > } > > @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca > > } > > #endif > > > > - kobject_uevent(&s->kobj, KOBJ_ADD); > > if (!unmergeable) { > > /* Setup first alias */ > > sysfs_slab_alias(s, s->name); > > @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) > > > > mutex_lock(&slab_mutex); > > > > - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); > > + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); > > if (!slab_kset) { > > mutex_unlock(&slab_mutex); > > pr_err("Cannot register slab subsystem.\n"); > > -- > Michal Hocko > SUSE Labs -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-06 11:57 ` Michal Hocko @ 2020-01-06 15:51 ` Christopher Lameter 2020-01-09 14:52 ` Michal Hocko 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2020-01-06 15:51 UTC (permalink / raw) To: Michal Hocko; +Cc: LKML, linux-mm, Andrew Morton On Mon, 6 Jan 2020, Michal Hocko wrote: > On Wed 04-12-19 18:32:24, Michal Hocko wrote: > > [Cc akpm - the email thread starts > > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] > > ping. There does not seem to be much of an interest in the patch? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-06 15:51 ` Christopher Lameter @ 2020-01-09 14:52 ` Michal Hocko 2020-01-09 19:44 ` Andrew Morton 0 siblings, 1 reply; 26+ messages in thread From: Michal Hocko @ 2020-01-09 14:52 UTC (permalink / raw) To: Christopher Lameter; +Cc: LKML, linux-mm, Andrew Morton On Mon 06-01-20 15:51:26, Cristopher Lameter wrote: > On Mon, 6 Jan 2020, Michal Hocko wrote: > > > On Wed 04-12-19 18:32:24, Michal Hocko wrote: > > > [Cc akpm - the email thread starts > > > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] > > > > ping. > > There does not seem to be much of an interest in the patch? It seems it has just fallen through cracks. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-09 14:52 ` Michal Hocko @ 2020-01-09 19:44 ` Andrew Morton 2020-01-09 20:13 ` Michal Hocko ` (2 more replies) 0 siblings, 3 replies; 26+ messages in thread From: Andrew Morton @ 2020-01-09 19:44 UTC (permalink / raw) To: Michal Hocko; +Cc: Christopher Lameter, LKML, linux-mm On Thu, 9 Jan 2020 15:52:36 +0100 Michal Hocko <mhocko@kernel.org> wrote: > On Mon 06-01-20 15:51:26, Cristopher Lameter wrote: > > On Mon, 6 Jan 2020, Michal Hocko wrote: > > > > > On Wed 04-12-19 18:32:24, Michal Hocko wrote: > > > > [Cc akpm - the email thread starts > > > > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] > > > > > > ping. > > > > There does not seem to be much of an interest in the patch? > > It seems it has just fallen through cracks. I looked at it - there wasn't really any compelling followup. If this change should be pursued then can we please have a formal resend? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-09 19:44 ` Andrew Morton @ 2020-01-09 20:13 ` Michal Hocko 2020-01-09 20:15 ` Michal Hocko 2020-01-17 17:13 ` Michal Koutný 2 siblings, 0 replies; 26+ messages in thread From: Michal Hocko @ 2020-01-09 20:13 UTC (permalink / raw) To: Andrew Morton; +Cc: Christopher Lameter, LKML, linux-mm On Thu 09-01-20 11:44:15, Andrew Morton wrote: > On Thu, 9 Jan 2020 15:52:36 +0100 Michal Hocko <mhocko@kernel.org> wrote: > > > On Mon 06-01-20 15:51:26, Cristopher Lameter wrote: > > > On Mon, 6 Jan 2020, Michal Hocko wrote: > > > > > > > On Wed 04-12-19 18:32:24, Michal Hocko wrote: > > > > > [Cc akpm - the email thread starts > > > > > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] > > > > > > > > ping. > > > > > > There does not seem to be much of an interest in the patch? > > > > It seems it has just fallen through cracks. > > I looked at it - there wasn't really any compelling followup. The primary motivation is the pointless udev event for each created cache. There are not that many on the global case but memcg just adds up. -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-09 19:44 ` Andrew Morton 2020-01-09 20:13 ` Michal Hocko @ 2020-01-09 20:15 ` Michal Hocko 2020-01-17 17:13 ` Michal Koutný 2 siblings, 0 replies; 26+ messages in thread From: Michal Hocko @ 2020-01-09 20:15 UTC (permalink / raw) To: Andrew Morton; +Cc: Christopher Lameter, LKML, linux-mm On Thu 09-01-20 11:44:15, Andrew Morton wrote: > On Thu, 9 Jan 2020 15:52:36 +0100 Michal Hocko <mhocko@kernel.org> wrote: > > > On Mon 06-01-20 15:51:26, Cristopher Lameter wrote: > > > On Mon, 6 Jan 2020, Michal Hocko wrote: > > > > > > > On Wed 04-12-19 18:32:24, Michal Hocko wrote: > > > > > [Cc akpm - the email thread starts > > > > > http://lkml.kernel.org/r/20191126121901.GE20912@dhcp22.suse.cz] > > > > > > > > ping. > > > > > > There does not seem to be much of an interest in the patch? > > > > It seems it has just fallen through cracks. > > I looked at it - there wasn't really any compelling followup. Btw. I would appreciate if this was explicit because if there is no reaction then it is not clear whether the patch has slipped through cracks or it is not worth pursuing for whatever reason. Thanks! -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-09 19:44 ` Andrew Morton 2020-01-09 20:13 ` Michal Hocko 2020-01-09 20:15 ` Michal Hocko @ 2020-01-17 17:13 ` Michal Koutný 2020-01-19 0:15 ` Andrew Morton 2 siblings, 1 reply; 26+ messages in thread From: Michal Koutný @ 2020-01-17 17:13 UTC (permalink / raw) To: Andrew Morton; +Cc: Michal Hocko, Christopher Lameter, LKML, linux-mm [-- Attachment #1: Type: text/plain, Size: 1442 bytes --] Hello. On Thu, Jan 09, 2020 at 11:44:15AM -0800, Andrew Morton <akpm@linux-foundation.org> wrote: > I looked at it - there wasn't really any compelling followup. FTR, I noticed udevd consuming non-negligible CPU cycles when doing some cgroup stress testing. And even extrapolating to less artificial situations, the udev events seem to cause useless tickling of udevd. I used the simple script below cat measure.sh <<EOD sample() { local n=$(echo|awk "END {print int(40/$1)}") for i in $(seq $n) ; do mkdir /sys/fs/cgroup/memory/grp1 ; echo 0 >/sys/fs/cgroup/memory/grp1/cgroup.procs ; /usr/bin/sleep $1 ; echo 0 >/sys/fs/cgroup/memory/cgroup.procs ; rmdir /sys/fs/cgroup/memory/grp1 ; done } for d in 0.004 0.008 0.016 0.032 0.064 0.128 0.256 0.5 1 ; do echo 0 >/sys/fs/cgroup/cpuacct/system.slice/systemd-udevd.service/cpuacct.usage time sample $d 2>&1 | grep real echo -n "udev " cat /sys/fs/cgroup/cpuacct/system.slice/systemd-udevd.service/cpuacct.usage done EOD and I drew the following ballpark conclusion: 1.7% CPU time at 1 event/s -> 60 event/s 100% cpu (The event is one mkdir/migrate/rmdir sequence. Numbers are from dummy test VM, so take with a grain of salt.) > If this change should be pursued then can we please have a formal > resend? Who's supposed to do that? Regards, Michal [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-17 17:13 ` Michal Koutný @ 2020-01-19 0:15 ` Andrew Morton 2020-01-27 17:33 ` Michal Koutný 0 siblings, 1 reply; 26+ messages in thread From: Andrew Morton @ 2020-01-19 0:15 UTC (permalink / raw) To: Michal Koutný; +Cc: Michal Hocko, Christopher Lameter, LKML, linux-mm On Fri, 17 Jan 2020 18:13:31 +0100 Michal Koutný <mkoutny@suse.com> wrote: > Hello. > > On Thu, Jan 09, 2020 at 11:44:15AM -0800, Andrew Morton <akpm@linux-foundation.org> wrote: > > I looked at it - there wasn't really any compelling followup. > FTR, I noticed udevd consuming non-negligible CPU cycles when doing some > cgroup stress testing. And even extrapolating to less artificial > situations, the udev events seem to cause useless tickling of udevd. > > I used the simple script below > cat measure.sh <<EOD > sample() { > local n=$(echo|awk "END {print int(40/$1)}") > > for i in $(seq $n) ; do > mkdir /sys/fs/cgroup/memory/grp1 ; > echo 0 >/sys/fs/cgroup/memory/grp1/cgroup.procs ; > /usr/bin/sleep $1 ; > echo 0 >/sys/fs/cgroup/memory/cgroup.procs ; > rmdir /sys/fs/cgroup/memory/grp1 ; > done > } > > for d in 0.004 0.008 0.016 0.032 0.064 0.128 0.256 0.5 1 ; do > echo 0 >/sys/fs/cgroup/cpuacct/system.slice/systemd-udevd.service/cpuacct.usage > time sample $d 2>&1 | grep real > echo -n "udev " > cat /sys/fs/cgroup/cpuacct/system.slice/systemd-udevd.service/cpuacct.usage > done > EOD > > and I drew the following ballpark conclusion: > 1.7% CPU time at 1 event/s -> 60 event/s 100% cpu > > (The event is one mkdir/migrate/rmdir sequence. Numbers are from dummy > test VM, so take with a grain of salt.) Thanks. What effect does this patch have upon these numbers? > > > If this change should be pursued then can we please have a formal > > resend? > Who's supposed to do that? Typically the author, but not always. If someone else is particularly motivated to get a patch merged up they can take it over. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-19 0:15 ` Andrew Morton @ 2020-01-27 17:33 ` Michal Koutný 2020-01-27 23:04 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Koutný @ 2020-01-27 17:33 UTC (permalink / raw) To: Andrew Morton, Christopher Lameter Cc: Michal Hocko, Christopher Lameter, LKML, linux-mm [-- Attachment #1: Type: text/plain, Size: 852 bytes --] On Sat, Jan 18, 2020 at 04:15:28PM -0800, Andrew Morton <akpm@linux-foundation.org> wrote: > > situations, the udev events seem to cause useless tickling of udevd. > > [...] > > and I drew the following ballpark conclusion: > > 1.7% CPU time at 1 event/s -> 60 event/s 100% cpu > Thanks. What effect does this patch have upon these numbers? When I rerun the script with patched kernel, udev sit mostly idle (there were no other udev event sources). So the number can be said to drop to 0% CPU time / event/s. > Typically the author, but not always. If someone else is particularly > motivated to get a patch merged up they can take it over. Christopher, do you consider resending your patch? (I second that it exposes the internal details (wrt cgroup caches) and I can observe the just reading the events by udevd wastes CPU time.) Thanks, Michal [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-27 17:33 ` Michal Koutný @ 2020-01-27 23:04 ` Christopher Lameter 2020-01-28 8:51 ` Michal Koutný 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2020-01-27 23:04 UTC (permalink / raw) To: Michal Koutný; +Cc: Andrew Morton, Michal Hocko, LKML, linux-mm [-- Attachment #1: Type: text/plain, Size: 631 bytes --] On Mon, 27 Jan 2020, Michal Koutný wrote: > When I rerun the script with patched kernel, udev sit mostly idle (there > were no other udev event sources). So the number can be said to drop to > 0% CPU time / event/s. > > > Typically the author, but not always. If someone else is particularly > > motivated to get a patch merged up they can take it over. > Christopher, do you consider resending your patch? (I second that it > exposes the internal details (wrt cgroup caches) and I can observe the > just reading the events by udevd wastes CPU time.) The patch exposes details of cgroup caches? Which patch are we talking about? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-27 23:04 ` Christopher Lameter @ 2020-01-28 8:51 ` Michal Koutný 2020-01-28 18:13 ` Christopher Lameter 0 siblings, 1 reply; 26+ messages in thread From: Michal Koutný @ 2020-01-28 8:51 UTC (permalink / raw) To: Christopher Lameter; +Cc: Andrew Morton, Michal Hocko, LKML, linux-mm [-- Attachment #1: Type: text/plain, Size: 454 bytes --] On Mon, Jan 27, 2020 at 11:04:53PM +0000, Christopher Lameter <cl@linux.com> wrote: > The patch exposes details of cgroup caches? Which patch are we talking > about? Sorry, that's misunderstanding. I mean the current state (sending uevents) exposes the internals (creation of caches per cgroup). The patch [1] removing uevent notifications is rectifying it. Michal [1] https://lore.kernel.org/lkml/alpine.DEB.2.21.1912041652410.29709@www.lameter.com/ [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-28 8:51 ` Michal Koutný @ 2020-01-28 18:13 ` Christopher Lameter 2020-01-30 13:16 ` Vlastimil Babka 0 siblings, 1 reply; 26+ messages in thread From: Christopher Lameter @ 2020-01-28 18:13 UTC (permalink / raw) To: Michal Koutný; +Cc: Andrew Morton, Michal Hocko, LKML, linux-mm [-- Attachment #1: Type: text/plain, Size: 2224 bytes --] On Tue, 28 Jan 2020, Michal Koutný wrote: > On Mon, Jan 27, 2020 at 11:04:53PM +0000, Christopher Lameter <cl@linux.com> wrote: > > The patch exposes details of cgroup caches? Which patch are we talking > > about? > Sorry, that's misunderstanding. I mean the current state (sending > uevents) exposes the internals (creation of caches per cgroup). The > patch [1] removing uevent notifications is rectifying it. From: Christoph Lameter <cl@linux.com> Subject: slub: Remove userspace notifier for cache add/remove Kmem caches are internal kernel structures so it is strange that userspace notifiers would be needed. And I am not aware of any use of these notifiers. These notifiers may just exist because in the initial slub release the sysfs code was copied from another subsystem. Signed-off-by: Christoph Lameter <cl@linux.com> Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c 2020-01-28 18:13:02.134506141 +0000 +++ linux/mm/slub.c 2020-01-28 18:13:02.134506141 +0000 @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { .release = kmem_cache_release, }; -static int uevent_filter(struct kset *kset, struct kobject *kobj) -{ - struct kobj_type *ktype = get_ktype(kobj); - - if (ktype == &slab_ktype) - return 1; - return 0; -} - -static const struct kset_uevent_ops slab_uevent_ops = { - .filter = uevent_filter, -}; - static struct kset *slab_kset; static inline struct kset *cache_kset(struct kmem_cache *s) @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str #ifdef CONFIG_MEMCG kset_unregister(s->memcg_kset); #endif - kobject_uevent(&s->kobj, KOBJ_REMOVE); out: kobject_put(&s->kobj); } @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca } #endif - kobject_uevent(&s->kobj, KOBJ_ADD); if (!unmergeable) { /* Setup first alias */ sysfs_slab_alias(s, s->name); @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) mutex_lock(&slab_mutex); - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); if (!slab_kset) { mutex_unlock(&slab_mutex); pr_err("Cannot register slab subsystem.\n"); ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2020-01-28 18:13 ` Christopher Lameter @ 2020-01-30 13:16 ` Vlastimil Babka 0 siblings, 0 replies; 26+ messages in thread From: Vlastimil Babka @ 2020-01-30 13:16 UTC (permalink / raw) To: Christopher Lameter, Michal Koutný Cc: Andrew Morton, Michal Hocko, LKML, linux-mm On 1/28/20 7:13 PM, Christopher Lameter wrote: > On Tue, 28 Jan 2020, Michal Koutný wrote: > >> On Mon, Jan 27, 2020 at 11:04:53PM +0000, Christopher Lameter <cl@linux.com> wrote: >>> The patch exposes details of cgroup caches? Which patch are we talking >>> about? >> Sorry, that's misunderstanding. I mean the current state (sending >> uevents) exposes the internals (creation of caches per cgroup). The >> patch [1] removing uevent notifications is rectifying it. > > > From: Christoph Lameter <cl@linux.com> > Subject: slub: Remove userspace notifier for cache add/remove > > Kmem caches are internal kernel structures so it is strange that > userspace notifiers would be needed. And I am not aware of any use > of these notifiers. These notifiers may just exist because in the > initial slub release the sysfs code was copied from another > subsystem. > > Signed-off-by: Christoph Lameter <cl@linux.com> Acked-by: Vlastimil Babka <vbabka@suse.cz> > Index: linux/mm/slub.c > =================================================================== > --- linux.orig/mm/slub.c 2020-01-28 18:13:02.134506141 +0000 > +++ linux/mm/slub.c 2020-01-28 18:13:02.134506141 +0000 > @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { > .release = kmem_cache_release, > }; > > -static int uevent_filter(struct kset *kset, struct kobject *kobj) > -{ > - struct kobj_type *ktype = get_ktype(kobj); > - > - if (ktype == &slab_ktype) > - return 1; > - return 0; > -} > - > -static const struct kset_uevent_ops slab_uevent_ops = { > - .filter = uevent_filter, > -}; > - > static struct kset *slab_kset; > > static inline struct kset *cache_kset(struct kmem_cache *s) > @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str > #ifdef CONFIG_MEMCG > kset_unregister(s->memcg_kset); > #endif > - kobject_uevent(&s->kobj, KOBJ_REMOVE); > out: > kobject_put(&s->kobj); > } > @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca > } > #endif > > - kobject_uevent(&s->kobj, KOBJ_ADD); > if (!unmergeable) { > /* Setup first alias */ > sysfs_slab_alias(s, s->name); > @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) > > mutex_lock(&slab_mutex); > > - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); > + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); > if (!slab_kset) { > mutex_unlock(&slab_mutex); > pr_err("Cannot register slab subsystem.\n"); > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: SLUB: purpose of sysfs events on cache creation/removal 2019-12-04 16:53 ` Christopher Lameter 2019-12-04 17:32 ` Michal Hocko @ 2020-01-09 14:07 ` Vlastimil Babka 1 sibling, 0 replies; 26+ messages in thread From: Vlastimil Babka @ 2020-01-09 14:07 UTC (permalink / raw) To: Christopher Lameter, Michal Hocko; +Cc: LKML, linux-mm, Andrew Morton On 12/4/19 5:53 PM, Christopher Lameter wrote: > On Wed, 4 Dec 2019, Michal Hocko wrote: > >> As I've said I believe it is quite risky. But if you as a maintainer >> believe this is the right thing to do I will not object. Care to send a >> patch? > > From: Christoph Lameter <cl@linux.com> > Subject: slub: Remove userspace notifier for cache add/remove > > Kmem caches are internal kernel structures so it is strange that > userspace notifiers would be needed. And I am not aware of any use > of these notifiers. These notifiers may just exist because in the > initial slub release the sysfs code was copied from another > subsystem. > > Signed-off-by: Christoph Lameter <cl@linux.com> If anyone cares, we'll find out eventually. Acked-by: Vlastimil Babka <vbabka@suse.cz> > > Index: linux/mm/slub.c > =================================================================== > --- linux.orig/mm/slub.c 2019-12-02 15:13:23.948312925 +0000 > +++ linux/mm/slub.c 2019-12-04 16:32:34.648550310 +0000 > @@ -5632,19 +5632,6 @@ static struct kobj_type slab_ktype = { > .release = kmem_cache_release, > }; > > -static int uevent_filter(struct kset *kset, struct kobject *kobj) > -{ > - struct kobj_type *ktype = get_ktype(kobj); > - > - if (ktype == &slab_ktype) > - return 1; > - return 0; > -} > - > -static const struct kset_uevent_ops slab_uevent_ops = { > - .filter = uevent_filter, > -}; > - > static struct kset *slab_kset; > > static inline struct kset *cache_kset(struct kmem_cache *s) > @@ -5712,7 +5699,6 @@ static void sysfs_slab_remove_workfn(str > #ifdef CONFIG_MEMCG > kset_unregister(s->memcg_kset); > #endif > - kobject_uevent(&s->kobj, KOBJ_REMOVE); > out: > kobject_put(&s->kobj); > } > @@ -5770,7 +5756,6 @@ static int sysfs_slab_add(struct kmem_ca > } > #endif > > - kobject_uevent(&s->kobj, KOBJ_ADD); > if (!unmergeable) { > /* Setup first alias */ > sysfs_slab_alias(s, s->name); > @@ -5851,7 +5836,7 @@ static int __init slab_sysfs_init(void) > > mutex_lock(&slab_mutex); > > - slab_kset = kset_create_and_add("slab", &slab_uevent_ops, kernel_kobj); > + slab_kset = kset_create_and_add("slab", NULL, kernel_kobj); > if (!slab_kset) { > mutex_unlock(&slab_mutex); > pr_err("Cannot register slab subsystem.\n"); > ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-01-30 13:16 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-26 12:19 SLUB: purpose of sysfs events on cache creation/removal Michal Hocko 2019-11-26 16:32 ` Christopher Lameter 2019-11-26 16:54 ` Michal Hocko 2019-11-27 15:40 ` Christopher Lameter 2019-11-27 16:24 ` Michal Hocko 2019-11-27 16:26 ` Christopher Lameter 2019-11-27 17:43 ` Michal Hocko 2019-12-04 13:28 ` Michal Hocko 2019-12-04 15:25 ` Christopher Lameter 2019-12-04 15:32 ` Michal Hocko 2019-12-04 16:53 ` Christopher Lameter 2019-12-04 17:32 ` Michal Hocko 2020-01-06 11:57 ` Michal Hocko 2020-01-06 15:51 ` Christopher Lameter 2020-01-09 14:52 ` Michal Hocko 2020-01-09 19:44 ` Andrew Morton 2020-01-09 20:13 ` Michal Hocko 2020-01-09 20:15 ` Michal Hocko 2020-01-17 17:13 ` Michal Koutný 2020-01-19 0:15 ` Andrew Morton 2020-01-27 17:33 ` Michal Koutný 2020-01-27 23:04 ` Christopher Lameter 2020-01-28 8:51 ` Michal Koutný 2020-01-28 18:13 ` Christopher Lameter 2020-01-30 13:16 ` Vlastimil Babka 2020-01-09 14:07 ` Vlastimil Babka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).