linux-ext4 - Re: [PATCH v2] ext4: shrink directory when last block is empty

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] [day] [month] [year] [list]

Message-Id: <818181A2-66E3-42D3-A65E-64F950C48516@dilger.ca>
Date:   Mon, 4 Feb 2019 12:31:40 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     harshadshirwadkar@...il.com
Cc:     linux-ext4@...r.kernel.org, tytso@....edu, umka@...udlinux.com
Subject: Re: [PATCH v2] ext4: shrink directory when last block is empty

On Feb 1, 2019, at 2:38 AM, harshadshirwadkar@...il.com wrote:
> 
> From: Harshad Shirwadkar <harshadshirwadkar@...il.com>
> 
> This patch is the first step towards shrinking htree based directories
> when files are deleted. We truncate directory inode when a directory
> entry removal causes last directory block to be empty. In order to be
> backwards compatible, we don't remove empty last directory block if it
> results in one of the intermediate htree nodes to be empty.
> 
> Changes since v1:
> - not freeing the last block if it makes intermediate htree block empty
> - style fixes
> 
> Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@...il.com>

Some minor style nits below, but looks fine otherwise, you can add:

Reviewed-by: Andreas Dilger <adilger@...ger.ca>

Have you run any kind of test (e.g. create 100k entries, ls -ld to get the
directory size, delete all entries, ls -ld to get the final "empty" size)
to see how much the directory shrinks from the maximum size?  Some info
about how well this works without further optimizations would be useful to
include into the commit comment.

It would also be good to run a loop with the above workload creating files
with different names (to change hash ordering), then unmount the filesystem
and run "e2fsck -f" on it to verify there are no errors introduced.

As the next step, it makes sense to implement checking if the previous leaf
block in the directory is also empty and free that as well.  Even before we
get to recursively freeing htree index blocks, being able to free all but
one leaf block per htree would give us a 509x reduction in space used by the
directory.

Cheers, Andreas

> +static inline bool is_empty_dirent_block(struct inode *dir,
> +					 struct buffer_head *bh)
> +{
> +	struct ext4_dir_entry_2 *de = (struct ext4_dir_entry_2 *)bh->b_data;
> +
> +	return 	ext4_rec_len_from_disk(de->rec_len, dir->i_sb->s_blocksize)

(style) remove extra space after "return"

> +			== dir->i_sb->s_blocksize && de->inode == 0;
> +}

(style) "==" should go at the end of the previous line

Cheers, Andreas

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