[PATCH] btrfs: remove unneeded variable: "ret"

[PATCH] btrfs: remove unneeded variable: "ret"

[13145886936]

From: gushengxian <gushengxian@yulong.com>

Remove unneeded variable: "ret".

Signed-off-by: gushengxian <13145886936@163.com>
Signed-off-by: gushengxian <gushengxian@yulong.com>
---
 fs/btrfs/disk-io.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b117dd3b8172..7e65a54b7839 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 	struct rb_node *node;
 	struct btrfs_delayed_ref_root *delayed_refs;
 	struct btrfs_delayed_ref_node *ref;
-	int ret = 0;
 
 	delayed_refs = &trans->delayed_refs;
 
@@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 	if (atomic_read(&delayed_refs->num_entries) == 0) {
 		spin_unlock(&delayed_refs->lock);
 		btrfs_debug(fs_info, "delayed_refs has NO entry");
-		return ret;
+		return 0;
 	}
 
 	while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
@@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 
 	spin_unlock(&delayed_refs->lock);
 
-	return ret;
+	return 0;
 }
 
 static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
-- 
2.25.1

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Qu Wenruo]

On 2021/6/28 下午4:30, 13145886936@163.com wrote:
> From: gushengxian <gushengxian@yulong.com>
>
> Remove unneeded variable: "ret".
>
> Signed-off-by: gushengxian <13145886936@163.com>
> Signed-off-by: gushengxian <gushengxian@yulong.com>

Is this detected by some script?

Mind to share the script and run it against the whole btrfs code base?

Thanks,
Qu

> ---
>   fs/btrfs/disk-io.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b117dd3b8172..7e65a54b7839 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   	struct rb_node *node;
>   	struct btrfs_delayed_ref_root *delayed_refs;
>   	struct btrfs_delayed_ref_node *ref;
> -	int ret = 0;
>
>   	delayed_refs = &trans->delayed_refs;
>
> @@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   	if (atomic_read(&delayed_refs->num_entries) == 0) {
>   		spin_unlock(&delayed_refs->lock);
>   		btrfs_debug(fs_info, "delayed_refs has NO entry");
> -		return ret;
> +		return 0;
>   	}
>
>   	while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
> @@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>
>   	spin_unlock(&delayed_refs->lock);
>
> -	return ret;
> +	return 0;
>   }
>
>   static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
>

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Damien Le Moal]

On 2021/06/28 18:22, Qu Wenruo wrote:
> 
> 
> On 2021/6/28 下午4:30, 13145886936@163.com wrote:
>> From: gushengxian <gushengxian@yulong.com>
>>
>> Remove unneeded variable: "ret".
>>
>> Signed-off-by: gushengxian <13145886936@163.com>
>> Signed-off-by: gushengxian <gushengxian@yulong.com>
> 
> Is this detected by some script?
> 
> Mind to share the script and run it against the whole btrfs code base?

make M=fs/btrfs W=1

should work.

With gcc, unused variable warnings show up only with W=1. clang is different I
think.

> 
> Thanks,
> Qu
> 
>> ---
>>   fs/btrfs/disk-io.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index b117dd3b8172..7e65a54b7839 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>>   	struct rb_node *node;
>>   	struct btrfs_delayed_ref_root *delayed_refs;
>>   	struct btrfs_delayed_ref_node *ref;
>> -	int ret = 0;
>>
>>   	delayed_refs = &trans->delayed_refs;
>>
>> @@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>>   	if (atomic_read(&delayed_refs->num_entries) == 0) {
>>   		spin_unlock(&delayed_refs->lock);
>>   		btrfs_debug(fs_info, "delayed_refs has NO entry");
>> -		return ret;
>> +		return 0;
>>   	}
>>
>>   	while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
>> @@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>>
>>   	spin_unlock(&delayed_refs->lock);
>>
>> -	return ret;
>> +	return 0;
>>   }
>>
>>   static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
>>
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Johannes Thumshirn]

On 28/06/2021 11:34, Damien Le Moal wrote:
> On 2021/06/28 18:22, Qu Wenruo wrote:
>>
>>
>> On 2021/6/28 下午4:30, 13145886936@163.com wrote:
>>> From: gushengxian <gushengxian@yulong.com>
>>>
>>> Remove unneeded variable: "ret".
>>>
>>> Signed-off-by: gushengxian <13145886936@163.com>
>>> Signed-off-by: gushengxian <gushengxian@yulong.com>
>>
>> Is this detected by some script?
>>
>> Mind to share the script and run it against the whole btrfs code base?
> 
> make M=fs/btrfs W=1
> 
> should work.
> 
> With gcc, unused variable warnings show up only with W=1. clang is different I
> think.

