All of lore.kernel.org
 help / color / mirror / Atom feed
* [kernel-5.11 regression] tune2fs fails after shutdown
@ 2021-08-05  2:35 Boyang Xue
  2021-08-12  1:47 ` Boyang Xue
  0 siblings, 1 reply; 4+ messages in thread
From: Boyang Xue @ 2021-08-05  2:35 UTC (permalink / raw)
  To: linux-ext4

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

Hi,

kernel commit

4274f516d4bc ext4: recalucate superblock checksum after updating free
blocks/inodes

had been reverted by

81414b4dd48 ext4: remove redundant sb checksum recomputation

since kernel-5.11-rc1. As a result, the original reproducer fails again.

Reproducer:
```
mkdir mntpt
fallocate -l 256M mntpt.img
mkfs.ext4 -Fq -t ext4 mntpt.img 128M
LPDEV=$(losetup -f --show mntpt.img)
mount "$LPDEV" mntpt
cp /proc/version mntpt/
./godown mntpt # godown program attached.
umount mntpt
mount "$LPDEV" mntpt
tune2fs -l "$LPDEV"
```

tune2fs fails with
```
tune2fs 1.46.2 (28-Feb-2021)
tune2fs: Superblock checksum does not match superblock while trying to
open /dev/loop0
Couldn't find valid filesystem superblock.
```

Tested on e2fsprogs-1.46.2 + kernel-5.14.0-0.rc3.29. I think it's a
regression. If this is the case, can we fix it again please?

Thanks,
Boyang

[-- Attachment #2: godown.c --]
[-- Type: application/octet-stream, Size: 2501 bytes --]

/*
 * Copy this program from xfstests/src/godown.c
 * change some code to make sure this program can
 * be compiled individually, don't need to compile
 * with all xfstests program. 
 * (still depends on xfsprogs-devel of course)
 *
 * gcc -o godown godown.c -Wall
 *
 * Modified by: Zorro Lang <zlang@redhat.com>
 *
 */

#define _GNU_SOURCE
#include <stdio.h>
#include <syslog.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <getopt.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/statvfs.h>
#include <fcntl.h>
#include <xfs/xfs.h>

static char *xprogname;


static void
usage(void)
{
	fprintf(stderr, "usage: %s [-f] [-v] mnt-dir\n", xprogname);
}

int
main(int argc, char *argv[])
{
	int c;
	int flag;
	int flushlog_opt = 0;
	int verbose_opt = 0;
	struct stat st;
	char *mnt_dir;
	int fd;

      	xprogname = argv[0];

	while ((c = getopt(argc, argv, "fv")) != -1) {
		switch (c) {
		case 'f':
			flushlog_opt = 1;
			break;
		case 'v':
			verbose_opt = 1;
			break;
		case '?':
			usage();
			return 1;
		}
	}

	/* process required cmd argument */
	if (optind == argc-1) {
		mnt_dir = argv[optind];
	}
	else {
		usage();
		return 1;
	}

	if ((stat(mnt_dir, &st)) == -1) {
		fprintf(stderr, "%s: error on stat \"%s\": %s\n",
			xprogname, mnt_dir, strerror(errno));
		return 1;
	}

	if (!S_ISDIR(st.st_mode)) {
		fprintf(stderr, "%s: argument \"%s\" is not a directory\n",
			xprogname, mnt_dir);
		return 1;
	}

	
#if 0
	{
		struct statvfs stvfs;
		if ((statvfs(mnt_dir, &stvfs)) == -1) {
			fprintf(stderr, "%s: error on statfs \"%s\": %s\n",
				xprogname, mnt_dir, strerror(errno));
			return 1;
		}

		if (strcmp(stvfs.f_basetype, "xfs") != 0) {
			fprintf(stderr, "%s: filesys for \"%s\" is not XFS:\"%s\"\n",
				xprogname, mnt_dir, stvfs.f_basetype);
			return 1;
		}
	}
#endif


	flag = (flushlog_opt ? XFS_FSOP_GOING_FLAGS_LOGFLUSH 
			    : XFS_FSOP_GOING_FLAGS_NOLOGFLUSH);

	if (verbose_opt) {
		printf("Opening \"%s\"\n", mnt_dir);
	}
	if ((fd = open(mnt_dir, O_RDONLY)) == -1) {
		fprintf(stderr, "%s: error on open of \"%s\": %s\n",
			xprogname, mnt_dir, strerror(errno));
		return 1;
	}

	if (verbose_opt) {
		printf("Calling XFS_IOC_GOINGDOWN\n");
	}
	syslog(LOG_WARNING, "xfstests-induced forced shutdown of %s:\n",
		mnt_dir);
	if ((xfsctl(mnt_dir, fd, XFS_IOC_GOINGDOWN, &flag)) == -1) {
		fprintf(stderr, "%s: error on xfsctl(GOINGDOWN) of \"%s\": %s\n",
			xprogname, mnt_dir, strerror(errno));
		return 1;
	}

	close(fd);

	return 0;
}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [kernel-5.11 regression] tune2fs fails after shutdown
  2021-08-05  2:35 [kernel-5.11 regression] tune2fs fails after shutdown Boyang Xue
@ 2021-08-12  1:47 ` Boyang Xue
  2021-08-12 12:47   ` Jan Kara
  0 siblings, 1 reply; 4+ messages in thread
From: Boyang Xue @ 2021-08-12  1:47 UTC (permalink / raw)
  To: linux-ext4, Jan Kara; +Cc: tytso

(Adding the author of the commits)

Hi Jan,

The commit

81414b4dd48 ext4: remove redundant sb checksum recomputation

breaks the original reproducer of

4274f516d4bc ext4: recalucate superblock checksum after updating free
blocks/inodes

I'm wondering is it expected please?

Thanks,
Boyang

On Thu, Aug 5, 2021 at 10:35 AM Boyang Xue <bxue@redhat.com> wrote:
>
> Hi,
>
> kernel commit
>
> 4274f516d4bc ext4: recalucate superblock checksum after updating free
> blocks/inodes
>
> had been reverted by
>
> 81414b4dd48 ext4: remove redundant sb checksum recomputation
>
> since kernel-5.11-rc1. As a result, the original reproducer fails again.
>
> Reproducer:
> ```
> mkdir mntpt
> fallocate -l 256M mntpt.img
> mkfs.ext4 -Fq -t ext4 mntpt.img 128M
> LPDEV=$(losetup -f --show mntpt.img)
> mount "$LPDEV" mntpt
> cp /proc/version mntpt/
> ./godown mntpt # godown program attached.
> umount mntpt
> mount "$LPDEV" mntpt
> tune2fs -l "$LPDEV"
> ```
>
> tune2fs fails with
> ```
> tune2fs 1.46.2 (28-Feb-2021)
> tune2fs: Superblock checksum does not match superblock while trying to
> open /dev/loop0
> Couldn't find valid filesystem superblock.
> ```
>
> Tested on e2fsprogs-1.46.2 + kernel-5.14.0-0.rc3.29. I think it's a
> regression. If this is the case, can we fix it again please?
>
> Thanks,
> Boyang


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [kernel-5.11 regression] tune2fs fails after shutdown
  2021-08-12  1:47 ` Boyang Xue
