2023-09-19 07:06:08

by Qi Zheng

[permalink] [raw]
Subject: [PATCH] mm: shrinker: some cleanup

Signed-off-by: Qi Zheng <[email protected]>
Reviewed-by: Muchun Song <[email protected]>
---
Hi Andrew, this is a cleanup patch for [PATCH v6 01/45], there will be a
small conflict with [PATCH v6 41/45].

include/linux/shrinker.h | 14 ++++++++------
mm/internal.h | 17 ++++++++++++++---
mm/shrinker.c | 20 ++++++++++++--------
mm/shrinker_debug.c | 16 ----------------
4 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 3f3fd9974ce5..f4a5249f00b2 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -88,16 +88,18 @@ struct shrinker {
};
#define DEFAULT_SEEKS 2 /* A good number if you don't know better. */

-/* Flags */
-#define SHRINKER_REGISTERED (1 << 0)
-#define SHRINKER_NUMA_AWARE (1 << 1)
-#define SHRINKER_MEMCG_AWARE (1 << 2)
+/* Internal flags */
+#define SHRINKER_REGISTERED BIT(0)
+#define SHRINKER_ALLOCATED BIT(1)
+
+/* Flags for users to use */
+#define SHRINKER_NUMA_AWARE BIT(2)
+#define SHRINKER_MEMCG_AWARE BIT(3)
/*
* It just makes sense when the shrinker is also MEMCG_AWARE for now,
* non-MEMCG_AWARE shrinker should not have this flag set.
*/
-#define SHRINKER_NONSLAB (1 << 3)
-#define SHRINKER_ALLOCATED (1 << 4)
+#define SHRINKER_NONSLAB BIT(4)

struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...);
void shrinker_register(struct shrinker *shrinker);
diff --git a/mm/internal.h b/mm/internal.h
index b9a116dce28e..0f418a11c7a8 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1161,10 +1161,21 @@ unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg,
int priority);

#ifdef CONFIG_SHRINKER_DEBUG
+static inline int shrinker_debugfs_name_alloc(struct shrinker *shrinker,
+ const char *fmt, va_list ap)
+{
+ shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
+
+ return shrinker->name ? 0 : -ENOMEM;
+}
+
+static inline void shrinker_debugfs_name_free(struct shrinker *shrinker)
+{
+ kfree_const(shrinker->name);
+ shrinker->name = NULL;
+}
+
extern int shrinker_debugfs_add(struct shrinker *shrinker);
-extern int shrinker_debugfs_name_alloc(struct shrinker *shrinker,
- const char *fmt, va_list ap);
-extern void shrinker_debugfs_name_free(struct shrinker *shrinker);
extern struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker,
int *debugfs_id);
extern void shrinker_debugfs_remove(struct dentry *debugfs_entry,
diff --git a/mm/shrinker.c b/mm/shrinker.c
index 201211a67827..d1032a4d5684 100644
--- a/mm/shrinker.c
+++ b/mm/shrinker.c
@@ -572,18 +572,23 @@ struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...)

if (flags & SHRINKER_MEMCG_AWARE) {
err = prealloc_memcg_shrinker(shrinker);
- if (err == -ENOSYS)
+ if (err == -ENOSYS) {
+ /* Memcg is not supported, fallback to non-memcg-aware shrinker. */
shrinker->flags &= ~SHRINKER_MEMCG_AWARE;
- else if (err == 0)
- goto done;
- else
+ goto non_memcg;
+ }
+
+ if (err)
goto err_flags;
+
+ return shrinker;
}

+non_memcg:
/*
* The nr_deferred is available on per memcg level for memcg aware
* shrinkers, so only allocate nr_deferred in the following cases:
- * - non memcg aware shrinkers
+ * - non-memcg-aware shrinkers
* - !CONFIG_MEMCG
* - memcg is disabled by kernel command line
*/
@@ -595,7 +600,6 @@ struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...)
if (!shrinker->nr_deferred)
goto err_flags;

-done:
return shrinker;

err_flags:
@@ -634,10 +638,10 @@ void shrinker_free(struct shrinker *shrinker)
list_del(&shrinker->list);
debugfs_entry = shrinker_debugfs_detach(shrinker, &debugfs_id);
shrinker->flags &= ~SHRINKER_REGISTERED;
- } else {
- shrinker_debugfs_name_free(shrinker);
}

+ shrinker_debugfs_name_free(shrinker);
+
if (shrinker->flags & SHRINKER_MEMCG_AWARE)
unregister_memcg_shrinker(shrinker);
up_write(&shrinker_rwsem);
diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c
index 38452f539f40..24aebe7c24cc 100644
--- a/mm/shrinker_debug.c
+++ b/mm/shrinker_debug.c
@@ -193,20 +193,6 @@ int shrinker_debugfs_add(struct shrinker *shrinker)
return 0;
}

-int shrinker_debugfs_name_alloc(struct shrinker *shrinker, const char *fmt,
- va_list ap)
-{
- shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap);
-
- return shrinker->name ? 0 : -ENOMEM;
-}
-
-void shrinker_debugfs_name_free(struct shrinker *shrinker)
-{
- kfree_const(shrinker->name);
- shrinker->name = NULL;
-}
-
int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...)
{
struct dentry *entry;
@@ -255,8 +241,6 @@ struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker,

lockdep_assert_held(&shrinker_rwsem);

- shrinker_debugfs_name_free(shrinker);
-
*debugfs_id = entry ? shrinker->debugfs_id : -1;
shrinker->debugfs_entry = NULL;

--
2.30.2


2023-09-19 12:32:38

by Qi Zheng

[permalink] [raw]
Subject: Re: [PATCH] mm: shrinker: some cleanup

Hi Greg,

On 2023/9/19 16:04, Greg KH wrote:
> On Tue, Sep 19, 2023 at 10:46:07AM +0800, Qi Zheng wrote:
>> Signed-off-by: Qi Zheng <[email protected]>
>> Reviewed-by: Muchun Song <[email protected]>
>> ---
>> Hi Andrew, this is a cleanup patch for [PATCH v6 01/45], there will be a
>> small conflict with [PATCH v6 41/45].
>
> I know I can't take patches without any changelog text, but maybe other
> maintainers are more lax.

This patch will be folded into one patch with [PATCH v6 01/45] and will
not enter the mainline as a separate patch, so I was too lazy to write
the commit message.

Anyway, I'll keep your words in mind next time.

Thanks,
Qi

>
> thanks,
>
> greg k-h

2023-09-19 22:04:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] mm: shrinker: some cleanup

On Tue, Sep 19, 2023 at 10:46:07AM +0800, Qi Zheng wrote:
> Signed-off-by: Qi Zheng <[email protected]>
> Reviewed-by: Muchun Song <[email protected]>
> ---
> Hi Andrew, this is a cleanup patch for [PATCH v6 01/45], there will be a
> small conflict with [PATCH v6 41/45].

I know I can't take patches without any changelog text, but maybe other
maintainers are more lax.

thanks,

greg k-h