Nope doesn't work here, as the variable is actually used (but not needed at all).

make M=fs/btrfs W=1 just prints some warnings about improper kernel-doc formatting.

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Qu Wenruo]

On 2021/6/28 下午5:38, Johannes Thumshirn wrote:
> On 28/06/2021 11:34, Damien Le Moal wrote:
>> On 2021/06/28 18:22, Qu Wenruo wrote:
>>>
>>>
>>> On 2021/6/28 下午4:30, 13145886936@163.com wrote:
>>>> From: gushengxian <gushengxian@yulong.com>
>>>>
>>>> Remove unneeded variable: "ret".
>>>>
>>>> Signed-off-by: gushengxian <13145886936@163.com>
>>>> Signed-off-by: gushengxian <gushengxian@yulong.com>
>>>
>>> Is this detected by some script?
>>>
>>> Mind to share the script and run it against the whole btrfs code base?
>>
>> make M=fs/btrfs W=1
>>
>> should work.
>>
>> With gcc, unused variable warnings show up only with W=1. clang is different I
>> think.
>
> Nope doesn't work here, as the variable is actually used (but not needed at all).
>
> make M=fs/btrfs W=1 just prints some warnings about improper kernel-doc formatting.
>

Yeah, that's why I'm asking for the script, which could be way more
valuable than all these small fixes.

And, again a Chinese company doing the same tons of small fixes...
So nothing could change their behaviors, no matter whatever...

Thanks,
Qu

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Damien Le Moal]

On 2021/06/28 18:45, Qu Wenruo wrote:
> 
> 
> On 2021/6/28 下午5:38, Johannes Thumshirn wrote:
>> On 28/06/2021 11:34, Damien Le Moal wrote:
>>> On 2021/06/28 18:22, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2021/6/28 下午4:30, 13145886936@163.com wrote:
>>>>> From: gushengxian <gushengxian@yulong.com>
>>>>>
>>>>> Remove unneeded variable: "ret".
>>>>>
>>>>> Signed-off-by: gushengxian <13145886936@163.com>
>>>>> Signed-off-by: gushengxian <gushengxian@yulong.com>
>>>>
>>>> Is this detected by some script?
>>>>
>>>> Mind to share the script and run it against the whole btrfs code base?
>>>
>>> make M=fs/btrfs W=1
>>>
>>> should work.
>>>
>>> With gcc, unused variable warnings show up only with W=1. clang is different I
>>> think.
>>
>> Nope doesn't work here, as the variable is actually used (but not needed at all).
>>
>> make M=fs/btrfs W=1 just prints some warnings about improper kernel-doc formatting.
>>
> 
> Yeah, that's why I'm asking for the script, which could be way more
> valuable than all these small fixes.
> 
> And, again a Chinese company doing the same tons of small fixes...
> So nothing could change their behaviors, no matter whatever...

Keep cool ! This one is actually a good fix :)

Just did a make with gcc 11 and W=2 and this warning does not show up, but there
are a lot more warnings about unused macros and some "variable may be used
uninitialized" in the zone code... -> Johannes ?

There are lots of warnings about ffs() and other core functions not in btrfs
code though.



-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Qu Wenruo]

On 2021/6/28 下午5:48, Damien Le Moal wrote:
> On 2021/06/28 18:45, Qu Wenruo wrote:
>>
>>
>> On 2021/6/28 下午5:38, Johannes Thumshirn wrote:
>>> On 28/06/2021 11:34, Damien Le Moal wrote:
>>>> On 2021/06/28 18:22, Qu Wenruo wrote:
>>>>>
>>>>>
>>>>> On 2021/6/28 下午4:30, 13145886936@163.com wrote:
>>>>>> From: gushengxian <gushengxian@yulong.com>
>>>>>>
>>>>>> Remove unneeded variable: "ret".
>>>>>>
>>>>>> Signed-off-by: gushengxian <13145886936@163.com>
>>>>>> Signed-off-by: gushengxian <gushengxian@yulong.com>
>>>>>
>>>>> Is this detected by some script?
>>>>>
>>>>> Mind to share the script and run it against the whole btrfs code base?
>>>>
>>>> make M=fs/btrfs W=1
>>>>
>>>> should work.
>>>>
>>>> With gcc, unused variable warnings show up only with W=1. clang is different I
>>>> think.
>>>
>>> Nope doesn't work here, as the variable is actually used (but not needed at all).
>>>
>>> make M=fs/btrfs W=1 just prints some warnings about improper kernel-doc formatting.
>>>
>>
>> Yeah, that's why I'm asking for the script, which could be way more
>> valuable than all these small fixes.
>>
>> And, again a Chinese company doing the same tons of small fixes...
>> So nothing could change their behaviors, no matter whatever...
>
> Keep cool ! This one is actually a good fix :)

