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

[Larry Chen]

Hi Changwei,

Thanks for your suggestions.

On 02/24/2018 12:40 PM, Changwei Ge wrote:
> Hi Larry,
> I have some quick questions:
> 1) It seems that your patch doesn't check if the backing disk is SSD or not, is
> that reasonable?
It that really necessary to check whether the backing disk is SSD or not?
If it is not an SSD or discard operation is not supported, the 
discard_blocks
function will exit on its first try to call _discard_blocks.

> 2) Why not just trim SSD before 'mkfs' performed by users? It's more
> straightforward.
Yeah, but that's one more step for users, isn't it?
> 3) Could you share some statistics telling the degree performance boosted.
I'll try.

Thanks,
Larry
>
> Thanks,
> Changwei
>
> On 2018/2/24 12:15, Larry Chen wrote:
>> 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 | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>    mkfs.ocfs2/mkfs.h |  9 +++++++++
>>    2 files changed, 53 insertions(+)
>>
>> diff --git a/mkfs.ocfs2/mkfs.c b/mkfs.ocfs2/mkfs.c
>> index 354b2ee5..e9c8a9fb 100644
>> --- a/mkfs.ocfs2/mkfs.c
>> +++ b/mkfs.ocfs2/mkfs.c
>> @@ -543,6 +543,48 @@ static void finish_normal_format(State *s)
>>    	ocfs2_close(fs);
>>    }
>>    
>> +static errcode_t _discard_blocks(State *s, uint64_t block,
>> +		uint64_t count)
>> +{
>> +	int ret;
>> +	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);
>> +	if (ret < 0) {
>> +		if (errno == EOPNOTSUPP && !s->quiet)
>> +			printf("block discard not supported...skip\n");
>> +		return errno;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int discard_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)
>> +			break;
>> +		cur += count;
>> +	}
>> +
>> +	return retval;
>> +}
>> +
>> +
>>    int
>>    main(int argc, char **argv)
>>    {
>> @@ -615,6 +657,8 @@ main(int argc, char **argv)
>>    		return 0;
>>    	}
>>    
>> +	discard_blocks(s);
>> +
>>    	clear_both_ends(s);
>>    
>>    	init_record(s, &superblock_rec, SFI_OTHER, S_IFREG | 0644);
>> diff --git a/mkfs.ocfs2/mkfs.h b/mkfs.ocfs2/mkfs.h
>> index f9ba4dcf..ac9709f3 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,
>>