Messages in this thread | | | Date | Thu, 23 Nov 2023 11:32:59 +0800 | Subject | Re: [PATCH 2/7] mm: shrinker: Add a .to_text() method for shrinkers | From | Qi Zheng <> |
| |
Hi Kent,
On 2023/11/23 07:25, Kent Overstreet wrote: > This adds a new callback method to shrinkers which they can use to > describe anything relevant to memory reclaim about their internal state, > for example object dirtyness.
I have no objection to this proposal, but maybe it would be better to place this feature under CONFIG_SHRINKER_DEBUG?
Hi Roman, what do you think?
Also +CC Dave.
More comments below.
> > This patch also adds shrinkers_to_text(), which reports on the top 10 > shrinkers - by object count - in sorted order, to be used in OOM > reporting. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Qi Zheng <zhengqi.arch@bytedance.com> > Cc: Roman Gushchin <roman.gushchin@linux.dev> > Cc: linux-mm@kvack.org > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > --- > include/linux/shrinker.h | 6 +++- > mm/shrinker.c | 73 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 77 insertions(+), 2 deletions(-) > > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h > index 1a00be90d93a..968c55474e78 100644 > --- a/include/linux/shrinker.h > +++ b/include/linux/shrinker.h > @@ -24,6 +24,8 @@ struct shrinker_info { > struct shrinker_info_unit *unit[]; > }; > > +struct seq_buf; > + > /* > * This struct is used to pass information from page reclaim to the shrinkers. > * We consolidate the values for easier extension later. > @@ -80,10 +82,12 @@ struct shrink_control { > * @flags determine the shrinker abilities, like numa awareness > */ > struct shrinker { > + const char *name; > unsigned long (*count_objects)(struct shrinker *, > struct shrink_control *sc); > unsigned long (*scan_objects)(struct shrinker *, > struct shrink_control *sc); > + void (*to_text)(struct seq_buf *, struct shrinker *);
The "to_text" looks a little strange, how about naming it "stat_objects"?
> > long batch; /* reclaim batch size, 0 = default */ > int seeks; /* seeks to recreate an obj */ > @@ -110,7 +114,6 @@ struct shrinker { > #endif > #ifdef CONFIG_SHRINKER_DEBUG > int debugfs_id; > - const char *name;
The name will only be allocated memory when the CONFIG_SHRINKER_DEBUG is enabled, otherwise its value is NULL. So you can't move it out and use it while CONFIG_SHRINKER_DEBUG is disabled.
> struct dentry *debugfs_entry; > #endif > /* objs pending delete, per node */ > @@ -135,6 +138,7 @@ __printf(2, 3) > struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...); > void shrinker_register(struct shrinker *shrinker); > void shrinker_free(struct shrinker *shrinker); > +void shrinkers_to_text(struct seq_buf *); > > static inline bool shrinker_try_get(struct shrinker *shrinker) > { > diff --git a/mm/shrinker.c b/mm/shrinker.c > index dd91eab43ed3..4976dbac4c83 100644 > --- a/mm/shrinker.c > +++ b/mm/shrinker.c > @@ -1,8 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/memcontrol.h> > +#include <linux/rculist.h> > #include <linux/rwsem.h> > +#include <linux/seq_buf.h> > #include <linux/shrinker.h> > -#include <linux/rculist.h> > #include <trace/events/vmscan.h> > > #include "internal.h" > @@ -807,3 +808,73 @@ void shrinker_free(struct shrinker *shrinker) > call_rcu(&shrinker->rcu, shrinker_free_rcu_cb); > } > EXPORT_SYMBOL_GPL(shrinker_free); > + > +void shrinker_to_text(struct seq_buf *out, struct shrinker *shrinker) > +{ > + struct shrink_control sc = { .gfp_mask = GFP_KERNEL, }; > + > + seq_buf_puts(out, shrinker->name); > + seq_buf_printf(out, " objects: %lu\n", shrinker->count_objects(shrinker, &sc)); > + > + if (shrinker->to_text) { > + shrinker->to_text(out, shrinker); > + seq_buf_puts(out, "\n"); > + } > +} > + > +/** > + * shrinkers_to_text - Report on shrinkers with highest usage > + * > + * This reports on the top 10 shrinkers, by object counts, in sorted order: > + * intended to be used for OOM reporting. > + */ > +void shrinkers_to_text(struct seq_buf *out) > +{ > + struct shrinker *shrinker; > + struct shrinker_by_mem { > + struct shrinker *shrinker; > + unsigned long mem;
The "mem" also looks a little strange, how about naming it "freeable"?
> + } shrinkers_by_mem[10]; > + int i, nr = 0; > + > + if (!mutex_trylock(&shrinker_mutex)) { > + seq_buf_puts(out, "(couldn't take shrinker lock)"); > + return; > + }
We now have lockless method (RCU + refcount) to iterate shrinker_list, please refer to shrink_slab().
Thanks, Qi
> + > + list_for_each_entry(shrinker, &shrinker_list, list) { > + struct shrink_control sc = { .gfp_mask = GFP_KERNEL, }; > + unsigned long mem = shrinker->count_objects(shrinker, &sc); > + > + if (!mem || mem == SHRINK_STOP || mem == SHRINK_EMPTY) > + continue; > + > + for (i = 0; i < nr; i++) > + if (mem < shrinkers_by_mem[i].mem) > + break; > + > + if (nr < ARRAY_SIZE(shrinkers_by_mem)) { > + memmove(&shrinkers_by_mem[i + 1], > + &shrinkers_by_mem[i], > + sizeof(shrinkers_by_mem[0]) * (nr - i)); > + nr++; > + } else if (i) { > + i--; > + memmove(&shrinkers_by_mem[0], > + &shrinkers_by_mem[1], > + sizeof(shrinkers_by_mem[0]) * i); > + } else { > + continue; > + } > + > + shrinkers_by_mem[i] = (struct shrinker_by_mem) { > + .shrinker = shrinker, > + .mem = mem, > + }; > + } > + > + for (i = nr - 1; i >= 0; --i) > + shrinker_to_text(out, shrinkers_by_mem[i].shrinker); > + > + mutex_unlock(&shrinker_mutex); > +}
| |