Yeah, that's why I'm not that exciting.

>
> Just did a make with gcc 11 and W=2 and this warning does not show up, but there
> are a lot more warnings about unused macros and some "variable may be used
> uninitialized" in the zone code... -> Johannes ?
>
> There are lots of warnings about ffs() and other core functions not in btrfs
> code though.

Hopes those guys see this comment and do better and more structured
cleanup before any of us :)

Thanks,
Qu

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Qu Wenruo]

Hi Gu,

On 2021/6/28 下午4:30, 13145886936@163.com wrote:
> From: gushengxian <gushengxian@yulong.com>
>
> Remove unneeded variable: "ret".

Considering you're really a newbie doing new contribution to the kernel,
I think it's better to give you some advice to encourage you do more
contribution, while also be more professional, and hopefully to help
other individuals in your company to contribute.

And I hope this could also be some guide for new developers to learn a
trick or two from.


=== WHEN NOT SURE, LEARN FROM OTHERS ===
First thing first, if you have something not sure, like how you should
setup your name and email in your git config, try to find some merged
patches to learn from them.


=== EMAIL ADDRESS ===
Firstly, your mail is sent from your 163 mail box, not your company mail
box.

Peter Zijlstra has already mentioned this in another patch.

Normally if you want to send a patch to represent your company, it's
common to send using a mail address of your company "@yulong.com" (Isn't
it called Coolpad Group nowadays?)

For how to configure git-send-email to use the SMTP server of your
company, I guess your colleague Hu Yue <huyue2@yulong.com> has more
experience and you can definitely learn from him.

If you want to the patch to be CCed to your personal mail box, you can
use "--cc" option of git-send-email, as most reviewer just reply-to-all,
thus your personal mail box will definite get the comment.

This will make the SOB line much cleaner.


=== MAIL LIST ===
This is not a big deal, just something optional but really helpful for
your next contribution.

In this particular patch, you only need to send the patch to btrfs mail
list <linux-btrfs@vger.kernel.org>, even no need to CC the maintainers.

LKML is fine for your first several patches to get more comments, like
this one.

But when you get settled to a certain field of kernel, it's better just
to send the patch to the related mail list.


=== FOR THE CLEANUP ===
As I mentioned in another thread, if you use some automatic tool or
script to expose the problem, that's fine.

But it would be even better to provide the tool. Fix one small problem
is OK, but fixing a type of problems is really what we want.

If you really just found the bug by manually scanning the code, kudos to
you.

But if you do more contribution, one day your time will be too precious
to be spent on things like this.


Just like an old Chinese saying, give a man a fish, and you feed him for
a day, teach a man to finish, and you feed him for a lifetime.



And since the patch itself is fine.

Reviewed-by: Qu Wenruo <wqu@suse.com>

And really hope you can do more and better contribution to linux kernel.

Thanks,
Qu
>
> Signed-off-by: gushengxian <13145886936@163.com>
> Signed-off-by: gushengxian <gushengxian@yulong.com>
> ---
>   fs/btrfs/disk-io.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b117dd3b8172..7e65a54b7839 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   	struct rb_node *node;
>   	struct btrfs_delayed_ref_root *delayed_refs;
>   	struct btrfs_delayed_ref_node *ref;
> -	int ret = 0;
>
>   	delayed_refs = &trans->delayed_refs;
>
> @@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   	if (atomic_read(&delayed_refs->num_entries) == 0) {
>   		spin_unlock(&delayed_refs->lock);
>   		btrfs_debug(fs_info, "delayed_refs has NO entry");
> -		return ret;
> +		return 0;
>   	}
>
>   	while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
> @@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>
>   	spin_unlock(&delayed_refs->lock);
>
> -	return ret;
> +	return 0;
>   }
>
>   static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
>

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[David Sterba]

On Mon, Jun 28, 2021 at 09:48:40AM +0000, Damien Le Moal wrote:
> This one is actually a good fix :)

