Hi Stephen,
On 2023/9/12 10:04, Stephen Rothwell wrote:
> Hi all,
>
> After merging the bcachefs tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> fs/bcachefs/btree_cache.c: In function 'bch2_fs_btree_cache_exit':
> fs/bcachefs/btree_cache.c:403:9: error: implicit declaration of function 'unregister_shrinker'; did you mean 'unregister_chrdev'? [-Werror=implicit-function-declaration]
> 403 | unregister_shrinker(&bc->shrink);
> | ^~~~~~~~~~~~~~~~~~~
> | unregister_chrdev
> fs/bcachefs/btree_cache.c: In function 'bch2_fs_btree_cache_init':
> fs/bcachefs/btree_cache.c:479:15: error: implicit declaration of function 'register_shrinker'; did you mean 'register_chrdev'? [-Werror=implicit-function-declaration]
> 479 | ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
> | ^~~~~~~~~~~~~~~~~
> | register_chrdev
> cc1: all warnings being treated as errors
>
> Caused by commits
>
> 5ec30115c066 ("bcachefs: Initial commit")
>
> interacting with commit
>
> eba045d9350d ("mm: shrinker: remove old APIs")
>
> from v6.6-rc1.
>
> I have applied the following merge resolution patch for today. More may
> be needed.
Thanks for doing this! Some needed fixes below.
>
> From 801ad185700d9a7abcf156233b9db6cf6d831581 Mon Sep 17 00:00:00 2001
> From: Stephen Rothwell <[email protected]>
> Date: Tue, 12 Sep 2023 11:27:22 +1000
> Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> fs/bcachefs/btree_cache.c | 19 +++++++++++--------
> fs/bcachefs/btree_key_cache.c | 18 +++++++++++-------
> fs/bcachefs/btree_types.h | 4 ++--
> fs/bcachefs/fs.c | 2 +-
> fs/bcachefs/sysfs.c | 2 +-
> 5 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
> index 245ddd92b2d1..7f0eded6c296 100644
> --- a/fs/bcachefs/btree_cache.c
> +++ b/fs/bcachefs/btree_cache.c
> @@ -285,7 +285,7 @@ static int btree_node_write_and_reclaim(struct bch_fs *c, struct btree *b)
> static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> btree_cache.shrink);
The shrink passed in here will be a local variable, so its address can
not be used directly. So need to be modified as follows:
struct bch_fs *c = shrink->private_data;
> struct btree_cache *bc = &c->btree_cache;
> struct btree *b, *t;
> @@ -384,7 +384,7 @@ static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> static unsigned long bch2_btree_cache_count(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> btree_cache.shrink);
Ditto.
> struct btree_cache *bc = &c->btree_cache;
>
> @@ -400,7 +400,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
> struct btree *b;
> unsigned i, flags;
>
> - unregister_shrinker(&bc->shrink);
> + shrinker_free(bc->shrink);
>
> /* vfree() can allocate memory: */
> flags = memalloc_nofs_save();
> @@ -454,6 +454,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
> int bch2_fs_btree_cache_init(struct bch_fs *c)
> {
> struct btree_cache *bc = &c->btree_cache;
> + struct shrinker *shrink;
> unsigned i;
> int ret = 0;
>
> @@ -473,12 +474,14 @@ int bch2_fs_btree_cache_init(struct bch_fs *c)
>
> mutex_init(&c->verify_lock);
>
> - bc->shrink.count_objects = bch2_btree_cache_count;
> - bc->shrink.scan_objects = bch2_btree_cache_scan;
> - bc->shrink.seeks = 4;
> - ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
> - if (ret)
> + shrink = shrinker_alloc(0, "%s/btree_cache", c->name);
> + if (!shrink)
> goto err;
Here the 'ret' needs to be set to -ENOMEM.
if (!shrink) {
ret = -ENOMEM;
goto err;
}
> + bc->shrink = shrink;
> + shrink->count_objects = bch2_btree_cache_count;
> + shrink->scan_objects = bch2_btree_cache_scan;
> + shrink->seeks = 4;
shrink->private_data = c;
> + shrinker_register(shrink);
>
> return 0;
> err:
> diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
> index 505e7c365ab7..88d33690233b 100644
> --- a/fs/bcachefs/btree_key_cache.c
> +++ b/fs/bcachefs/btree_key_cache.c
> @@ -838,7 +838,7 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
> static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> btree_key_cache.shrink);
struct bch_fs *c = shrink->private_data;
> struct btree_key_cache *bc = &c->btree_key_cache;
> struct bucket_table *tbl;
> @@ -936,7 +936,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> static unsigned long bch2_btree_key_cache_count(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> btree_key_cache.shrink);
Ditto.
> struct btree_key_cache *bc = &c->btree_key_cache;
> long nr = atomic_long_read(&bc->nr_keys) -
> @@ -957,7 +957,7 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
> int cpu;
> #endif
>
> - unregister_shrinker(&bc->shrink);
> + shrinker_free(bc->shrink);
>
> mutex_lock(&bc->lock);
>
> @@ -1031,6 +1031,7 @@ void bch2_fs_btree_key_cache_init_early(struct btree_key_cache *c)
> int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
> {
> struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
> + struct shrinker *shrink;
>
> #ifdef __KERNEL__
> bc->pcpu_freed = alloc_percpu(struct btree_key_cache_freelist);
> @@ -1043,11 +1044,14 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
>
> bc->table_init_done = true;
>
> - bc->shrink.seeks = 0;
> - bc->shrink.count_objects = bch2_btree_key_cache_count;
> - bc->shrink.scan_objects = bch2_btree_key_cache_scan;
> - if (register_shrinker(&bc->shrink, "%s/btree_key_cache", c->name))
> + shrink = shrinker_alloc(0, "%s/btree_key_cache", c->name);
> + if (!shrink)
> return -BCH_ERR_ENOMEM_fs_btree_cache_init;
> + bc->shrink = shrink;
> + shrink->seeks = 0;
> + shrink->count_objects = bch2_btree_key_cache_count;
> + shrink->scan_objects = bch2_btree_key_cache_scan;
shrink->private_data = c;
> + shrinker_register(shrink);
> return 0;
> }
>
> diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h
> index 70398aaa095e..fac0abdaf167 100644
> --- a/fs/bcachefs/btree_types.h
> +++ b/fs/bcachefs/btree_types.h
> @@ -163,7 +163,7 @@ struct btree_cache {
> unsigned used;
> unsigned reserve;
> atomic_t dirty;
> - struct shrinker shrink;
> + struct shrinker *shrink;
>
> /*
> * If we need to allocate memory for a new btree node and that
> @@ -321,7 +321,7 @@ struct btree_key_cache {
> bool table_init_done;
> struct list_head freed_pcpu;
> struct list_head freed_nonpcpu;
> - struct shrinker shrink;
> + struct shrinker *shrink;
> unsigned shrink_iter;
> struct btree_key_cache_freelist __percpu *pcpu_freed;
>
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 48431700b83e..bdc8573631bd 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -1885,7 +1885,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
> sb->s_flags |= SB_POSIXACL;
> #endif
>
> - sb->s_shrink.seeks = 0;
> + sb->s_shrink->seeks = 0;
>
> vinode = bch2_vfs_inode_get(c, BCACHEFS_ROOT_SUBVOL_INUM);
> ret = PTR_ERR_OR_ZERO(vinode);
> diff --git a/fs/bcachefs/sysfs.c b/fs/bcachefs/sysfs.c
> index 41c6900c34c1..a9f480c26bb4 100644
> --- a/fs/bcachefs/sysfs.c
> +++ b/fs/bcachefs/sysfs.c
> @@ -522,7 +522,7 @@ STORE(bch2_fs)
>
> sc.gfp_mask = GFP_KERNEL;
> sc.nr_to_scan = strtoul_or_return(buf);
> - c->btree_cache.shrink.scan_objects(&c->btree_cache.shrink, &sc);
> + c->btree_cache.shrink->scan_objects(c->btree_cache.shrink, &sc);
> }
>
> if (attr == &sysfs_btree_wakeup)
Hi Qi,
Thanks for the corrections. See below.
On Tue, 12 Sep 2023 10:47:14 +0800 Qi Zheng <[email protected]> wrote:
>
> > diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
> > index 245ddd92b2d1..7f0eded6c296 100644
> > --- a/fs/bcachefs/btree_cache.c
> > +++ b/fs/bcachefs/btree_cache.c
> > @@ -285,7 +285,7 @@ static int btree_node_write_and_reclaim(struct bch_fs *c, struct btree *b)
> > static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> > struct shrink_control *sc)
> > {
> > - struct bch_fs *c = container_of(shrink, struct bch_fs,
> > + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> > btree_cache.shrink);
>
> The shrink passed in here will be a local variable, so its address can
> not be used directly. So need to be modified as follows:
>
> struct bch_fs *c = shrink->private_data;
OK.
> > @@ -384,7 +384,7 @@ static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> > static unsigned long bch2_btree_cache_count(struct shrinker *shrink,
> > struct shrink_control *sc)
> > {
> > - struct bch_fs *c = container_of(shrink, struct bch_fs,
> > + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> > btree_cache.shrink);
>
> Ditto.
OK
> > > @@ -473,12 +474,14 @@ int bch2_fs_btree_cache_init(struct bch_fs *c)
> > > mutex_init(&c->verify_lock);
> > > - bc->shrink.count_objects = bch2_btree_cache_count;
> > - bc->shrink.scan_objects = bch2_btree_cache_scan;
> > - bc->shrink.seeks = 4;
> > - ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
> > - if (ret)
> > + shrink = shrinker_alloc(0, "%s/btree_cache", c->name);
> > + if (!shrink)
> > goto err;
>
> Here the 'ret' needs to be set to -ENOMEM.
>
> if (!shrink) {
> ret = -ENOMEM;
> goto err;
> }
Except err: does this:
return -BCH_ERR_ENOMEM_fs_btree_cache_init;
so ret does not need to be set.
> > + bc->shrink = shrink;
> > + shrink->count_objects = bch2_btree_cache_count;
> > + shrink->scan_objects = bch2_btree_cache_scan;
> > + shrink->seeks = 4;
>
> shrink->private_data = c;
OK
> > diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
> > index 505e7c365ab7..88d33690233b 100644
> > --- a/fs/bcachefs/btree_key_cache.c
> > +++ b/fs/bcachefs/btree_key_cache.c
> > @@ -838,7 +838,7 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
> > static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> > struct shrink_control *sc)
> > {
> > - struct bch_fs *c = container_of(shrink, struct bch_fs,
> > + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> > btree_key_cache.shrink);
>
> struct bch_fs *c = shrink->private_data;
>
OK
> > @@ -936,7 +936,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> > static unsigned long bch2_btree_key_cache_count(struct shrinker *shrink,
> > struct shrink_control *sc)
> > {
> > - struct bch_fs *c = container_of(shrink, struct bch_fs,
> > + struct bch_fs *c = container_of(&shrink, struct bch_fs,
> > btree_key_cache.shrink);
>
> Ditto.
OK
> > @@ -957,7 +957,7 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
> > int cpu;
> > #endif
> > > - unregister_shrinker(&bc->shrink);
> > + shrinker_free(bc->shrink);
> > > mutex_lock(&bc->lock);
> > > @@ -1031,6 +1031,7 @@ void bch2_fs_btree_key_cache_init_early(struct btree_key_cache *c)
> > int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
> > {
> > struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
> > + struct shrinker *shrink;
> > > #ifdef __KERNEL__
> > bc->pcpu_freed = alloc_percpu(struct btree_key_cache_freelist);
> > @@ -1043,11 +1044,14 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
>
> struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
Already done n this function.
> > > bc->table_init_done = true;
> > > - bc->shrink.seeks = 0;
> > - bc->shrink.count_objects = bch2_btree_key_cache_count;
> > - bc->shrink.scan_objects = bch2_btree_key_cache_scan;
> > - if (register_shrinker(&bc->shrink, "%s/btree_key_cache", c->name))
> > + shrink = shrinker_alloc(0, "%s/btree_key_cache", c->name);
> > + if (!shrink)
> > return -BCH_ERR_ENOMEM_fs_btree_cache_init;
> > + bc->shrink = shrink;
> > + shrink->seeks = 0;
> > + shrink->count_objects = bch2_btree_key_cache_count;
> > + shrink->scan_objects = bch2_btree_key_cache_scan;
>
> shrink->private_data = c;
OK
So the merge resolution patch now looks like this:
From: Stephen Rothwell <[email protected]>
Date: Tue, 12 Sep 2023 11:27:22 +1000
Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
Signed-off-by: Stephen Rothwell <[email protected]>
---
fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
fs/bcachefs/btree_types.h | 4 ++--
fs/bcachefs/fs.c | 2 +-
fs/bcachefs/sysfs.c | 2 +-
5 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
index 245ddd92b2d1..d8cd0bbc33cc 100644
--- a/fs/bcachefs/btree_cache.c
+++ b/fs/bcachefs/btree_cache.c
@@ -285,8 +285,7 @@ static int btree_node_write_and_reclaim(struct bch_fs *c, struct btree *b)
static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct bch_fs *c = container_of(shrink, struct bch_fs,
- btree_cache.shrink);
+ struct bch_fs *c = shrink->private_data;
struct btree_cache *bc = &c->btree_cache;
struct btree *b, *t;
unsigned long nr = sc->nr_to_scan;
@@ -384,8 +383,7 @@ static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
static unsigned long bch2_btree_cache_count(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct bch_fs *c = container_of(shrink, struct bch_fs,
- btree_cache.shrink);
+ struct bch_fs *c = shrink->private_data;
struct btree_cache *bc = &c->btree_cache;
if (bch2_btree_shrinker_disabled)
@@ -400,7 +398,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
struct btree *b;
unsigned i, flags;
- unregister_shrinker(&bc->shrink);
+ shrinker_free(bc->shrink);
/* vfree() can allocate memory: */
flags = memalloc_nofs_save();
@@ -454,6 +452,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
int bch2_fs_btree_cache_init(struct bch_fs *c)
{
struct btree_cache *bc = &c->btree_cache;
+ struct shrinker *shrink;
unsigned i;
int ret = 0;
@@ -473,12 +472,15 @@ int bch2_fs_btree_cache_init(struct bch_fs *c)
mutex_init(&c->verify_lock);
- bc->shrink.count_objects = bch2_btree_cache_count;
- bc->shrink.scan_objects = bch2_btree_cache_scan;
- bc->shrink.seeks = 4;
- ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
- if (ret)
+ shrink = shrinker_alloc(0, "%s/btree_cache", c->name);
+ if (!shrink)
goto err;
+ bc->shrink = shrink;
+ shrink->count_objects = bch2_btree_cache_count;
+ shrink->scan_objects = bch2_btree_cache_scan;
+ shrink->seeks = 4;
+ shrink->private_data = c;
+ shrinker_register(shrink);
return 0;
err:
diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
index 505e7c365ab7..ed387eb915c3 100644
--- a/fs/bcachefs/btree_key_cache.c
+++ b/fs/bcachefs/btree_key_cache.c
@@ -838,8 +838,7 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct bch_fs *c = container_of(shrink, struct bch_fs,
- btree_key_cache.shrink);
+ struct bch_fs *c = shrink->private_data;
struct btree_key_cache *bc = &c->btree_key_cache;
struct bucket_table *tbl;
struct bkey_cached *ck, *t;
@@ -936,8 +935,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
static unsigned long bch2_btree_key_cache_count(struct shrinker *shrink,
struct shrink_control *sc)
{
- struct bch_fs *c = container_of(shrink, struct bch_fs,
- btree_key_cache.shrink);
+ struct bch_fs *c = shrink->private_data;
struct btree_key_cache *bc = &c->btree_key_cache;
long nr = atomic_long_read(&bc->nr_keys) -
atomic_long_read(&bc->nr_dirty);
@@ -957,7 +955,7 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
int cpu;
#endif
- unregister_shrinker(&bc->shrink);
+ shrinker_free(bc->shrink);
mutex_lock(&bc->lock);
@@ -1031,6 +1029,7 @@ void bch2_fs_btree_key_cache_init_early(struct btree_key_cache *c)
int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
{
struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
+ struct shrinker *shrink;
#ifdef __KERNEL__
bc->pcpu_freed = alloc_percpu(struct btree_key_cache_freelist);
@@ -1043,11 +1042,15 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
bc->table_init_done = true;
- bc->shrink.seeks = 0;
- bc->shrink.count_objects = bch2_btree_key_cache_count;
- bc->shrink.scan_objects = bch2_btree_key_cache_scan;
- if (register_shrinker(&bc->shrink, "%s/btree_key_cache", c->name))
+ shrink = shrinker_alloc(0, "%s/btree_key_cache", c->name);
+ if (!shrink)
return -BCH_ERR_ENOMEM_fs_btree_cache_init;
+ bc->shrink = shrink;
+ shrink->seeks = 0;
+ shrink->count_objects = bch2_btree_key_cache_count;
+ shrink->scan_objects = bch2_btree_key_cache_scan;
+ shrink->private_data = c;
+ shrinker_register(shrink);
return 0;
}
diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h
index 70398aaa095e..fac0abdaf167 100644
--- a/fs/bcachefs/btree_types.h
+++ b/fs/bcachefs/btree_types.h
@@ -163,7 +163,7 @@ struct btree_cache {
unsigned used;
unsigned reserve;
atomic_t dirty;
- struct shrinker shrink;
+ struct shrinker *shrink;
/*
* If we need to allocate memory for a new btree node and that
@@ -321,7 +321,7 @@ struct btree_key_cache {
bool table_init_done;
struct list_head freed_pcpu;
struct list_head freed_nonpcpu;
- struct shrinker shrink;
+ struct shrinker *shrink;
unsigned shrink_iter;
struct btree_key_cache_freelist __percpu *pcpu_freed;
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 48431700b83e..bdc8573631bd 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1885,7 +1885,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
sb->s_flags |= SB_POSIXACL;
#endif
- sb->s_shrink.seeks = 0;
+ sb->s_shrink->seeks = 0;
vinode = bch2_vfs_inode_get(c, BCACHEFS_ROOT_SUBVOL_INUM);
ret = PTR_ERR_OR_ZERO(vinode);
diff --git a/fs/bcachefs/sysfs.c b/fs/bcachefs/sysfs.c
index 41c6900c34c1..a9f480c26bb4 100644
--- a/fs/bcachefs/sysfs.c
+++ b/fs/bcachefs/sysfs.c
@@ -522,7 +522,7 @@ STORE(bch2_fs)
sc.gfp_mask = GFP_KERNEL;
sc.nr_to_scan = strtoul_or_return(buf);
- c->btree_cache.shrink.scan_objects(&c->btree_cache.shrink, &sc);
+ c->btree_cache.shrink->scan_objects(c->btree_cache.shrink, &sc);
}
if (attr == &sysfs_btree_wakeup)
--
2.40.1
--
Cheers,
Stephen Rothwell
Hi Stephen,
On 2023/9/13 07:35, Stephen Rothwell wrote:
> Hi Qi,
>
> Thanks for the corrections. See below.
>
> On Tue, 12 Sep 2023 10:47:14 +0800 Qi Zheng <[email protected]> wrote:
>>
>>> diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
>>> index 245ddd92b2d1..7f0eded6c296 100644
>>> --- a/fs/bcachefs/btree_cache.c
>>> +++ b/fs/bcachefs/btree_cache.c
>>> @@ -285,7 +285,7 @@ static int btree_node_write_and_reclaim(struct bch_fs *c, struct btree *b)
>>> static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
>>> struct shrink_control *sc)
>>> {
>>> - struct bch_fs *c = container_of(shrink, struct bch_fs,
>>> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
>>> btree_cache.shrink);
>>
>> The shrink passed in here will be a local variable, so its address can
>> not be used directly. So need to be modified as follows:
>>
>> struct bch_fs *c = shrink->private_data;
>
> OK.
>
>>> @@ -384,7 +384,7 @@ static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
>>> static unsigned long bch2_btree_cache_count(struct shrinker *shrink,
>>> struct shrink_control *sc)
>>> {
>>> - struct bch_fs *c = container_of(shrink, struct bch_fs,
>>> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
>>> btree_cache.shrink);
>>
>> Ditto.
>
> OK
>
>>> > @@ -473,12 +474,14 @@ int bch2_fs_btree_cache_init(struct bch_fs *c)
>>> > mutex_init(&c->verify_lock);
>>> > - bc->shrink.count_objects = bch2_btree_cache_count;
>>> - bc->shrink.scan_objects = bch2_btree_cache_scan;
>>> - bc->shrink.seeks = 4;
>>> - ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
>>> - if (ret)
>>> + shrink = shrinker_alloc(0, "%s/btree_cache", c->name);
>>> + if (!shrink)
>>> goto err;
>>
>> Here the 'ret' needs to be set to -ENOMEM.
>>
>> if (!shrink) {
>> ret = -ENOMEM;
>> goto err;
>> }
>
> Except err: does this:
>
> return -BCH_ERR_ENOMEM_fs_btree_cache_init;
>
> so ret does not need to be set.
>
>>> + bc->shrink = shrink;
>>> + shrink->count_objects = bch2_btree_cache_count;
>>> + shrink->scan_objects = bch2_btree_cache_scan;
>>> + shrink->seeks = 4;
>>
>> shrink->private_data = c;
>
> OK
>
>>> diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
>>> index 505e7c365ab7..88d33690233b 100644
>>> --- a/fs/bcachefs/btree_key_cache.c
>>> +++ b/fs/bcachefs/btree_key_cache.c
>>> @@ -838,7 +838,7 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
>>> static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
>>> struct shrink_control *sc)
>>> {
>>> - struct bch_fs *c = container_of(shrink, struct bch_fs,
>>> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
>>> btree_key_cache.shrink);
>>
>> struct bch_fs *c = shrink->private_data;
>>
>
> OK
>
>>> @@ -936,7 +936,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
>>> static unsigned long bch2_btree_key_cache_count(struct shrinker *shrink,
>>> struct shrink_control *sc)
>>> {
>>> - struct bch_fs *c = container_of(shrink, struct bch_fs,
>>> + struct bch_fs *c = container_of(&shrink, struct bch_fs,
>>> btree_key_cache.shrink);
>>
>> Ditto.
>
> OK
>
>>> @@ -957,7 +957,7 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
>>> int cpu;
>>> #endif
>>> > - unregister_shrinker(&bc->shrink);
>>> + shrinker_free(bc->shrink);
>>> > mutex_lock(&bc->lock);
>>> > @@ -1031,6 +1031,7 @@ void bch2_fs_btree_key_cache_init_early(struct btree_key_cache *c)
>>> int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
>>> {
>>> struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
>>> + struct shrinker *shrink;
>>> > #ifdef __KERNEL__
>>> bc->pcpu_freed = alloc_percpu(struct btree_key_cache_freelist);
>>> @@ -1043,11 +1044,14 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
>>
>> struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
>
> Already done n this function.
>
>>> > bc->table_init_done = true;
>>> > - bc->shrink.seeks = 0;
>>> - bc->shrink.count_objects = bch2_btree_key_cache_count;
>>> - bc->shrink.scan_objects = bch2_btree_key_cache_scan;
>>> - if (register_shrinker(&bc->shrink, "%s/btree_key_cache", c->name))
>>> + shrink = shrinker_alloc(0, "%s/btree_key_cache", c->name);
>>> + if (!shrink)
>>> return -BCH_ERR_ENOMEM_fs_btree_cache_init;
>>> + bc->shrink = shrink;
>>> + shrink->seeks = 0;
>>> + shrink->count_objects = bch2_btree_key_cache_count;
>>> + shrink->scan_objects = bch2_btree_key_cache_scan;
>>
>> shrink->private_data = c;
>
> OK
>
> So the merge resolution patch now looks like this:
>
> From: Stephen Rothwell <[email protected]>
> Date: Tue, 12 Sep 2023 11:27:22 +1000
> Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
> fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
> fs/bcachefs/btree_types.h | 4 ++--
> fs/bcachefs/fs.c | 2 +-
> fs/bcachefs/sysfs.c | 2 +-
> 5 files changed, 28 insertions(+), 23 deletions(-)
This version looks good to me.
Reviewed-by: Qi Zheng <[email protected]>
Thanks,
Qi
>
> diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c
> index 245ddd92b2d1..d8cd0bbc33cc 100644
> --- a/fs/bcachefs/btree_cache.c
> +++ b/fs/bcachefs/btree_cache.c
> @@ -285,8 +285,7 @@ static int btree_node_write_and_reclaim(struct bch_fs *c, struct btree *b)
> static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> - btree_cache.shrink);
> + struct bch_fs *c = shrink->private_data;
> struct btree_cache *bc = &c->btree_cache;
> struct btree *b, *t;
> unsigned long nr = sc->nr_to_scan;
> @@ -384,8 +383,7 @@ static unsigned long bch2_btree_cache_scan(struct shrinker *shrink,
> static unsigned long bch2_btree_cache_count(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> - btree_cache.shrink);
> + struct bch_fs *c = shrink->private_data;
> struct btree_cache *bc = &c->btree_cache;
>
> if (bch2_btree_shrinker_disabled)
> @@ -400,7 +398,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
> struct btree *b;
> unsigned i, flags;
>
> - unregister_shrinker(&bc->shrink);
> + shrinker_free(bc->shrink);
>
> /* vfree() can allocate memory: */
> flags = memalloc_nofs_save();
> @@ -454,6 +452,7 @@ void bch2_fs_btree_cache_exit(struct bch_fs *c)
> int bch2_fs_btree_cache_init(struct bch_fs *c)
> {
> struct btree_cache *bc = &c->btree_cache;
> + struct shrinker *shrink;
> unsigned i;
> int ret = 0;
>
> @@ -473,12 +472,15 @@ int bch2_fs_btree_cache_init(struct bch_fs *c)
>
> mutex_init(&c->verify_lock);
>
> - bc->shrink.count_objects = bch2_btree_cache_count;
> - bc->shrink.scan_objects = bch2_btree_cache_scan;
> - bc->shrink.seeks = 4;
> - ret = register_shrinker(&bc->shrink, "%s/btree_cache", c->name);
> - if (ret)
> + shrink = shrinker_alloc(0, "%s/btree_cache", c->name);
> + if (!shrink)
> goto err;
> + bc->shrink = shrink;
> + shrink->count_objects = bch2_btree_cache_count;
> + shrink->scan_objects = bch2_btree_cache_scan;
> + shrink->seeks = 4;
> + shrink->private_data = c;
> + shrinker_register(shrink);
>
> return 0;
> err:
> diff --git a/fs/bcachefs/btree_key_cache.c b/fs/bcachefs/btree_key_cache.c
> index 505e7c365ab7..ed387eb915c3 100644
> --- a/fs/bcachefs/btree_key_cache.c
> +++ b/fs/bcachefs/btree_key_cache.c
> @@ -838,8 +838,7 @@ void bch2_btree_key_cache_drop(struct btree_trans *trans,
> static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> - btree_key_cache.shrink);
> + struct bch_fs *c = shrink->private_data;
> struct btree_key_cache *bc = &c->btree_key_cache;
> struct bucket_table *tbl;
> struct bkey_cached *ck, *t;
> @@ -936,8 +935,7 @@ static unsigned long bch2_btree_key_cache_scan(struct shrinker *shrink,
> static unsigned long bch2_btree_key_cache_count(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct bch_fs *c = container_of(shrink, struct bch_fs,
> - btree_key_cache.shrink);
> + struct bch_fs *c = shrink->private_data;
> struct btree_key_cache *bc = &c->btree_key_cache;
> long nr = atomic_long_read(&bc->nr_keys) -
> atomic_long_read(&bc->nr_dirty);
> @@ -957,7 +955,7 @@ void bch2_fs_btree_key_cache_exit(struct btree_key_cache *bc)
> int cpu;
> #endif
>
> - unregister_shrinker(&bc->shrink);
> + shrinker_free(bc->shrink);
>
> mutex_lock(&bc->lock);
>
> @@ -1031,6 +1029,7 @@ void bch2_fs_btree_key_cache_init_early(struct btree_key_cache *c)
> int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
> {
> struct bch_fs *c = container_of(bc, struct bch_fs, btree_key_cache);
> + struct shrinker *shrink;
>
> #ifdef __KERNEL__
> bc->pcpu_freed = alloc_percpu(struct btree_key_cache_freelist);
> @@ -1043,11 +1042,15 @@ int bch2_fs_btree_key_cache_init(struct btree_key_cache *bc)
>
> bc->table_init_done = true;
>
> - bc->shrink.seeks = 0;
> - bc->shrink.count_objects = bch2_btree_key_cache_count;
> - bc->shrink.scan_objects = bch2_btree_key_cache_scan;
> - if (register_shrinker(&bc->shrink, "%s/btree_key_cache", c->name))
> + shrink = shrinker_alloc(0, "%s/btree_key_cache", c->name);
> + if (!shrink)
> return -BCH_ERR_ENOMEM_fs_btree_cache_init;
> + bc->shrink = shrink;
> + shrink->seeks = 0;
> + shrink->count_objects = bch2_btree_key_cache_count;
> + shrink->scan_objects = bch2_btree_key_cache_scan;
> + shrink->private_data = c;
> + shrinker_register(shrink);
> return 0;
> }
>
> diff --git a/fs/bcachefs/btree_types.h b/fs/bcachefs/btree_types.h
> index 70398aaa095e..fac0abdaf167 100644
> --- a/fs/bcachefs/btree_types.h
> +++ b/fs/bcachefs/btree_types.h
> @@ -163,7 +163,7 @@ struct btree_cache {
> unsigned used;
> unsigned reserve;
> atomic_t dirty;
> - struct shrinker shrink;
> + struct shrinker *shrink;
>
> /*
> * If we need to allocate memory for a new btree node and that
> @@ -321,7 +321,7 @@ struct btree_key_cache {
> bool table_init_done;
> struct list_head freed_pcpu;
> struct list_head freed_nonpcpu;
> - struct shrinker shrink;
> + struct shrinker *shrink;
> unsigned shrink_iter;
> struct btree_key_cache_freelist __percpu *pcpu_freed;
>
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 48431700b83e..bdc8573631bd 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -1885,7 +1885,7 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
> sb->s_flags |= SB_POSIXACL;
> #endif
>
> - sb->s_shrink.seeks = 0;
> + sb->s_shrink->seeks = 0;
>
> vinode = bch2_vfs_inode_get(c, BCACHEFS_ROOT_SUBVOL_INUM);
> ret = PTR_ERR_OR_ZERO(vinode);
> diff --git a/fs/bcachefs/sysfs.c b/fs/bcachefs/sysfs.c
> index 41c6900c34c1..a9f480c26bb4 100644
> --- a/fs/bcachefs/sysfs.c
> +++ b/fs/bcachefs/sysfs.c
> @@ -522,7 +522,7 @@ STORE(bch2_fs)
>
> sc.gfp_mask = GFP_KERNEL;
> sc.nr_to_scan = strtoul_or_return(buf);
> - c->btree_cache.shrink.scan_objects(&c->btree_cache.shrink, &sc);
> + c->btree_cache.shrink->scan_objects(c->btree_cache.shrink, &sc);
> }
>
> if (attr == &sysfs_btree_wakeup)
On Wed, 13 Sep 2023 09:10:11 +0800 Qi Zheng <[email protected]> wrote:
> > From: Stephen Rothwell <[email protected]>
> > Date: Tue, 12 Sep 2023 11:27:22 +1000
> > Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
> >
> > Signed-off-by: Stephen Rothwell <[email protected]>
> > ---
> > fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
> > fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
> > fs/bcachefs/btree_types.h | 4 ++--
> > fs/bcachefs/fs.c | 2 +-
> > fs/bcachefs/sysfs.c | 2 +-
> > 5 files changed, 28 insertions(+), 23 deletions(-)
>
> This version looks good to me.
>
> Reviewed-by: Qi Zheng <[email protected]>
I not longer carry a post-linux-next patch queue, so there's nothing I
can do with this patch. I'll assume that either Kent or I will merge
it later, depending upon whose stuff goes into mainline first.
Hi Andrew,
On Wed, 13 Sep 2023 13:23:30 -0700 Andrew Morton <[email protected]> wrote:
>
> On Wed, 13 Sep 2023 09:10:11 +0800 Qi Zheng <[email protected]> wrote:
>
> > > From: Stephen Rothwell <[email protected]>
> > > Date: Tue, 12 Sep 2023 11:27:22 +1000
> > > Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
> > >
> > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > ---
> > > fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
> > > fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
> > > fs/bcachefs/btree_types.h | 4 ++--
> > > fs/bcachefs/fs.c | 2 +-
> > > fs/bcachefs/sysfs.c | 2 +-
> > > 5 files changed, 28 insertions(+), 23 deletions(-)
> >
> > This version looks good to me.
> >
> > Reviewed-by: Qi Zheng <[email protected]>
>
> I not longer carry a post-linux-next patch queue, so there's nothing I
> can do with this patch. I'll assume that either Kent or I will merge
> it later, depending upon whose stuff goes into mainline first.
Actually the correct plan is for you and Kent to inform Linus of the
need for this patch as part of the merge resolution when he merges the
latter of your trees (unless you want to stabilise the shrinker changes
into a separate branch that is never rewritten and is merged into your
tree and the bcachefs tree).
--
Cheers,
Stephen Rothwell
Hi Andrew,
On Thu, 14 Sep 2023 08:31:45 +1000 Stephen Rothwell <[email protected]> wrote:
>
> On Wed, 13 Sep 2023 13:23:30 -0700 Andrew Morton <[email protected]> wrote:
> >
> > On Wed, 13 Sep 2023 09:10:11 +0800 Qi Zheng <[email protected]> wrote:
> >
> > > > From: Stephen Rothwell <[email protected]>
> > > > Date: Tue, 12 Sep 2023 11:27:22 +1000
> > > > Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
> > > >
> > > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > > ---
> > > > fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
> > > > fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
> > > > fs/bcachefs/btree_types.h | 4 ++--
> > > > fs/bcachefs/fs.c | 2 +-
> > > > fs/bcachefs/sysfs.c | 2 +-
> > > > 5 files changed, 28 insertions(+), 23 deletions(-)
> > >
> > > This version looks good to me.
> > >
> > > Reviewed-by: Qi Zheng <[email protected]>
> >
> > I not longer carry a post-linux-next patch queue, so there's nothing I
> > can do with this patch. I'll assume that either Kent or I will merge
> > it later, depending upon whose stuff goes into mainline first.
>
> Actually the correct plan is for you and Kent to inform Linus of the
> need for this patch as part of the merge resolution when he merges the
> latter of your trees (unless you want to stabilise the shrinker changes
> into a separate branch that is never rewritten and is merged into your
> tree and the bcachefs tree).
This is now a conflict between the mm-stable tree and Linus' tree.
--
Cheers,
Stephen Rothwell
On Wed, Nov 01, 2023 at 11:32:22AM +1100, Stephen Rothwell wrote:
> Hi Andrew,
>
> On Thu, 14 Sep 2023 08:31:45 +1000 Stephen Rothwell <[email protected]> wrote:
> >
> > On Wed, 13 Sep 2023 13:23:30 -0700 Andrew Morton <[email protected]> wrote:
> > >
> > > On Wed, 13 Sep 2023 09:10:11 +0800 Qi Zheng <[email protected]> wrote:
> > >
> > > > > From: Stephen Rothwell <[email protected]>
> > > > > Date: Tue, 12 Sep 2023 11:27:22 +1000
> > > > > Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers
> > > > >
> > > > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > > > ---
> > > > > fs/bcachefs/btree_cache.c | 22 ++++++++++++----------
> > > > > fs/bcachefs/btree_key_cache.c | 21 ++++++++++++---------
> > > > > fs/bcachefs/btree_types.h | 4 ++--
> > > > > fs/bcachefs/fs.c | 2 +-
> > > > > fs/bcachefs/sysfs.c | 2 +-
> > > > > 5 files changed, 28 insertions(+), 23 deletions(-)
> > > >
> > > > This version looks good to me.
> > > >
> > > > Reviewed-by: Qi Zheng <[email protected]>
> > >
> > > I not longer carry a post-linux-next patch queue, so there's nothing I
> > > can do with this patch. I'll assume that either Kent or I will merge
> > > it later, depending upon whose stuff goes into mainline first.
> >
> > Actually the correct plan is for you and Kent to inform Linus of the
> > need for this patch as part of the merge resolution when he merges the
> > latter of your trees (unless you want to stabilise the shrinker changes
> > into a separate branch that is never rewritten and is merged into your
> > tree and the bcachefs tree).
>
> This is now a conflict between the mm-stable tree and Linus' tree.
Is/was there a procedure for me here?
Hi Kent,
On Tue, 31 Oct 2023 20:53:07 -0400 Kent Overstreet <[email protected]> wrote:
>
> Is/was there a procedure for me here?
You should have mentioned it in your pull request to Linus (in case he
merged Andrew's tree first (I don't know if you did). And presumably
Andrew will mention it in his pull request to Linus.
--
Cheers,
Stephen Rothwell