[Ocfs2-tools-devel] [PATCH] discard unused blocks before mkfs

[Gang He]

Hi Larry,

Please use Patch Version Number in the patch subject,
second, please write the related change log in the patch description.



>>> 
> From: Larry <lchen at suse.com>
> 
> When using a SSD as a block device, if unused SSD blocks could
> be discarded in advance, performance will be improved.
> 
> This patch uses ioctl interface to release unused blocks on SSD
> layer.
> 
> Signed-off-by: Larry Chen <lchen at suse.com>
> ---
>  mkfs.ocfs2/mkfs.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  mkfs.ocfs2/mkfs.h | 10 ++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/mkfs.ocfs2/mkfs.c b/mkfs.ocfs2/mkfs.c
> index 354b2ee5..2aff6973 100644
> --- a/mkfs.ocfs2/mkfs.c
> +++ b/mkfs.ocfs2/mkfs.c
> @@ -543,6 +543,46 @@ static void finish_normal_format(State *s)
>  	ocfs2_close(fs);
>  }
>  
> +static inline errcode_t discard_blocks(State *s, uint64_t block,
> +		uint64_t count)
> +{
> +	int ret;
The ret variable is not necessary.

> +	uint64_t range[2];
> +
> +	range[0] = (uint64_t)(block) << s->blocksize_bits;
> +	range[1] = (uint64_t)(count) << s->blocksize_bits;
> +
> +	ret = ioctl(s->fd, BLKDISCARD, &range);
> +	return ret;
> +}
> +
> +static int discard_device_blocks(State *s)
> +{
> +	uint64_t blocks = s->volume_size_in_blocks;
> +	uint64_t count = DISCARD_STEP_MB;
> +	uint64_t cur = 0;
> +	int retval = 0;
> +
> +	count *= (1024 * 1024);
> +	count >>= s->blocksize_bits;
> +
> +	while (cur < blocks) {
> +		if (cur + count > blocks)
> +			count = blocks - cur;
> +
> +		retval = discard_blocks(s, cur, count);
> +		if (retval) {
> +			if (!s->quiet && errno != EOPNOTSUPP)
> +				com_err(s->progname, errno,
> +					"While discarding unused blocks...skip\n");
> +			break;
> +		}
> +		cur += count;
> +	}
> +
> +	return retval;
> +}
> +
>  int
>  main(int argc, char **argv)
>  {
> @@ -615,6 +655,10 @@ main(int argc, char **argv)
>  		return 0;
>  	}
>  
> +	if (s->discard_blocks) {
> +		discard_device_blocks(s);
> +	}
> +
>  	clear_both_ends(s);
>  
>  	init_record(s, &superblock_rec, SFI_OTHER, S_IFREG | 0644);
> @@ -889,6 +933,7 @@ get_state(int argc, char **argv)
>  	int no_backup_super = -1;
>  	enum ocfs2_feature_levels level = OCFS2_FEATURE_LEVEL_DEFAULT;
>  	ocfs2_fs_options feature_flags = {0,0,0}, reverse_flags = {0,0,0};
> +	int discard_blocks = 1;
>  
>  	static struct option long_options[] = {
>  		{ "block-size", 1, 0, 'b' },
> @@ -903,6 +948,7 @@ get_state(int argc, char **argv)
>  		{ "force", 0, 0, 'F'},
>  		{ "mount", 1, 0, 'M'},
>  		{ "dry-run", 0, 0, 'n' },
> +		{ "no-discard", 0, 0, 'o'},
the option "nodiscard" looks more user-friendly, I suggest you also handle "discard" option,
since the mkfs option noXXX and XXX are always twins.
please see mkfs.ext4 man page,

                   discard
                          Attempt to discard blocks at mkfs time (discarding blocks initially is useful on solid state devices and sparse / thin-provisioned storage). When  the  device  adver-
                          tises  that  discard also zeroes data (any subsequent read after the discard and before write returns zero), then mark all not-yet-zeroed inode tables as zeroed. This
                          significantly speeds up filesystem initialization. This is set as default.

                   nodiscard
                          Do not attempt to discard blocks at mkfs time.

Finally, please update mkfs.ocfs2 man page in this patch.
Thanks
Gang 

>  		{ "no-backup-super", 0, 0, BACKUP_SUPER_OPTION },
>  		{ "fs-feature-level=", 1, 0, FEATURE_LEVEL },
>  		{ "fs-features=", 1, 0, FEATURES_OPTION },
> @@ -1103,6 +1149,10 @@ get_state(int argc, char **argv)
>  			globalhb = 1;
>  			break;
>  
> +		case 'o':
> +			discard_blocks = 0;
> +			break;
> +
>  		default:
>  			usage(progname);
>  			break;
> @@ -1148,6 +1198,7 @@ get_state(int argc, char **argv)
>  	s->quiet         = quiet;
>  	s->force         = force;
>  	s->dry_run       = dry_run;
> +	s->discard_blocks = discard_blocks;
>  
>  	s->prompt        = xtool ? 0 : 1;
>  
> diff --git a/mkfs.ocfs2/mkfs.h b/mkfs.ocfs2/mkfs.h
> index f9ba4dcf..b144341f 100644
> --- a/mkfs.ocfs2/mkfs.h
> +++ b/mkfs.ocfs2/mkfs.h
> @@ -41,6 +41,7 @@
>  #include <inttypes.h>
>  #include <ctype.h>
>  #include <assert.h>
> +#include <sys/ioctl.h>
>  
>  #include <uuid/uuid.h>
>  
> @@ -92,6 +93,14 @@
>  
>  #define MAX_EXTALLOC_RESERVE_PERCENT	5
>  
> +#define DISCARD_STEP_MB         2048
> +
> +#if defined(__linux__) && !defined(BLKDISCARD)
> +#define BLKDISCARD		_IO(0x12,119)
> +#endif
> +
> +
> +
>  enum {
>  	SFI_JOURNAL,
>  	SFI_CLUSTER,
> @@ -193,6 +202,7 @@ struct _State {
>  	int inline_data;
>  	int dx_dirs;
>  	int dry_run;
> +	int discard_blocks;
>  
>  	uint32_t blocksize;
>  	uint32_t blocksize_bits;
> -- 
> 2.16.2