[PATCH 0/4] Parameter cleanup

[PATCH 0/4] Parameter cleanup

[David Sterba]

A few more cases where value passed by parameter can be used directly.

David Sterba (4):
  btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
  btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
  btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
  btrfs: sink gfp_t parameter to alloc_scrub_sector

 fs/btrfs/backref.c    |  5 ++---
 fs/btrfs/backref.h    |  3 +--
 fs/btrfs/qgroup.c     | 17 +++++++----------
 fs/btrfs/qgroup.h     |  2 +-
 fs/btrfs/relocation.c |  2 +-
 fs/btrfs/scrub.c      | 14 +++++++-------
 fs/btrfs/tree-log.c   |  3 +--
 7 files changed, 20 insertions(+), 26 deletions(-)

-- 
2.37.3

[PATCH 1/4] btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc

[David Sterba]

There's only one caller that passes GFP_NOFS, we can drop the parameter
an use the flags directly.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/backref.c | 5 ++---
 fs/btrfs/backref.h | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index e6b69ac1a77c..a5e548f30242 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -2634,12 +2634,11 @@ void free_ipath(struct inode_fs_paths *ipath)
 	kfree(ipath);
 }
 
-struct btrfs_backref_iter *btrfs_backref_iter_alloc(
-		struct btrfs_fs_info *fs_info, gfp_t gfp_flag)
+struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_backref_iter *ret;
 
-	ret = kzalloc(sizeof(*ret), gfp_flag);
+	ret = kzalloc(sizeof(*ret), GFP_NOFS);
 	if (!ret)
 		return NULL;
 
diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
index 6dac462430b0..ea8b59a201e6 100644
--- a/fs/btrfs/backref.h
+++ b/fs/btrfs/backref.h
@@ -155,8 +155,7 @@ struct btrfs_backref_iter {
 	u32 end_ptr;
 };
 
-struct btrfs_backref_iter *btrfs_backref_iter_alloc(
-		struct btrfs_fs_info *fs_info, gfp_t gfp_flag);
+struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
 
 static inline void btrfs_backref_iter_free(struct btrfs_backref_iter *iter)
 {
-- 
2.37.3

[PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent

[David Sterba]

All callers pass GFP_NOFS, we can drop the parameter and use it
directly.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/qgroup.c     | 17 +++++++----------
 fs/btrfs/qgroup.h     |  2 +-
 fs/btrfs/relocation.c |  2 +-
 fs/btrfs/tree-log.c   |  3 +--
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 9334c3157c22..34f0e4dabe25 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1840,7 +1840,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
 }
 
 int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
-			      u64 num_bytes, gfp_t gfp_flag)
+			      u64 num_bytes)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_qgroup_extent_record *record;
@@ -1850,7 +1850,7 @@ int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
 	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)
 	    || bytenr == 0 || num_bytes == 0)
 		return 0;
-	record = kzalloc(sizeof(*record), gfp_flag);
+	record = kzalloc(sizeof(*record), GFP_NOFS);
 	if (!record)
 		return -ENOMEM;
 
@@ -1902,8 +1902,7 @@ int btrfs_qgroup_trace_leaf_items(struct btrfs_trans_handle *trans,
 
 		num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
 
-		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes,
-						GFP_NOFS);
+		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes);
 		if (ret)
 			return ret;
 	}
@@ -2102,12 +2101,11 @@ static int qgroup_trace_extent_swap(struct btrfs_trans_handle* trans,
 	 * blocks for qgroup accounting.
 	 */
 	ret = btrfs_qgroup_trace_extent(trans, src_path->nodes[dst_level]->start,
-			nodesize, GFP_NOFS);
+					nodesize);
 	if (ret < 0)
 		goto out;
-	ret = btrfs_qgroup_trace_extent(trans,
-			dst_path->nodes[dst_level]->start,
-			nodesize, GFP_NOFS);
+	ret = btrfs_qgroup_trace_extent(trans, dst_path->nodes[dst_level]->start,
+					nodesize);
 	if (ret < 0)
 		goto out;
 
