[PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[Yangtao Li]

It seems that it has never been used, so remove it.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/btrfs/delayed-ref.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index f5ae880308d3..78cc23837610 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -262,7 +262,6 @@ enum btrfs_ref_type {
 	BTRFS_REF_NOT_SET,
 	BTRFS_REF_DATA,
 	BTRFS_REF_METADATA,
-	BTRFS_REF_LAST,
 } __packed;
 
 struct btrfs_ref {
-- 
2.39.0

Re: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[Qu Wenruo]

在 2025/4/15 18:08, Yangtao Li 写道:
> It seems that it has never been used, so remove it.

History please.

> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/btrfs/delayed-ref.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index f5ae880308d3..78cc23837610 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -262,7 +262,6 @@ enum btrfs_ref_type {
>   	BTRFS_REF_NOT_SET,
>   	BTRFS_REF_DATA,
>   	BTRFS_REF_METADATA,
> -	BTRFS_REF_LAST,

The _LAST or _NR suffix can be utilized to do sanity checks, and this is 
not part of the on-disk format.

And if this exposed by some automatic tools, please also mention it.

>   } __packed;
>   
>   struct btrfs_ref {

回复: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[李扬韬]

> History please.

Did you mean change commit msg to below?

	Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented delayed ref structures") introduce BTRFS_REF_LAST but never use it,
	So let's remove it.

> The _LAST or _NR suffix can be utilized to do sanity checks, and this is not part of the on-disk format.

IIRC, delayed ref belongs to the extent tree memory kv cache.

> And if this exposed by some automatic tools, please also mention it.

I'm just looking at this code.

Thx,
Yangtao

Re: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[Qu Wenruo]

在 2025/4/15 18:26, 李扬韬 写道:
>> History please.
>
> Did you mean change commit msg to below?
>
> 	Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented delayed ref structures") introduce BTRFS_REF_LAST but never use it,
> 	So let's remove it.

It's the common practice to leave a last entry for sanity checks.

But since it's not utilized for anything, I'm fine to remove it.

>
>> The _LAST or _NR suffix can be utilized to do sanity checks, and this is not part of the on-disk format.
>
> IIRC, delayed ref belongs to the extent tree memory kv cache.
>
>> And if this exposed by some automatic tools, please also mention it.
>
> I'm just looking at this code.
>
> Thx,
> Yangtao

Re: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[Sun YangKai]

> >> History please.
> > 
> > Did you mean change commit msg to below?
> > 
> > 	Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented
> > 	delayed ref structures") introduce BTRFS_REF_LAST but never use it, So
> > 	let's remove it.
> 
> It's the common practice to leave a last entry for sanity checks.
> 
> But since it's not utilized for anything, I'm fine to remove it.
We can also add comments for all this kind of codes,
so that further readers will understand this common practice better.
> 
> >> The _LAST or _NR suffix can be utilized to do sanity checks, and this is
> >> not part of the on-disk format.> > 
> > IIRC, delayed ref belongs to the extent tree memory kv cache.
> > 
> >> And if this exposed by some automatic tools, please also mention it.
> > 
> > I'm just looking at this code.
> > 
> > Thx,
> > Yangtao

Re: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[David Sterba]

On Tue, Apr 15, 2025 at 06:46:48PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/4/15 18:26, 李扬韬 写道:
> >> History please.
> >
> > Did you mean change commit msg to below?
> >
> > 	Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented delayed ref structures") introduce BTRFS_REF_LAST but never use it,
> > 	So let's remove it.
> 
> It's the common practice to leave a last entry for sanity checks.
> 
> But since it's not utilized for anything, I'm fine to remove it.

I think in this case it's ok to remove it, although I agree that we have
the _LAST or _NR elsewhere. In btrfs_ref_type() tere's an assertion

  ASSERT(ref->type == BTRFS_REF_DATA || ref->type == BTRFS_REF_METADATA);

which is validating the values. There's no enumeration or switch that
could utilize the upper bound.

回复: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[李扬韬]

> I think in this case it's ok to remove it, although I agree that we have the _LAST or _NR elsewhere. In btrfs_ref_type() tere's an assertion

>  ASSERT(ref->type == BTRFS_REF_DATA || ref->type == BTRFS_REF_METADATA);

> which is validating the values. There's no enumeration or switch that could utilize the upper bound.

Do I need to modify the submission information and resend this patch?

Thx,
Yangtao

Re: 回复: [PATCH] btrfs: remove BTRFS_REF_LAST from btrfs_ref_type

[David Sterba]

On Wed, Apr 16, 2025 at 01:25:34PM +0000, 李扬韬 wrote:
> > I think in this case it's ok to remove it, although I agree that we have the _LAST or _NR elsewhere. In btrfs_ref_type() tere's an assertion
> 
> >  ASSERT(ref->type == BTRFS_REF_DATA || ref->type == BTRFS_REF_METADATA);
> 
> > which is validating the values. There's no enumeration or switch that could utilize the upper bound.
> 
> Do I need to modify the submission information and resend this patch?

Yes please, the reasoning was missing from the original patch, we've
come to a conclusion in the discussion so this should be summarized in
v2.