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  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]
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)

Powered by blists - more mailing lists