It's not a good fix and has been posted several times already. What
needs to be done in this function is to propagate error codes from the
whole call tree starting in that function. Removing code triggering a
warning is perhaps the simplest thing to make the warning go away but
the right fix needs some understanding of the function and context.

The patch also comes without any explanation so that does not help to
back the intentions to remove it. Reveiew of such path is "Please
explain".

Replying to patches attempting to fix the warning (and not the code)
does not seem to help, it's just pointing to the previous iterations.

Everybody is free to run checkers, find the warnings and send patches,
that's fine and that's how open communities work.  But in this case
we'll probably have to put a note in code not to touch that particular
line/variable.

> Just did a make with gcc 11 and W=2 and this warning does not show up, but there
> are a lot more warnings about unused macros and some "variable may be used
> uninitialized" in the zone code... -> Johannes ?
> 
> There are lots of warnings about ffs() and other core functions not in btrfs
> code though.

That the higher W= warning levels are too noisy and have to be filtered
or specific issues fixed after manual selection. We've recently added a
more fine grained list of warnings that would apply only in fs/btrfs, so
if you find more that are worth fixing and then enabling by default, no
problem.

Some set-but-not used could be useful to point to code to analyze if
it's not obscuring some bug but given that thre are lots of instances in
the system includes we can't enable it.

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Damien Le Moal]

On 2021/06/28 21:07, David Sterba wrote:
> On Mon, Jun 28, 2021 at 09:48:40AM +0000, Damien Le Moal wrote:
>> This one is actually a good fix :)
> 
> It's not a good fix and has been posted several times already. What
> needs to be done in this function is to propagate error codes from the
> whole call tree starting in that function. Removing code triggering a
> warning is perhaps the simplest thing to make the warning go away but
> the right fix needs some understanding of the function and context.

Thanks for clarifying. I actually did not look into the details of the patch.
From a 10,000 feet view, it did seem like something OK. Obviously, it is not :)

> 
> The patch also comes without any explanation so that does not help to
> back the intentions to remove it. Reveiew of such path is "Please
> explain".
> 
> Replying to patches attempting to fix the warning (and not the code)
> does not seem to help, it's just pointing to the previous iterations.
> 
> Everybody is free to run checkers, find the warnings and send patches,
> that's fine and that's how open communities work.  But in this case
> we'll probably have to put a note in code not to touch that particular
> line/variable.
> 
>> Just did a make with gcc 11 and W=2 and this warning does not show up, but there
>> are a lot more warnings about unused macros and some "variable may be used
>> uninitialized" in the zone code... -> Johannes ?
>>
>> There are lots of warnings about ffs() and other core functions not in btrfs
>> code though.
> 
> That the higher W= warning levels are too noisy and have to be filtered
> or specific issues fixed after manual selection. We've recently added a
> more fine grained list of warnings that would apply only in fs/btrfs, so
> if you find more that are worth fixing and then enabling by default, no
> problem.
> 
> Some set-but-not used could be useful to point to code to analyze if
> it's not obscuring some bug but given that thre are lots of instances in
> the system includes we can't enable it.

Yes. Agree there. I personally use W=1 and W=2 only when compile testing patches
before sending. Not having these enabled by default is fine with me. And since I
just noticed the warnings related to zone code with W=2, I mentioned it. Most of
the W=2 warnings for btrfs are clearly not related to btrfs itself.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Qu Wenruo]

On 2021/6/28 下午8:04, David Sterba wrote:
> On Mon, Jun 28, 2021 at 09:48:40AM +0000, Damien Le Moal wrote:
>> This one is actually a good fix :)
> 
> It's not a good fix and has been posted several times already. What
> needs to be done in this function is to propagate error codes from the
> whole call tree starting in that function. Removing code triggering a
> warning is perhaps the simplest thing to make the warning go away but
> the right fix needs some understanding of the function and context.
> 
> The patch also comes without any explanation so that does not help to
> back the intentions to remove it. Reveiew of such path is "Please
> explain".
> 
> Replying to patches attempting to fix the warning (and not the code)
> does not seem to help, it's just pointing to the previous iterations.

But in this particular patch, it's really doing proper cleanup.

The truth is, the whole function, btrfs_destroy_delayed_refs(), only get 
called when a transaction is aborted, and to cleanup all pending delayed 
refs.

Thus in this particular function, there is nothing to return an error, 
since we're already erroring out.

