Date:   Sat, 8 Aug 2020 22:33:08 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     tytso@....edu
Cc:     Wang Shilong <wangshilong1991@...il.com>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Wang Shilong <wshilong@....com>, Shuichi Ihara <sihara@....com>
Subject: Re: [PATCH v3 1/2] ext4: introduce EXT4_BG_WAS_TRIMMED to optimize
 trim

On Aug 8, 2020, at 9:18 AM, tytso@....edu wrote:
> 
> On Sat, Aug 08, 2020 at 09:29:50AM +0800, Wang Shilong wrote:
>>> I suppose the question is whether the sysadmin really wants unused
>>> blocks to be discarded, either to not leak blocks in some kind of
>>> thin-provisioned storage device, or if the sysadmin is depending on
>>> the discard for some kind of security/privacy application (because
>>> they know that a particular storage device actually has reliable,
>>> secure discards), and how does that get balanced with sysadmins think
>>> performance of fstrim is more important, especially if the device is
>>> really slow at doing discard.
>> 
>> Yup, that is good point, for our case, fstrim could take hours to complete
>> as it needs extra IO for disk arrays, so we really want repeated fstrim.
>> 
>> So what do you think extra mount option or a feature bit in the superblock.
>> In default, we still keep ext4 in previous behavior, but once turned
>> on it, we have this optimized  "inaccurate" optimizations.
> 
> So what I was thinking was we could define a new flag which would be
> set in es->s_flags in the on-disk superblock:
> 
> #define EXT2_FLAGS_PERSISTENT_TRIM_TRACKING 0x0008
> 
> If this flag is set, then the EXT4_BG_WAS_TRIMMED flags will be
> honored; otherwise, they will be ignored when FITRIM is executed and
> the block group will be unconditionally trimmed.
> 
> The advantage of doing this way is that we don't need to allocate a
> new feature bit, and older versions of e2fsck won't have heartburn
> over seeing a feature bit it doesn't understand.  I also suspect this
> is something that the system administrator will either always want
> enabled or disabled, so it's better to make it be a tunable to be set
> via tune2fs.
> 
> The other thing we could do is to define a new variant of the FITRIM
> ioctl which will also force the unconditional trimming of the block
> groups, so that an administrator can force trim all of the block
> groups without needing to mess with mounting and unmounting the
> superblock.
> 
> What do you think?

What about storing "s_min_freed_blocks_to_trim" persistently in the
superblock, and then the admin can adjust this as desired?  If it is
set =1, then the "lazy trim" optimization would be disabled (every
FITRIM request would honor the trim requests whenever there is a
freed block in a group).  I suppose we could allow =0 to mean "do not
store the WAS_TRIMMED flag persistently", so there would be no change
for current behavior, and it would require a tune2fs option to set the
new value into the superblock (though we might consider setting this
to a non-zero value in mke2fs by default).



The other thing we were thinkgin about was changing the "-o discard" code
to leverage the WAS_TRIMMED flag, and just do bulk trim periodically
in the filesystem as blocks are freed from groups, rather than tracking
freed extents in memory and submitting trims actively during IO.  Instead,
it would track groups that exceed "s_min_freed_blocks_to_trim", and trim
the whole group in the background when the filesystem is not active.

Cheers, Andreas






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

Powered by blists - more mailing lists