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;
Date:   Tue, 18 Jan 2022 10:03:46 +0100
From:   Lukas Czerner <lczerner@redhat.com>
To:     Sasha Levin <sashal@kernel.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Laurent GUERBY <laurent@guerby.net>,
        Andreas Dilger <adilger@dilger.ca>,
        Theodore Ts'o <tytso@mit.edu>, adilger.kernel@dilger.ca,
        linux-ext4@vger.kernel.org
Subject: Re: [PATCH AUTOSEL 5.10 116/116] ext4: allow to change
 s_last_trim_minblks via sysfs
Message-ID: <20220118090346.nw4ckd5smuvui2rp@work>
References: <20220118024007.1950576-1-sashal@kernel.org>
 <20220118024007.1950576-116-sashal@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220118024007.1950576-116-sashal@kernel.org>
Precedence: bulk

Why was this selected? It is a new feature not a fix, it's not marked
for a stable kernel and moreover it's incomplete
(2327fb2e23416cfb2795ccca2f77d4d65925be99 is a prerequisite).

I have the same question for the patches for 5.15 and 5.16.

I think we should either drop it, or at the very least include the above
mentioned commit as well.

Thanks!
-Lukas

On Mon, Jan 17, 2022 at 09:40:07PM -0500, Sasha Levin wrote:
> From: Lukas Czerner <lczerner@redhat.com>
> 
> [ Upstream commit 4a69aecbfb30a3fc85bf8028386c047d5607a97a ]
> 
> Ext4 has an optimization mechanism for batched disacrd (FITRIM) that
> should help speed up subsequent calls of FITRIM ioctl by skipping the
> groups that were previously trimmed. However because the FITRIM allows
> to set the minimum size of an extent to trim, ext4 stores the last
> minimum extent size and only avoids trimming the group if it was
> previously trimmed with minimum extent size equal to, or smaller than
> the current call.
> 
> There is currently no way to bypass the optimization without
> umount/mount cycle. This becomes a problem when the file system is
> live migrated to a different storage, because the optimization will
> prevent possibly useful discard calls to the storage.
> 
> Fix it by exporting the s_last_trim_minblks via sysfs interface which
> will allow us to set the minimum size to the number of blocks larger
> than subsequent FITRIM call, effectively bypassing the optimization.
> 
> By setting the s_last_trim_minblks to ULONG_MAX the optimization will be
> effectively cleared regardless of the previous state, or file system
> configuration.
> 
> For example:
> getconf ULONG_MAX > /sys/fs/ext4/dm-1/last_trim_minblks
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Laurent GUERBY <laurent@guerby.net>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Link: https://lore.kernel.org/r/20211103145122.17338-2-lczerner@redhat.com
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  fs/ext4/sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
> index f24bef3be48a3..4192b4af10602 100644
> --- a/fs/ext4/sysfs.c
> +++ b/fs/ext4/sysfs.c
> @@ -250,6 +250,7 @@ EXT4_ATTR(last_error_time, 0444, last_error_time);
>  EXT4_ATTR(journal_task, 0444, journal_task);
>  EXT4_RW_ATTR_SBI_UI(mb_prefetch, s_mb_prefetch);
>  EXT4_RW_ATTR_SBI_UI(mb_prefetch_limit, s_mb_prefetch_limit);
> +EXT4_RW_ATTR_SBI_UL(last_trim_minblks, s_last_trim_minblks);
>  
>  static unsigned int old_bump_val = 128;
>  EXT4_ATTR_PTR(max_writeback_mb_bump, 0444, pointer_ui, &old_bump_val);
> @@ -299,6 +300,7 @@ static struct attribute *ext4_attrs[] = {
>  #endif
>  	ATTR_LIST(mb_prefetch),
>  	ATTR_LIST(mb_prefetch_limit),
> +	ATTR_LIST(last_trim_minblks),
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(ext4);
> -- 
> 2.34.1
>