@ 2021-08-12 12:47   ` Jan Kara
  2021-08-12 13:38     ` Boyang Xue
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2021-08-12 12:47 UTC (permalink / raw)
  To: Boyang Xue; +Cc: linux-ext4, Jan Kara, tytso

Hello Boyang,

On Thu 12-08-21 09:47:30, Boyang Xue wrote:
> (Adding the author of the commits)
> Hi Jan,
> 
> The commit
> 
> 81414b4dd48 ext4: remove redundant sb checksum recomputation
> 
> breaks the original reproducer of
> 
> 4274f516d4bc ext4: recalucate superblock checksum after updating free
> blocks/inodes
> 
> I'm wondering is it expected please?

Thanks for report! So for record the problem is not that superblock with
incorrect checksum would ever get to disk with my patches but the checksum
will be incorrect in the buffer cache until the moment we start writeout of
the superblock. And tune2fs accesses the buffer cache and sees the
incorrect (stale) checksum. It is impossible to fix this problem completely
(the tune2fs access is fundamentally racy) but yes, I guess returning the
checksum recalculation back will make the race window small and the cost is
small. I'll send a patch for this shortly.

Also can you perhaps make this sequence into a fstests testcase for ext4
filesystem so that we have it covered? Thanks!

								Honza

> On Thu, Aug 5, 2021 at 10:35 AM Boyang Xue <bxue@redhat.com> wrote:
> >
> > Hi,
> >
> > kernel commit
> >
> > 4274f516d4bc ext4: recalucate superblock checksum after updating free
> > blocks/inodes
> >
> > had been reverted by
> >
> > 81414b4dd48 ext4: remove redundant sb checksum recomputation
> >
> > since kernel-5.11-rc1. As a result, the original reproducer fails again.
> >
> > Reproducer:
> > ```
> > mkdir mntpt
> > fallocate -l 256M mntpt.img
> > mkfs.ext4 -Fq -t ext4 mntpt.img 128M
> > LPDEV=$(losetup -f --show mntpt.img)
> > mount "$LPDEV" mntpt
> > cp /proc/version mntpt/
> > ./godown mntpt # godown program attached.
> > umount mntpt
> > mount "$LPDEV" mntpt
> > tune2fs -l "$LPDEV"
> > ```
> >
> > tune2fs fails with
> > ```
> > tune2fs 1.46.2 (28-Feb-2021)
> > tune2fs: Superblock checksum does not match superblock while trying to
> > open /dev/loop0
> > Couldn't find valid filesystem superblock.
> > ```
> >
> > Tested on e2fsprogs-1.46.2 + kernel-5.14.0-0.rc3.29. I think it's a
> > regression. If this is the case, can we fix it again please?
> >
> > Thanks,
> > Boyang
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [kernel-5.11 regression] tune2fs fails after shutdown
  2021-08-12 12:47   ` Jan Kara
@ 2021-08-12 13:38     ` Boyang Xue
  0 siblings, 0 replies; 4+ messages in thread
