All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Rothwell <sfr@canb.auug.org.au>
To: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: Kent Overstreet <kent.overstreet@linux.dev>,
	Kent Overstreet <kent.overstreet@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>
Subject: Re: linux-next: build failure after merge of the bcachefs tree
Date: Wed, 13 Sep 2023 09:35:53 +1000	[thread overview]
Message-ID: <20230913093553.4290421e@canb.auug.org.au> (raw)
In-Reply-To: <e639a428-0fb7-7329-ce52-e51f7951a146@bytedance.com>

[-- Attachment #1: Type: text/plain, Size: 11429 bytes --]

Hi Qi,

Thanks for the corrections.  See below.

On Tue, 12 Sep 2023 10:47:14 +0800 Qi Zheng <zhengqi.arch@bytedance.com> 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 <sfr@canb.auug.org.au>
Date: Tue, 12 Sep 2023 11:27:22 +1000
Subject: [PATCH] bcachefs: convert to dynamically allocated shrinkers

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2023-09-12 23:35 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-12  2:04 linux-next: build failure after merge of the bcachefs tree Stephen Rothwell
2023-09-12  2:47 ` Qi Zheng
2023-09-12 23:35   ` Stephen Rothwell [this message]
2023-09-13  1:10     ` Qi Zheng
2023-09-13 20:23       ` Andrew Morton
2023-09-13 22:31         ` Stephen Rothwell
2023-11-01  0:32           ` Stephen Rothwell
2023-11-01  0:53             ` Kent Overstreet
2023-11-01  1:14               ` Stephen Rothwell
2023-09-13 22:14 ` Stephen Rothwell
2023-11-12 22:30 Stephen Rothwell
2023-11-15 23:02 Stephen Rothwell
2024-02-07  0:57 Stephen Rothwell
2024-02-08 22:46 ` Stephen Rothwell
2024-02-08 23:59   ` Kent Overstreet
2024-02-11 23:54 Stephen Rothwell
2024-02-14  0:16 ` Kees Cook
2024-02-14  0:29   ` Kent Overstreet
2024-02-14  0:39     ` Darrick J. Wong
2024-02-14  1:09     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230913093553.4290421e@canb.auug.org.au \
    --to=sfr@canb.auug.org.au \
    --cc=akpm@linux-foundation.org \
    --cc=kent.overstreet@gmail.com \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=zhengqi.arch@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.