On Mon, Aug 26, 2024 at 11:01:50AM GMT, Qi Zheng wrote:
>
>
> On 2024/8/25 03:10, 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.
> >
> > 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@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> > Cc: Roman Gushchin <roman.gushchin@xxxxxxxxx>
> > Cc: linux-mm@xxxxxxxxx
> > Signed-off-by: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> > ---
> > include/linux/shrinker.h | 7 +++-
> > mm/shrinker.c | 73 +++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 78 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
> > index 1a00be90d93a..6193612617a1 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 *);
> > 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;
> > struct dentry *debugfs_entry;
> > #endif
> > /* objs pending delete, per node */
> > @@ -135,6 +138,8 @@ __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 shrinker_to_text(struct seq_buf *, struct 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 dc5d2a6fcfc4..ad52c269bb48 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;
> > + } shrinkers_by_mem[10];
> > + int i, nr = 0;
> > +
> > + if (!mutex_trylock(&shrinker_mutex)) {
> > + seq_buf_puts(out, "(couldn't take shrinker lock)");
> > + return;
> > + }
>
> I remember I pointed out that the RCU + refcount method should be used
> here. Otherwise you will block other shrinkers from
> registering/unregistering, etc.
The more complex iteration isn't needed here - this is a slowpath
function and we're not doing anything blocking inside it.