@@ -2391,8 +2389,7 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans,
 			path->locks[level] = BTRFS_READ_LOCK;
 
 			ret = btrfs_qgroup_trace_extent(trans, child_bytenr,
-							fs_info->nodesize,
-							GFP_NOFS);
+							fs_info->nodesize);
 			if (ret)
 				goto out;
 		}
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index 3fb5459c9309..7bffa10589d6 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -321,7 +321,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
  * (NULL trans)
  */
 int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
-			      u64 num_bytes, gfp_t gfp_flag);
+			      u64 num_bytes);
 
 /*
  * Inform qgroup to trace all leaf items of data
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 216a4485d914..f5564aa313f5 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -471,7 +471,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
 	int ret;
 	int err = 0;
 
-	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
+	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
 	if (!iter)
 		return ERR_PTR(-ENOMEM);
 	path = btrfs_alloc_path();
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 813986e38258..3b44b325aba6 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -747,8 +747,7 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
 		 */
 		ret = btrfs_qgroup_trace_extent(trans,
 				btrfs_file_extent_disk_bytenr(eb, item),
-				btrfs_file_extent_disk_num_bytes(eb, item),
-				GFP_NOFS);
+				btrfs_file_extent_disk_num_bytes(eb, item));
 		if (ret < 0)
 			goto out;
 
-- 
2.37.3

[PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block

[David Sterba]

There's only one caller that calls scrub_setup_recheck_block in the
memalloc_nofs_save/_restore protection so it's effectively already
GFP_NOFS and it's safe to use GFP_KERNEL.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/scrub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 9e3b2e60e571..2fc70a2cc7fe 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1491,7 +1491,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
 			return -EIO;
 		}
 
