[PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

The reclaim_size counter of a space_info object is unsigned. So its value
can never be negative, it's pointless to have an assertion that checks
its value is >= 0, therefore remove it.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/space-info.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index ff17a4420358..88d7dea215ff 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1198,7 +1198,6 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
 	 * the list and we will do our own flushing further down.
 	 */
 	if (ret && flush != BTRFS_RESERVE_NO_FLUSH) {
-		ASSERT(space_info->reclaim_size >= 0);
 		ticket.bytes = orig_bytes;
 		ticket.error = 0;
 		space_info->reclaim_size += ticket.bytes;
-- 
2.11.0

Re: [PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[Nikolay Borisov]

On 7.04.20 г. 13:38 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> The reclaim_size counter of a space_info object is unsigned. So its value
> can never be negative, it's pointless to have an assertion that checks
> its value is >= 0, therefore remove it.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

True,

Reviewed-by: Nikolay Borisov <nborisov@suse.com> I guess this could be
squashed.

> ---
>  fs/btrfs/space-info.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index ff17a4420358..88d7dea215ff 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -1198,7 +1198,6 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
>  	 * the list and we will do our own flushing further down.
>  	 */
>  	if (ret && flush != BTRFS_RESERVE_NO_FLUSH) {
> -		ASSERT(space_info->reclaim_size >= 0);
>  		ticket.bytes = orig_bytes;
>  		ticket.error = 0;
>  		space_info->reclaim_size += ticket.bytes;
> 

Re: [PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[Filipe Manana]

On Tue, Apr 7, 2020 at 12:32 PM Nikolay Borisov <nborisov@suse.com> wrote:
>
>
>
> On 7.04.20 г. 13:38 ч., fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > The reclaim_size counter of a space_info object is unsigned. So its value
> > can never be negative, it's pointless to have an assertion that checks
> > its value is >= 0, therefore remove it.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
>
> True,
>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com> I guess this could be
> squashed.

Despite being a trivial and small change, I don't think it should be
squashed into the previous patch, as it's not part of the bug fix
regarding the counter leak.
Different changes and unrelated changes should be separate patches.

Thanks.

>
> > ---
> >  fs/btrfs/space-info.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> > index ff17a4420358..88d7dea215ff 100644
> > --- a/fs/btrfs/space-info.c
> > +++ b/fs/btrfs/space-info.c
> > @@ -1198,7 +1198,6 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
> >        * the list and we will do our own flushing further down.
> >        */
> >       if (ret && flush != BTRFS_RESERVE_NO_FLUSH) {
> > -             ASSERT(space_info->reclaim_size >= 0);
> >               ticket.bytes = orig_bytes;
> >               ticket.error = 0;
> >               space_info->reclaim_size += ticket.bytes;
> >

Re: [PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[Nikolay Borisov]

On 7.04.20 г. 17:14 ч., Filipe Manana wrote:
> On Tue, Apr 7, 2020 at 12:32 PM Nikolay Borisov <nborisov@suse.com> wrote:
>>
>>
>>
>> On 7.04.20 г. 13:38 ч., fdmanana@kernel.org wrote:
>>> From: Filipe Manana <fdmanana@suse.com>
>>>
>>> The reclaim_size counter of a space_info object is unsigned. So its value
>>> can never be negative, it's pointless to have an assertion that checks
>>> its value is >= 0, therefore remove it.
>>>
>>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>>
>> True,
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com> I guess this could be
>> squashed.
> 
> Despite being a trivial and small change, I don't think it should be
> squashed into the previous patch, as it's not part of the bug fix
> regarding the counter leak.
> Different changes and unrelated changes should be separate patches.

I meant to say squashed into the original commit that introduced the
assert but it seems it has already been merged into master so yeah, it
will go as it is.

Re: [PATCH 2/2] Btrfs: remove pointless assertion on reclaim_size counter

[David Sterba]

On Tue, Apr 07, 2020 at 11:38:58AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> The reclaim_size counter of a space_info object is unsigned. So its value
> can never be negative, it's pointless to have an assertion that checks
> its value is >= 0, therefore remove it.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Added to misc-next, thanks.