Re: [e2fsprogs PATCH] resize2fs: use directio when reading superblock

[Krister Johansen <kjlx@templeofstupid.com>]

Hi Ted,

On Fri, Jun 09, 2023 at 12:22:39AM -0400, Theodore Ts'o wrote:
> On Wed, Jun 07, 2023 at 11:50:41AM -0700, Krister Johansen wrote:
> > The growpart / resize2fs in the reproducer are essentially verbatim from
> > our system provisioning scripts.  Unless those modify the UUID, we're
> > not taking any explicit action to do so.
> 
> Ah, OK.  OK, I'm guessing that your system provisioning scripts are
> attempting mess with the file system a lot (creating, deleting, etc.)
> files while trying to run resize2fs in parallel, then?

The growpart and resize bits are triggered out of cloud-init when the
machine initially boots.  Some provisioning steps are machine-type
depenent.  The instances in which this problem have manifested are on
machines where the initial provisioning has more to do.  It's also on
machine types that are frequently provisioned.  I, unfortunately, only
get to look at these machines once they break.  It's been hard to say
whether it's because some other step of the provisioning is happening in
parallel, or if the probability is roughly the same, and these systems
are hitting it because more come and go.  I wish I had a better answer
for you. :/

> As far as your patch is concerned, resize2fs can do both off-line
> (unmounted) and on-line (mounted) resizes.  And turning direct I/O
> unconditionally isn't a great idea for off-line resizes --- it will
> really trash the performance of the resize.

Thanks for the additional detail.

I also double-checked to make sure these systems had the following patch
applied:

05c2c00f3769 ext4: protect superblock modifications with a buffer lock

And they do.  Not sure if that's directly applicable to the online
resize case though.

> Does this patch work for you instead?

Thanks, it does!

> 					- Ted
> 
> diff --git a/resize/main.c b/resize/main.c
> index 94f5ec6d..f914c050 100644
> --- a/resize/main.c
> +++ b/resize/main.c
> @@ -409,6 +409,8 @@ int main (int argc, char ** argv)
>  
>  	if (!(mount_flags & EXT2_MF_MOUNTED) && !print_min_size)
>  		io_flags = EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE;
> +	if (mount_flags & EXT2_MF_MOUNTED)
> +		io_flags |= EXT2_FLAG_DIRECT_IO;
>  
>  	io_flags |= EXT2_FLAG_64BITS | EXT2_FLAG_THREADS;
>  	if (undo_file) {

If it counts:

Reviewed-by: Krister Johansen <kjlx@templeofstupid.com>
Tested-by: Krister Johansen <kjlx@templeofstupid.com>

Thanks again,

-K