Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1592831677-13945-1-git-send-email-wangshilong1991@gmail.com> <20200806044703.GC7657@mit.edu>
In-Reply-To: <20200806044703.GC7657@mit.edu>
From:   Wang Shilong <wangshilong1991@gmail.com>
Date:   Sat, 8 Aug 2020 09:29:50 +0800
Message-ID: <CAP9B-Qnv2LXva_szv+sDOiawQ6zRb9a8u-UAsbXqSqWiK+emiQ@mail.gmail.com>
Subject: Re: [PATCH v3 1/2] ext4: introduce EXT4_BG_WAS_TRIMMED to optimize trim
To:     "Theodore Ts'o" <tytso@mit.edu>
Cc:     Ext4 Developers List <linux-ext4@vger.kernel.org>,
        Wang Shilong <wshilong@ddn.com>,
        Shuichi Ihara <sihara@ddn.com>,
        Andreas Dilger <adilger@dilger.ca>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 6, 2020 at 12:47 PM <tytso@mit.edu> wrote:
>
> On Mon, Jun 22, 2020 at 10:14:36PM +0900, Wang Shilong wrote:
> > From: Wang Shilong <wshilong@ddn.com>
> >
> > Currently WAS_TRIMMED flag is not persistent, whenever filesystem was
> > remounted, fstrim need walk all block groups again, the problem with
> > this is FSTRIM could be slow on very large LUN SSD based filesystem.
> >
> > To avoid this kind of problem, we introduce a block group flag
> > EXT4_BG_WAS_TRIMMED, the side effect of this is we need introduce
> > extra one block group dirty write after trimming block group.
> >
> > And When clearing TRIMMED flag, block group will be journalled
> > anyway, so it won't introduce any overhead.
>
> This persistent flag will not be accurate if there are blocks that
> were freed in the block group in the same transaction, before
> EXT4_BG_WAS_TRIMMED flag is set.

Yup, i thought something like this, this might not be accurate, but this won't
cause some data corruptions etc, and trying to write data which was not
trimmed in advance, SSD will do erase finally.

And i sent e2fsprogs support which could clear all block groups
EXT4_BG_WAS_TRIMMED
flags, but it needs umounted state of course.

And for our case, when admin running fstrim on filesystem, usually there
are few IO for filesystem, so most of case, it might be fine.
>
> That's because we can't trim (or reuse) a block which has been
> released until the transaction has committed, since if we crash before
> it is commited, the file unlink or truncate will not have happened,
> and so we can't trash the block until after the deallocation has been
> freed.
>
> This problem is also there with a non-persistent flag, granted; but
> when the file system is unmounted and remounted, we will eventually
> trim the block via a fstrim.  When we make the flag persistent, the
> problem becomes worse, since it might mean that there are some blocks
> that have been released, that might never get discarded.
>
> 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.

Thank you!
Shilong


>
>                                         - Ted