From: Boyang Xue @ 2021-08-12 13:38 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, tytso

Thanks Jan!

Yes. I will create a test case for fstests.

Thanks,
Boyang

On Thu, Aug 12, 2021 at 8:47 PM Jan Kara <jack@suse.cz> wrote:
>
> Hello Boyang,
>
> On Thu 12-08-21 09:47:30, Boyang Xue wrote:
> > (Adding the author of the commits)
> > Hi Jan,
> >
> > The commit
> >
> > 81414b4dd48 ext4: remove redundant sb checksum recomputation
> >
> > breaks the original reproducer of
> >
> > 4274f516d4bc ext4: recalucate superblock checksum after updating free
> > blocks/inodes
> >
> > I'm wondering is it expected please?
>
> Thanks for report! So for record the problem is not that superblock with
> incorrect checksum would ever get to disk with my patches but the checksum
> will be incorrect in the buffer cache until the moment we start writeout of
> the superblock. And tune2fs accesses the buffer cache and sees the
> incorrect (stale) checksum. It is impossible to fix this problem completely
> (the tune2fs access is fundamentally racy) but yes, I guess returning the
> checksum recalculation back will make the race window small and the cost is
> small. I'll send a patch for this shortly.
>
> Also can you perhaps make this sequence into a fstests testcase for ext4
> filesystem so that we have it covered? Thanks!
>
>                                                                 Honza
>
> > On Thu, Aug 5, 2021 at 10:35 AM Boyang Xue <bxue@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > kernel commit
> > >
> > > 4274f516d4bc ext4: recalucate superblock checksum after updating free
> > > blocks/inodes
> > >
> > > had been reverted by
> > >
> > > 81414b4dd48 ext4: remove redundant sb checksum recomputation
> > >
> > > since kernel-5.11-rc1. As a result, the original reproducer fails again.
> > >
> > > Reproducer:
> > > ```
> > > mkdir mntpt
> > > fallocate -l 256M mntpt.img
> > > mkfs.ext4 -Fq -t ext4 mntpt.img 128M
> > > LPDEV=$(losetup -f --show mntpt.img)
> > > mount "$LPDEV" mntpt
> > > cp /proc/version mntpt/
> > > ./godown mntpt # godown program attached.
> > > umount mntpt
> > > mount "$LPDEV" mntpt
> > > tune2fs -l "$LPDEV"
> > > ```
> > >
> > > tune2fs fails with
> > > ```
> > > tune2fs 1.46.2 (28-Feb-2021)
> > > tune2fs: Superblock checksum does not match superblock while trying to
> > > open /dev/loop0
> > > Couldn't find valid filesystem superblock.
> > > ```
> > >
> > > Tested on e2fsprogs-1.46.2 + kernel-5.14.0-0.rc3.29. I think it's a
> > > regression. If this is the case, can we fix it again please?
> > >
> > > Thanks,
> > > Boyang
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-12 13:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05  2:35 [kernel-5.11 regression] tune2fs fails after shutdown Boyang Xue
2021-08-12  1:47 ` Boyang Xue
2021-08-12 12:47   ` Jan Kara
2021-08-12 13:38     ` Boyang Xue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.