-		recover = kzalloc(sizeof(struct scrub_recover), GFP_NOFS);
+		recover = kzalloc(sizeof(struct scrub_recover), GFP_KERNEL);
 		if (!recover) {
 			btrfs_put_bioc(bioc);
 			btrfs_bio_counter_dec(fs_info);
@@ -1514,7 +1514,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
 			sblock = sblocks_for_recheck[mirror_index];
 			sblock->sctx = sctx;
 
-			sector = alloc_scrub_sector(sblock, logical, GFP_NOFS);
+			sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
 			if (!sector) {
 				spin_lock(&sctx->stat_lock);
 				sctx->stat.malloc_errors++;
-- 
2.37.3

[PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector

[David Sterba]

All callers pas GFP_KERNEL as parameter so we can use it directly in
alloc_scrub_sector.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/scrub.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 2fc70a2cc7fe..79d0ef534d5f 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -295,7 +295,7 @@ static struct scrub_block *alloc_scrub_block(struct scrub_ctx *sctx,
  * Will also allocate new pages for @sblock if needed.
  */
 static struct scrub_sector *alloc_scrub_sector(struct scrub_block *sblock,
-					       u64 logical, gfp_t gfp)
+					       u64 logical)
 {
 	const pgoff_t page_index = (logical - sblock->logical) >> PAGE_SHIFT;
 	struct scrub_sector *ssector;
@@ -303,7 +303,7 @@ static struct scrub_sector *alloc_scrub_sector(struct scrub_block *sblock,
 	/* We must never have scrub_block exceed U32_MAX in size. */
 	ASSERT(logical - sblock->logical < U32_MAX);
 
-	ssector = kzalloc(sizeof(*ssector), gfp);
+	ssector = kzalloc(sizeof(*ssector), GFP_KERNEL);
 	if (!ssector)
 		return NULL;
 
@@ -311,7 +311,7 @@ static struct scrub_sector *alloc_scrub_sector(struct scrub_block *sblock,
 	if (!sblock->pages[page_index]) {
 		int ret;
 
-		sblock->pages[page_index] = alloc_page(gfp);
+		sblock->pages[page_index] = alloc_page(GFP_KERNEL);
 		if (!sblock->pages[page_index]) {
 			kfree(ssector);
 			return NULL;
@@ -1514,7 +1514,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
 			sblock = sblocks_for_recheck[mirror_index];
 			sblock->sctx = sctx;
 
-			sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
+			sector = alloc_scrub_sector(sblock, logical);
 			if (!sector) {
 				spin_lock(&sctx->stat_lock);
 				sctx->stat.malloc_errors++;
@@ -2436,7 +2436,7 @@ static int scrub_sectors(struct scrub_ctx *sctx, u64 logical, u32 len,
 		 */
 		u32 l = min(sectorsize, len);
 
-		sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
+		sector = alloc_scrub_sector(sblock, logical);
 		if (!sector) {
 			spin_lock(&sctx->stat_lock);
 			sctx->stat.malloc_errors++;
@@ -2773,7 +2773,7 @@ static int scrub_sectors_for_parity(struct scrub_parity *sparity,
 	for (index = 0; len > 0; index++) {
 		struct scrub_sector *sector;
 
-		sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
+		sector = alloc_scrub_sector(sblock, logical);
 		if (!sector) {
 			spin_lock(&sctx->stat_lock);
 			sctx->stat.malloc_errors++;
-- 
2.37.3

Re: [PATCH 0/4] Parameter cleanup

[Johannes Thumshirn]

On 18.10.22 16:27, David Sterba wrote:
> A few more cases where value passed by parameter can be used directly.
> 
> David Sterba (4):
>   btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
>   btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
>   btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
>   btrfs: sink gfp_t parameter to alloc_scrub_sector
> 
>  fs/btrfs/backref.c    |  5 ++---
>  fs/btrfs/backref.h    |  3 +--
>  fs/btrfs/qgroup.c     | 17 +++++++----------
>  fs/btrfs/qgroup.h     |  2 +-
>  fs/btrfs/relocation.c |  2 +-
>  fs/btrfs/scrub.c      | 14 +++++++-------
>  fs/btrfs/tree-log.c   |  3 +--
>  7 files changed, 20 insertions(+), 26 deletions(-)
> 

What base is this on?

I got the following when applying it for review:

[johannes@redsun91:linux (review)]$ b4 am -o - cover.1666103172.git.dsterba@suse.com | git am
Looking up https://lore.kernel.org/r/cover.1666103172.git.dsterba%40suse.com
Grabbing thread from lore.kernel.org/all/cover.1666103172.git.dsterba%40suse.com/t.mbox.gz
Analyzing 5 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH 1/4] btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
  ✓ [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
  ✓ [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
  ✓ [PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector
  ---
  ✓ Signed: DKIM/suse.com
---
Total patches: 4
---
 Link: https://lore.kernel.org/r/cover.1666103172.git.dsterba@suse.com
 Base: not specified
Applying: btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc

Error in reading or end of file.

Error in reading or end of file.

Error in reading or end of file.

Error in reading or end of file.

Error in reading or end of file.

Error in reading or end of file.
fs/btrfs/relocation.c: In function ‘build_backref_tree’:
fs/btrfs/relocation.c:474:16: error: too many arguments to function ‘btrfs_backref_iter_alloc’
  474 |         iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/btrfs/relocation.c:25:
fs/btrfs/backref.h:158:28: note: declared here
  158 | struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
      |                            ^~~~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:250: fs/btrfs/relocation.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:500: fs/btrfs] Error 2
make[1]: *** [scripts/Makefile.build:500: fs] Error 2

Re: [PATCH 0/4] Parameter cleanup

[David Sterba]

On Wed, Oct 19, 2022 at 10:28:53AM +0000, Johannes Thumshirn wrote:
> On 18.10.22 16:27, David Sterba wrote:
> > A few more cases where value passed by parameter can be used directly.
> > 
> > David Sterba (4):
> >   btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
> >   btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
> >   btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
> >   btrfs: sink gfp_t parameter to alloc_scrub_sector
> > 
> >  fs/btrfs/backref.c    |  5 ++---
> >  fs/btrfs/backref.h    |  3 +--
> >  fs/btrfs/qgroup.c     | 17 +++++++----------
> >  fs/btrfs/qgroup.h     |  2 +-
> >  fs/btrfs/relocation.c |  2 +-
> >  fs/btrfs/scrub.c      | 14 +++++++-------
> >  fs/btrfs/tree-log.c   |  3 +--
> >  7 files changed, 20 insertions(+), 26 deletions(-)
> > 
> 
> What base is this on?
> 
> I got the following when applying it for review:
> 
> [johannes@redsun91:linux (review)]$ b4 am -o - cover.1666103172.git.dsterba@suse.com | git am
> Looking up https://lore.kernel.org/r/cover.1666103172.git.dsterba%40suse.com
> Grabbing thread from lore.kernel.org/all/cover.1666103172.git.dsterba%40suse.com/t.mbox.gz
> Analyzing 5 messages in the thread
> Checking attestation on all messages, may take a moment...
> ---
>   ✓ [PATCH 1/4] btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
>   ✓ [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
>   ✓ [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
>   ✓ [PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector
>   ---
>   ✓ Signed: DKIM/suse.com
> ---
> Total patches: 4
> ---
>  Link: https://lore.kernel.org/r/cover.1666103172.git.dsterba@suse.com
>  Base: not specified
> Applying: btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
> 
> Error in reading or end of file.
> fs/btrfs/relocation.c: In function ‘build_backref_tree’:
> fs/btrfs/relocation.c:474:16: error: too many arguments to function ‘btrfs_backref_iter_alloc’
>   474 |         iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
>       |                ^~~~~~~~~~~~~~~~~~~~~~~~
> In file included from fs/btrfs/relocation.c:25:
> fs/btrfs/backref.h:158:28: note: declared here
>   158 | struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
>       |                            ^~~~~~~~~~~~~~~~~~~~~~~~

I have it in a branch on top of some misc-next snapshot, the date is
from 3 days ago and rebase to current misc-next is clean and builds.

Re: [PATCH 0/4] Parameter cleanup

[Johannes Thumshirn]

On 19.10.22 17:16, David Sterba wrote:
> On Wed, Oct 19, 2022 at 10:28:53AM +0000, Johannes Thumshirn wrote:
>> On 18.10.22 16:27, David Sterba wrote:
>>> A few more cases where value passed by parameter can be used directly.
>>>
>>> David Sterba (4):
>>>   btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
>>>   btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
>>>   btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
>>>   btrfs: sink gfp_t parameter to alloc_scrub_sector
>>>
>>>  fs/btrfs/backref.c    |  5 ++---
>>>  fs/btrfs/backref.h    |  3 +--
>>>  fs/btrfs/qgroup.c     | 17 +++++++----------
>>>  fs/btrfs/qgroup.h     |  2 +-
>>>  fs/btrfs/relocation.c |  2 +-
>>>  fs/btrfs/scrub.c      | 14 +++++++-------
>>>  fs/btrfs/tree-log.c   |  3 +--
>>>  7 files changed, 20 insertions(+), 26 deletions(-)
>>>
>>
>> What base is this on?
>>
>> I got the following when applying it for review:
>>
>> [johannes@redsun91:linux (review)]$ b4 am -o - cover.1666103172.git.dsterba@suse.com | git am
>> Looking up https://lore.kernel.org/r/cover.1666103172.git.dsterba%40suse.com
>> Grabbing thread from lore.kernel.org/all/cover.1666103172.git.dsterba%40suse.com/t.mbox.gz
>> Analyzing 5 messages in the thread
>> Checking attestation on all messages, may take a moment...
>> ---
>>   ✓ [PATCH 1/4] btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
>>   ✓ [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
>>   ✓ [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
>>   ✓ [PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector
>>   ---
>>   ✓ Signed: DKIM/suse.com
>> ---
>> Total patches: 4
>> ---
>>  Link: https://lore.kernel.org/r/cover.1666103172.git.dsterba@suse.com
>>  Base: not specified
>> Applying: btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
>>
>> Error in reading or end of file.
>> fs/btrfs/relocation.c: In function ‘build_backref_tree’:
>> fs/btrfs/relocation.c:474:16: error: too many arguments to function ‘btrfs_backref_iter_alloc’
>>   474 |         iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
>>       |                ^~~~~~~~~~~~~~~~~~~~~~~~
>> In file included from fs/btrfs/relocation.c:25:
>> fs/btrfs/backref.h:158:28: note: declared here
>>   158 | struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
>>       |                            ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> I have it in a branch on top of some misc-next snapshot, the date is
> from 3 days ago and rebase to current misc-next is clean and builds.
> 

My topmost commit is 8ffce84c9455 ("btrfs: send: fix send failure of a subcase of orphan inodes")
and I still experience build failures.

Re: [PATCH 0/4] Parameter cleanup

[David Sterba]

On Wed, Oct 19, 2022 at 03:23:23PM +0000, Johannes Thumshirn wrote:
> On 19.10.22 17:16, David Sterba wrote:
> > On Wed, Oct 19, 2022 at 10:28:53AM +0000, Johannes Thumshirn wrote:
> >> On 18.10.22 16:27, David Sterba wrote:
> >>> A few more cases where value passed by parameter can be used directly.
> >>   ✓ [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent
> >>   ✓ [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block
> >>   ✓ [PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector
> >>   ---
> >>   ✓ Signed: DKIM/suse.com
> >> ---
> >> Total patches: 4
> >> ---
> >>  Link: https://lore.kernel.org/r/cover.1666103172.git.dsterba@suse.com
> >>  Base: not specified
> >> Applying: btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc
> >>
> >> Error in reading or end of file.
> >> fs/btrfs/relocation.c: In function ‘build_backref_tree’:
> >> fs/btrfs/relocation.c:474:16: error: too many arguments to function ‘btrfs_backref_iter_alloc’
> >>   474 |         iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
> >>       |                ^~~~~~~~~~~~~~~~~~~~~~~~
> >> In file included from fs/btrfs/relocation.c:25:
> >> fs/btrfs/backref.h:158:28: note: declared here
> >>   158 | struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
> >>       |                            ^~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > I have it in a branch on top of some misc-next snapshot, the date is
> > from 3 days ago and rebase to current misc-next is clean and builds.
> > 
> 
> My topmost commit is 8ffce84c9455 ("btrfs: send: fix send failure of a subcase of orphan inodes")
> and I still experience build failures.

Works for me:

- checkout commit 8ffce84c9455
- b4 ... | git am (same as yours)
- make

And also on my current misc-next 81c7fe2157e5182.

Re: [PATCH 1/4] btrfs: sink gfp_t parameter to btrfs_backref_iter_alloc

[Anand Jain]

On 18/10/2022 22:27, David Sterba wrote:
> There's only one caller that passes GFP_NOFS, we can drop the parameter
> an use the flags directly.
> 

Compile fails with this patch, needs to update the relocate.c as well.

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 216a4485d914..f5564aa313f5 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -471,7 +471,7 @@ static noinline_for_stack struct btrfs_backref_node 
*build_backref_tree(
         int ret;
         int err = 0;

-       iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
+       iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
         if (!iter)
                 return ERR_PTR(-ENOMEM);
         path = btrfs_alloc_path();

-Anand


> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/backref.c | 5 ++---
>   fs/btrfs/backref.h | 3 +--
>   2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index e6b69ac1a77c..a5e548f30242 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2634,12 +2634,11 @@ void free_ipath(struct inode_fs_paths *ipath)
>   	kfree(ipath);
>   }
>   
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(
> -		struct btrfs_fs_info *fs_info, gfp_t gfp_flag)
> +struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info)
>   {
>   	struct btrfs_backref_iter *ret;
>   
> -	ret = kzalloc(sizeof(*ret), gfp_flag);
> +	ret = kzalloc(sizeof(*ret), GFP_NOFS);
>   	if (!ret)
>   		return NULL;
>   
> diff --git a/fs/btrfs/backref.h b/fs/btrfs/backref.h
> index 6dac462430b0..ea8b59a201e6 100644
> --- a/fs/btrfs/backref.h
> +++ b/fs/btrfs/backref.h
> @@ -155,8 +155,7 @@ struct btrfs_backref_iter {
>   	u32 end_ptr;
>   };
>   
> -struct btrfs_backref_iter *btrfs_backref_iter_alloc(
> -		struct btrfs_fs_info *fs_info, gfp_t gfp_flag);
> +struct btrfs_backref_iter *btrfs_backref_iter_alloc(struct btrfs_fs_info *fs_info);
>   
>   static inline void btrfs_backref_iter_free(struct btrfs_backref_iter *iter)
>   {

Re: [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent

[Anand Jain]

On 18/10/2022 22:27, David Sterba wrote:
> All callers pass GFP_NOFS, we can drop the parameter and use it
> directly.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/qgroup.c     | 17 +++++++----------
>   fs/btrfs/qgroup.h     |  2 +-
>   fs/btrfs/relocation.c |  2 +-
>   fs/btrfs/tree-log.c   |  3 +--
>   4 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 9334c3157c22..34f0e4dabe25 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1840,7 +1840,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
>   }
>   
>   int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> -			      u64 num_bytes, gfp_t gfp_flag)
> +			      u64 num_bytes)
>   {
>   	struct btrfs_fs_info *fs_info = trans->fs_info;
>   	struct btrfs_qgroup_extent_record *record;
> @@ -1850,7 +1850,7 @@ int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
>   	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)
>   	    || bytenr == 0 || num_bytes == 0)
>   		return 0;
> -	record = kzalloc(sizeof(*record), gfp_flag);
> +	record = kzalloc(sizeof(*record), GFP_NOFS);
>   	if (!record)
>   		return -ENOMEM;
>   
> @@ -1902,8 +1902,7 @@ int btrfs_qgroup_trace_leaf_items(struct btrfs_trans_handle *trans,
>   
>   		num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
>   
> -		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes,
> -						GFP_NOFS);
> +		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes);
>   		if (ret)
>   			return ret;
>   	}
> @@ -2102,12 +2101,11 @@ static int qgroup_trace_extent_swap(struct btrfs_trans_handle* trans,
>   	 * blocks for qgroup accounting.
>   	 */
>   	ret = btrfs_qgroup_trace_extent(trans, src_path->nodes[dst_level]->start,
> -			nodesize, GFP_NOFS);
> +					nodesize);
>   	if (ret < 0)
>   		goto out;
> -	ret = btrfs_qgroup_trace_extent(trans,
> -			dst_path->nodes[dst_level]->start,
> -			nodesize, GFP_NOFS);
> +	ret = btrfs_qgroup_trace_extent(trans, dst_path->nodes[dst_level]->start,
> +					nodesize);
>   	if (ret < 0)
>   		goto out;
>   
> @@ -2391,8 +2389,7 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans,
>   			path->locks[level] = BTRFS_READ_LOCK;
>   
>   			ret = btrfs_qgroup_trace_extent(trans, child_bytenr,
> -							fs_info->nodesize,
> -							GFP_NOFS);
> +							fs_info->nodesize);
>   			if (ret)
>   				goto out;
>   		}
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index 3fb5459c9309..7bffa10589d6 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -321,7 +321,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
>    * (NULL trans)
>    */
>   int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> -			      u64 num_bytes, gfp_t gfp_flag);
> +			      u64 num_bytes);
>   
>   /*
>    * Inform qgroup to trace all leaf items of data



> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 216a4485d914..f5564aa313f5 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -471,7 +471,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
>   	int ret;
>   	int err = 0;
>   
> -	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
> +	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
>   	if (!iter)
>   		return ERR_PTR(-ENOMEM);
>   	path = btrfs_alloc_path();


  This change should be part of the patch 1/4.
  Except that, rest looks good.


-Anand

> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 813986e38258..3b44b325aba6 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -747,8 +747,7 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
>   		 */
>   		ret = btrfs_qgroup_trace_extent(trans,
>   				btrfs_file_extent_disk_bytenr(eb, item),
> -				btrfs_file_extent_disk_num_bytes(eb, item),
> -				GFP_NOFS);
> +				btrfs_file_extent_disk_num_bytes(eb, item));
>   		if (ret < 0)
>   			goto out;
>   

Re: [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block

[Anand Jain]

On 18/10/2022 22:27, David Sterba wrote:
> There's only one caller that calls scrub_setup_recheck_block in the
> memalloc_nofs_save/_restore protection so it's effectively already
> GFP_NOFS and it's safe to use GFP_KERNEL.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/scrub.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 9e3b2e60e571..2fc70a2cc7fe 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1491,7 +1491,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
>   			return -EIO;
>   		}
>   
> -		recover = kzalloc(sizeof(struct scrub_recover), GFP_NOFS);
> +		recover = kzalloc(sizeof(struct scrub_recover), GFP_KERNEL);


I didn't get why GFP_KERNEL is better here, or would it make any 
difference, given that we are already (and rightly) in the 
memalloc_nofs_save() scope.

Thanks, Anand


>   		if (!recover) {
>   			btrfs_put_bioc(bioc);
>   			btrfs_bio_counter_dec(fs_info);
> @@ -1514,7 +1514,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
>   			sblock = sblocks_for_recheck[mirror_index];
>   			sblock->sctx = sctx;
>   
> -			sector = alloc_scrub_sector(sblock, logical, GFP_NOFS);
> +			sector = alloc_scrub_sector(sblock, logical, GFP_KERNEL);
>   			if (!sector) {
>   				spin_lock(&sctx->stat_lock);
>   				sctx->stat.malloc_errors++;

Re: [PATCH 4/4] btrfs: sink gfp_t parameter to alloc_scrub_sector

[Anand Jain]

On 18/10/2022 22:27, David Sterba wrote:
> All callers pas GFP_KERNEL as parameter so we can use it directly in
> alloc_scrub_sector.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block

[David Sterba]

On Thu, Oct 20, 2022 at 03:27:31PM +0800, Anand Jain wrote:
> On 18/10/2022 22:27, David Sterba wrote:
> > There's only one caller that calls scrub_setup_recheck_block in the
> > memalloc_nofs_save/_restore protection so it's effectively already
> > GFP_NOFS and it's safe to use GFP_KERNEL.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/scrub.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> > index 9e3b2e60e571..2fc70a2cc7fe 100644
> > --- a/fs/btrfs/scrub.c
> > +++ b/fs/btrfs/scrub.c
> > @@ -1491,7 +1491,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
> >   			return -EIO;
> >   		}
> >   
> > -		recover = kzalloc(sizeof(struct scrub_recover), GFP_NOFS);
> > +		recover = kzalloc(sizeof(struct scrub_recover), GFP_KERNEL);
> 
> 
> I didn't get why GFP_KERNEL is better here, or would it make any 
> difference, given that we are already (and rightly) in the 
> memalloc_nofs_save() scope.

You said what's the reason, so I can only repeat the patterns:

1) plain GFP_NOFS, with notice that it does not work with kvalloc
2) memalloc_nofs_save/k.alloc(GFP_KERNEL)/memalloc_nofs_restore

I.e. GFP_NOFS in the memalloc_nofs_ protection is redundant because we
get th NOFS protection and can safely use GFP_KERNEL. Because we want to
minimize use of GFP_NOFS or completely switch to scoped nofs, but this
requires other changes and we do that incrementally.

Re: [PATCH 2/4] btrfs: sink gfp_t parameter to btrfs_qgroup_trace_extent

[David Sterba]

On Thu, Oct 20, 2022 at 02:01:56PM +0800, Anand Jain wrote:
> On 18/10/2022 22:27, David Sterba wrote:
> > All callers pass GFP_NOFS, we can drop the parameter and use it
> > directly.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   fs/btrfs/qgroup.c     | 17 +++++++----------
> >   fs/btrfs/qgroup.h     |  2 +-
> >   fs/btrfs/relocation.c |  2 +-
> >   fs/btrfs/tree-log.c   |  3 +--
> >   4 files changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > index 9334c3157c22..34f0e4dabe25 100644
> > --- a/fs/btrfs/qgroup.c
> > +++ b/fs/btrfs/qgroup.c
> > @@ -1840,7 +1840,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
> >   }
> >   
> >   int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> > -			      u64 num_bytes, gfp_t gfp_flag)
> > +			      u64 num_bytes)
> >   {
> >   	struct btrfs_fs_info *fs_info = trans->fs_info;
> >   	struct btrfs_qgroup_extent_record *record;
> > @@ -1850,7 +1850,7 @@ int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> >   	if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags)
> >   	    || bytenr == 0 || num_bytes == 0)
> >   		return 0;
> > -	record = kzalloc(sizeof(*record), gfp_flag);
> > +	record = kzalloc(sizeof(*record), GFP_NOFS);
> >   	if (!record)
> >   		return -ENOMEM;
> >   
> > @@ -1902,8 +1902,7 @@ int btrfs_qgroup_trace_leaf_items(struct btrfs_trans_handle *trans,
> >   
> >   		num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
> >   
> > -		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes,
> > -						GFP_NOFS);
> > +		ret = btrfs_qgroup_trace_extent(trans, bytenr, num_bytes);
> >   		if (ret)
> >   			return ret;
> >   	}
> > @@ -2102,12 +2101,11 @@ static int qgroup_trace_extent_swap(struct btrfs_trans_handle* trans,
> >   	 * blocks for qgroup accounting.
> >   	 */
> >   	ret = btrfs_qgroup_trace_extent(trans, src_path->nodes[dst_level]->start,
> > -			nodesize, GFP_NOFS);
> > +					nodesize);
> >   	if (ret < 0)
> >   		goto out;
> > -	ret = btrfs_qgroup_trace_extent(trans,
> > -			dst_path->nodes[dst_level]->start,
> > -			nodesize, GFP_NOFS);
> > +	ret = btrfs_qgroup_trace_extent(trans, dst_path->nodes[dst_level]->start,
> > +					nodesize);
> >   	if (ret < 0)
> >   		goto out;
> >   
> > @@ -2391,8 +2389,7 @@ int btrfs_qgroup_trace_subtree(struct btrfs_trans_handle *trans,
> >   			path->locks[level] = BTRFS_READ_LOCK;
> >   
> >   			ret = btrfs_qgroup_trace_extent(trans, child_bytenr,
> > -							fs_info->nodesize,
> > -							GFP_NOFS);
> > +							fs_info->nodesize);
> >   			if (ret)
> >   				goto out;
> >   		}
> > diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> > index 3fb5459c9309..7bffa10589d6 100644
> > --- a/fs/btrfs/qgroup.h
> > +++ b/fs/btrfs/qgroup.h
> > @@ -321,7 +321,7 @@ int btrfs_qgroup_trace_extent_post(struct btrfs_trans_handle *trans,
> >    * (NULL trans)
> >    */
> >   int btrfs_qgroup_trace_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> > -			      u64 num_bytes, gfp_t gfp_flag);
> > +			      u64 num_bytes);
> >   
> >   /*
> >    * Inform qgroup to trace all leaf items of data
> 
> 
> 
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 216a4485d914..f5564aa313f5 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -471,7 +471,7 @@ static noinline_for_stack struct btrfs_backref_node *build_backref_tree(
> >   	int ret;
> >   	int err = 0;
> >   
> > -	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info, GFP_NOFS);
> > +	iter = btrfs_backref_iter_alloc(rc->extent_root->fs_info);
> >   	if (!iter)
> >   		return ERR_PTR(-ENOMEM);
> >   	path = btrfs_alloc_path();
> 
> 
>   This change should be part of the patch 1/4.
>   Except that, rest looks good.

Ah I see, that's what was Johannes complained about, I did not realize
that the change was split. I'll fix it.

Re: [PATCH 3/4] btrfs: switch GFP_NOFS to GFP_KERNEL in scrub_setup_recheck_block

[Anand Jain]

On 21/10/2022 00:35, David Sterba wrote:
> On Thu, Oct 20, 2022 at 03:27:31PM +0800, Anand Jain wrote:
>> On 18/10/2022 22:27, David Sterba wrote:
>>> There's only one caller that calls scrub_setup_recheck_block in the
>>> memalloc_nofs_save/_restore protection so it's effectively already
>>> GFP_NOFS and it's safe to use GFP_KERNEL.
>>>
>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>> ---
>>>    fs/btrfs/scrub.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>>> index 9e3b2e60e571..2fc70a2cc7fe 100644
>>> --- a/fs/btrfs/scrub.c
>>> +++ b/fs/btrfs/scrub.c
>>> @@ -1491,7 +1491,7 @@ static int scrub_setup_recheck_block(struct scrub_block *original_sblock,
>>>    			return -EIO;
>>>    		}
>>>    
>>> -		recover = kzalloc(sizeof(struct scrub_recover), GFP_NOFS);
>>> +		recover = kzalloc(sizeof(struct scrub_recover), GFP_KERNEL);
>>
>>
>> I didn't get why GFP_KERNEL is better here, or would it make any
>> difference, given that we are already (and rightly) in the
>> memalloc_nofs_save() scope.
> 
> You said what's the reason, so I can only repeat the patterns:
> 
> 1) plain GFP_NOFS, with notice that it does not work with kvalloc
> 2) memalloc_nofs_save/k.alloc(GFP_KERNEL)/memalloc_nofs_restore
> 
> I.e. GFP_NOFS in the memalloc_nofs_ protection is redundant because we
> get th NOFS protection and can safely use GFP_KERNEL. 



> Because we want to
> minimize use of GFP_NOFS or completely switch to scoped nofs, but this
> requires other changes and we do that incrementally.

  Ok. Got it.