linux-kernel - Re: [PATCH 2/6] ext4: remove unneeded bits mask in dx_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <8554AB6E-55CA-4F61-BE34-1260555785D1@dilger.ca>
Date: Thu, 19 Dec 2024 18:10:24 -0700
From: Andreas Dilger <adilger@...ger.ca>
To: Kemeng Shi <shikemeng@...weicloud.com>
Cc: Theodore Ts'o <tytso@....edu>,
 Ext4 Developers List <linux-ext4@...r.kernel.org>,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/6] ext4: remove unneeded bits mask in dx_get_block()

On Dec 19, 2024, at 4:00 AM, Kemeng Shi <shikemeng@...weicloud.com> wrote:
> 
> As high four bits of block in dx_entry is not used by any feature for now, we can remove unneeded bits mask in dx_get_block() and add it back
> when it's really needed.

Actually, the opposite is true.  This mask protects the *CURRENT* code
from any future use for these bits, so removing it now means that they
could never be used in the future, since the block number would be
taken as all 32 bits instead of only the bottom 28 bits.  I don't think
we are in any danger of having a 16TB single directory any time soon.

However, the top bits were intended to store a "fullness" for the index
blocks, to optimize online directory shrinking without having to scan
each of the blocks for how many entries are currently in the block.
This would allow the dirent removal to easily see "this block and the
previous/next block are only 1/3 full and could be merged".

See the following thread for a prototype patch and discussion on this:
https://patchwork.ozlabs.org/project/linux-ext4/patch/20190821182740.97127-1-harshadshirwadkar@gmail.com/

I think removing this mask has a negative effect on future usefulness,
and virtually no benefit to the code today, so I would object to landing
it.

Cheers, Andreas

> 
> Signed-off-by: Kemeng Shi <shikemeng@...weicloud.com>
> ---
> fs/ext4/namei.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index adec145b6f7d..8ff840ef4730 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -561,14 +561,9 @@ ext4_next_entry(struct ext4_dir_entry_2 *p, unsigned long blocksize)
> 		ext4_rec_len_from_disk(p->rec_len, blocksize));
> }
> 
> -/*
> - * Future: use high four bits of block for coalesce-on-delete flags
> - * Mask them off for now.
> - */
> -
> static inline ext4_lblk_t dx_get_block(struct dx_entry *entry)
> {
> -	return le32_to_cpu(entry->block) & 0x0fffffff;
> +	return le32_to_cpu(entry->block);
> }
> 
> static inline void dx_set_block(struct dx_entry *entry, ext4_lblk_t value)
> --
> 2.30.0
> 

Cheers, Andreas

Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)