On Aug 8, 2020, at 9:18 AM, tytso@mit.edu wrote:

On Sat, Aug 08, 2020 at 09:29:50AM +0800, Wang Shilong wrote:

quoted

quoted

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