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:   Wed, 30 Jun 2021 19:17:50 -0700
From:   Josh Triplett <josh@joshtriplett.org>
To:     Theodore Ts'o <tytso@mit.edu>
Cc:     linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
        Andreas Dilger <adilger.kernel@dilger.ca>
Subject: Re: [PATCH 1/2] fs: ext4: Consolidate checks for resize of bigalloc
 into ext4_resize_begin
Message-ID: <YN0lzsMfCWli9Qp+@localhost>
References: <bee03303d999225ecb3bfa5be8576b2f4c6edbe6.1623093259.git.josh@joshtriplett.org>
 <YNSVUy/DTWdhzuci@mit.edu>
 <YN0QxMh7ymC8obre@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YN0QxMh7ymC8obre@mit.edu>
Precedence: bulk

On Wed, Jun 30, 2021 at 08:48:04PM -0400, Theodore Ts'o wrote:
> On Thu, Jun 24, 2021 at 10:23:15AM -0400, Theodore Ts'o wrote:
> > On Mon, Jun 07, 2021 at 12:15:08PM -0700, Josh Triplett wrote:
> > > Two different places checked for attempts to resize a filesystem with
> > > the bigalloc feature. Move the check into ext4_resize_begin, which both
> > > places already call.
> > > 
> > > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > Applied, thanks.
> 
> I'm going to have to revert this change, since it breaks online
> resizing for bigalloc file system.  The issue is that
> ext4_resize_begin() is called from *three* places: for
> EXT4_IOC_GROUP_ADD, EXT4_IOC_GROUP_EXTEND, and EXT4_IOC_RESIZE_FS.
> The first two ioctls are used for the "old-style" online resize, and
> bigalloc is not supported for those two ioctls.  However, it *is*
> supposed to work for EXT4_IOC_RESIZE_FS.
> 
> Unfortunately, this just caused some tests to be skipped (assuming
> that this was an old kernel that didn't support this feature) and I
> didn't notice it right away.

Ah, I see. I didn't realize that resizing bigalloc was possible with
EXT4_IOC_RESIZE_FS; I'd assumed (incorrectly) from the error message
that it wasn't.

This patch was *purely* a side story of the second patch. I'd discovered
that the kernel couldn't successfully resize a filesystem with
sparse_super2, and wanted to catch that in the kernel and return a clear
error, rather than partially resizing the filesystem. In the course of
making that change, I noticed the two copies of the error for the
bigalloc case and tried to consolidate them.

Sorry to have missed the third case here, and no problems with the
revert. I'm hoping that the second patch can be kept as-is, assuming
there's no support for resizing sparse_super2 by any code path?

- Josh Triplett