And in fact we should use void for btrfs_destroy_delayed_refs().

Thus opposite to my initial thought, it's in fact OK for the cleanup.
Just one step left to change the function to return void.

And obviously, with all these comment above added to commit message.


If this is from some guy with years experience, I would definitely bark 
at him, but this is one really looks like a newbie, thus it's more or 
less acceptable.

Thanks,
Qu
> 
> Everybody is free to run checkers, find the warnings and send patches,
> that's fine and that's how open communities work.  But in this case
> we'll probably have to put a note in code not to touch that particular
> line/variable.
> 
>> Just did a make with gcc 11 and W=2 and this warning does not show up, but there
>> are a lot more warnings about unused macros and some "variable may be used
>> uninitialized" in the zone code... -> Johannes ?
>>
>> There are lots of warnings about ffs() and other core functions not in btrfs
>> code though.
> 
> That the higher W= warning levels are too noisy and have to be filtered
> or specific issues fixed after manual selection. We've recently added a
> more fine grained list of warnings that would apply only in fs/btrfs, so
> if you find more that are worth fixing and then enabling by default, no
> problem.
> 
> Some set-but-not used could be useful to point to code to analyze if
> it's not obscuring some bug but given that thre are lots of instances in
> the system includes we can't enable it.
> 

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[Nikolay Borisov]

<snip>

> 
> If this is from some guy with years experience, I would definitely bark
> at him, but this is one really looks like a newbie, thus it's more or
> less acceptable.

I beg to differ, if we want to have quality code we should be imposing
standards and shouldn't be doing exception just because someone is a
newbie. SO even if the patch is good per-se, because removing the ret is
fine, missing comprehensible rationale of why this is the case
constitutes a patch which falls short of said standards. In this case
feedback should be given and if the person doesn't take a note and
improve on his subsequent posting they should be ignored as this is
actively wasting everyone's time.


> 
> Thanks,
> Qu
<snip>

Re: [PATCH] btrfs: remove unneeded variable: "ret"

[David Sterba]

On Mon, Jun 28, 2021 at 08:19:28PM +0800, Qu Wenruo wrote:
> On 2021/6/28 下午8:04, David Sterba wrote:
> > On Mon, Jun 28, 2021 at 09:48:40AM +0000, Damien Le Moal wrote:
> >> This one is actually a good fix :)
> > 
> > It's not a good fix and has been posted several times already. What
> > needs to be done in this function is to propagate error codes from the
> > whole call tree starting in that function. Removing code triggering a
> > warning is perhaps the simplest thing to make the warning go away but
> > the right fix needs some understanding of the function and context.
> > 
> > The patch also comes without any explanation so that does not help to
> > back the intentions to remove it. Reveiew of such path is "Please
> > explain".
> > 
> > Replying to patches attempting to fix the warning (and not the code)
> > does not seem to help, it's just pointing to the previous iterations.
> 
> But in this particular patch, it's really doing proper cleanup.
> 
> The truth is, the whole function, btrfs_destroy_delayed_refs(), only get 
> called when a transaction is aborted, and to cleanup all pending delayed 
> refs.
> 
> Thus in this particular function, there is nothing to return an error, 
> since we're already erroring out.
> 
> And in fact we should use void for btrfs_destroy_delayed_refs().
> 
> Thus opposite to my initial thought, it's in fact OK for the cleanup.
> Just one step left to change the function to return void.
> 
> And obviously, with all these comment above added to commit message.

The logic about making a function void and removing the return value has
some assumptions:

- no BUG/BUG_ON inside the function itself
- all called functions are return-value clean and either handle
  everything transparently or return a value
- the above applies recursively to the whole call chain

btrfs_destroy_delayed_refs itself contains one BUG, so that needs to be
fixed. The whole problem of making it void is in the pinned rage, see
eg. btrfs_error_unpin_extent_range. Nikolay did some cleanups there but
there are still some BUGs left in place of error handling, namely in
unpin_extent_range and I did not check completely.

I understand that it's tempting to just turn the function to void,
because there's nothing obviously wrong, but that's only on first sight.
A proper review must follow the code and once there's unhandled error
deeper in the callchain, it has to be solved first.

IIRC we've cleaned most of if not all the easy cases so it may not be
that easy to identify something we haven't seen yet. I'll put something
to the wiki in case people are interested. There's
https://btrfs.wiki.kernel.org/index.php/Project_ideas#Cleanup_projects
it hasn't been up to date but it is the place where to look for such
things.