On Feb 1, 2019, at 2:38 AM, harshadshirwadkar@xxxxxxxxx wrote:
>
> From: Harshad Shirwadkar <harshadshirwadkar@xxxxxxxxx>
>
> 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@xxxxxxxxx>
Some minor style nits below, but looks fine otherwise, you can add:
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
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
Attachment:
signature.asc
Description: Message signed with OpenPGP
