2024-03-27 12:07:40

by Sasha Levin

[permalink] [raw]
Subject: FAILED: Patch "btrfs: do not skip re-registration for the mounted device" failed to apply to 6.8-stable tree

The patch below does not apply to the 6.8-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <[email protected]>.

Thanks,
Sasha

------------------ original commit in Linus's tree ------------------

From d565fffa68560ac540bf3d62cc79719da50d5e7a Mon Sep 17 00:00:00 2001
From: Anand Jain <[email protected]>
Date: Tue, 13 Feb 2024 09:13:56 +0800
Subject: [PATCH] btrfs: do not skip re-registration for the mounted device

There are reports that since version 6.7 update-grub fails to find the
device of the root on systems without initrd and on a single device.

This looks like the device name changed in the output of
/proc/self/mountinfo:

6.5-rc5 working

18 1 0:16 / / rw,noatime - btrfs /dev/sda8 ...

6.7 not working:

17 1 0:15 / / rw,noatime - btrfs /dev/root ...

and "update-grub" shows this error:

/usr/sbin/grub-probe: error: cannot find a device for / (is /dev mounted?)

This looks like it's related to the device name, but grub-probe
recognizes the "/dev/root" path and tries to find the underlying device.
However there's a special case for some filesystems, for btrfs in
particular.

The generic root device detection heuristic is not done and it all
relies on reading the device infos by a btrfs specific ioctl. This ioctl
returns the device name as it was saved at the time of device scan (in
this case it's /dev/root).

The change in 6.7 for temp_fsid to allow several single device
filesystem to exist with the same fsid (and transparently generate a new
UUID at mount time) was to skip caching/registering such devices.

This also skipped mounted device. One step of scanning is to check if
the device name hasn't changed, and if yes then update the cached value.

This broke the grub-probe as it always read the device /dev/root and
couldn't find it in the system. A temporary workaround is to create a
symlink but this does not survive reboot.

The right fix is to allow updating the device path of a mounted
filesystem even if this is a single device one.

In the fix, check if the device's major:minor number matches with the
cached device. If they do, then we can allow the scan to happen so that
device_list_add() can take care of updating the device path. The file
descriptor remains unchanged.

This does not affect the temp_fsid feature, the UUID of the mounted
filesystem remains the same and the matching is based on device major:minor
which is unique per mounted filesystem.

This covers the path when the device (that exists for all mounted
devices) name changes, updating /dev/root to /dev/sdx. Any other single
device with filesystem and is not mounted is still skipped.

Note that if a system is booted and initial mount is done on the
/dev/root device, this will be the cached name of the device. Only after
the command "btrfs device scan" it will change as it triggers the
rename.

The fix was verified by users whose systems were affected.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218353
Link: https://lore.kernel.org/lkml/CAKLYgeJ1tUuqLcsquwuFqjDXPSJpEiokrWK2gisPKDZLs8Y2TQ@mail.gmail.com/
Fixes: bc27d6f0aa0e ("btrfs: scan but don't register device on single device filesystem")
CC: [email protected] # 6.7+
Tested-by: Alex Romosan <[email protected]>
Tested-by: [email protected]
Signed-off-by: Anand Jain <[email protected]>
Reviewed-by: David Sterba <[email protected]>
Signed-off-by: David Sterba <[email protected]>
---
fs/btrfs/volumes.c | 58 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a2d07fa3cfdff..1dc1f1946ae0e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1303,6 +1303,47 @@ int btrfs_forget_devices(dev_t devt)
return ret;
}

+static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
+ const char *path, dev_t devt,
+ bool mount_arg_dev)
+{
+ struct btrfs_fs_devices *fs_devices;
+
+ /*
+ * Do not skip device registration for mounted devices with matching
+ * maj:min but different paths. Booting without initrd relies on
+ * /dev/root initially, later replaced with the actual root device.
+ * A successful scan ensures grub2-probe selects the correct device.
+ */
+ list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+ struct btrfs_device *device;
+
+ mutex_lock(&fs_devices->device_list_mutex);
+
+ if (!fs_devices->opened) {
+ mutex_unlock(&fs_devices->device_list_mutex);
+ continue;
+ }
+
+ list_for_each_entry(device, &fs_devices->devices, dev_list) {
+ if (device->bdev && (device->bdev->bd_dev == devt) &&
+ strcmp(device->name->str, path) != 0) {
+ mutex_unlock(&fs_devices->device_list_mutex);
+
+ /* Do not skip registration. */
+ return false;
+ }
+ }
+ mutex_unlock(&fs_devices->device_list_mutex);
+ }
+
+ if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
+ !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
+ return true;
+
+ return false;
+}
+
/*
* Look for a btrfs signature on a device. This may be called out of the mount path
* and we are not allowed to call set_blocksize during the scan. The superblock
@@ -1320,6 +1361,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
struct btrfs_device *device = NULL;
struct file *bdev_file;
u64 bytenr, bytenr_orig;
+ dev_t devt;
int ret;

lockdep_assert_held(&uuid_mutex);
@@ -1359,19 +1401,13 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
goto error_bdev_put;
}

- if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
- !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)) {
- dev_t devt;
+ devt = file_bdev(bdev_file)->bd_dev;
+ if (btrfs_skip_registration(disk_super, path, devt, mount_arg_dev)) {
+ pr_debug("BTRFS: skip registering single non-seed device %s (%d:%d)\n",
+ path, MAJOR(devt), MINOR(devt));

- ret = lookup_bdev(path, &devt);
- if (ret)
- btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
- path, ret);
- else
- btrfs_free_stale_devices(devt, NULL);
+ btrfs_free_stale_devices(devt, NULL);

- pr_debug("BTRFS: skip registering single non-seed device %s (%d:%d)\n",
- path, MAJOR(devt), MINOR(devt));
device = NULL;
goto free_disk_super;
}
--
2.43.0






2024-03-27 21:47:06

by David Sterba

[permalink] [raw]
Subject: Re: FAILED: Patch "btrfs: do not skip re-registration for the mounted device" failed to apply to 6.8-stable tree

On Wed, Mar 27, 2024 at 08:06:40AM -0400, Sasha Levin wrote:
> The patch below does not apply to the 6.8-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <[email protected]>.

Please use the version below, applies cleanly on 6.7.x and 6.8.x.

---

From: Anand Jain <[email protected]>
Subject: [PATCH] btrfs: do not skip re-registration for the mounted device

[ Upstream commit d565fffa68560ac540bf3d62cc79719da50d5e7a ]

There are reports that since version 6.7 update-grub fails to find the
device of the root on systems without initrd and on a single device.

This looks like the device name changed in the output of
/proc/self/mountinfo:

6.5-rc5 working

18 1 0:16 / / rw,noatime - btrfs /dev/sda8 ...

6.7 not working:

17 1 0:15 / / rw,noatime - btrfs /dev/root ...

and "update-grub" shows this error:

/usr/sbin/grub-probe: error: cannot find a device for / (is /dev mounted?)

This looks like it's related to the device name, but grub-probe
recognizes the "/dev/root" path and tries to find the underlying device.
However there's a special case for some filesystems, for btrfs in
particular.

The generic root device detection heuristic is not done and it all
relies on reading the device infos by a btrfs specific ioctl. This ioctl
returns the device name as it was saved at the time of device scan (in
this case it's /dev/root).

The change in 6.7 for temp_fsid to allow several single device
filesystem to exist with the same fsid (and transparently generate a new
UUID at mount time) was to skip caching/registering such devices.

This also skipped mounted device. One step of scanning is to check if
the device name hasn't changed, and if yes then update the cached value.

This broke the grub-probe as it always read the device /dev/root and
couldn't find it in the system. A temporary workaround is to create a
symlink but this does not survive reboot.

The right fix is to allow updating the device path of a mounted
filesystem even if this is a single device one.

In the fix, check if the device's major:minor number matches with the
cached device. If they do, then we can allow the scan to happen so that
device_list_add() can take care of updating the device path. The file
descriptor remains unchanged.

This does not affect the temp_fsid feature, the UUID of the mounted
filesystem remains the same and the matching is based on device major:minor
which is unique per mounted filesystem.

This covers the path when the device (that exists for all mounted
devices) name changes, updating /dev/root to /dev/sdx. Any other single
device with filesystem and is not mounted is still skipped.

Note that if a system is booted and initial mount is done on the
/dev/root device, this will be the cached name of the device. Only after
the command "btrfs device scan" it will change as it triggers the
rename.

The fix was verified by users whose systems were affected.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218353
Link: https://lore.kernel.org/lkml/CAKLYgeJ1tUuqLcsquwuFqjDXPSJpEiokrWK2gisPKDZLs8Y2TQ@mail.gmail.com/
Fixes: bc27d6f0aa0e ("btrfs: scan but don't register device on single device filesystem")
CC: [email protected] # 6.7+
Tested-by: Alex Romosan <[email protected]>
Tested-by: [email protected]
Signed-off-by: Anand Jain <[email protected]>
Reviewed-by: David Sterba <[email protected]>
Signed-off-by: David Sterba <[email protected]>
---
fs/btrfs/volumes.c | 57 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f627674b37db..6f4dd3ebbf7a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1290,6 +1290,47 @@ int btrfs_forget_devices(dev_t devt)
return ret;
}

+static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
+ const char *path, dev_t devt,
+ bool mount_arg_dev)
+{
+ struct btrfs_fs_devices *fs_devices;
+
+ /*
+ * Do not skip device registration for mounted devices with matching
+ * maj:min but different paths. Booting without initrd relies on
+ * /dev/root initially, later replaced with the actual root device.
+ * A successful scan ensures grub2-probe selects the correct device.
+ */
+ list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+ struct btrfs_device *device;
+
+ mutex_lock(&fs_devices->device_list_mutex);
+
+ if (!fs_devices->opened) {
+ mutex_unlock(&fs_devices->device_list_mutex);
+ continue;
+ }
+
+ list_for_each_entry(device, &fs_devices->devices, dev_list) {
+ if (device->bdev && (device->bdev->bd_dev == devt) &&
+ strcmp(device->name->str, path) != 0) {
+ mutex_unlock(&fs_devices->device_list_mutex);
+
+ /* Do not skip registration. */
+ return false;
+ }
+ }
+ mutex_unlock(&fs_devices->device_list_mutex);
+ }
+
+ if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
+ !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
+ return true;
+
+ return false;
+}
+
/*
* Look for a btrfs signature on a device. This may be called out of the mount path
* and we are not allowed to call set_blocksize during the scan. The superblock
@@ -1346,18 +1387,14 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
goto error_bdev_put;
}

- if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
- !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)) {
- dev_t devt;
+ if (btrfs_skip_registration(disk_super, path, bdev_handle->bdev->bd_dev,
+ mount_arg_dev)) {
+ pr_debug("BTRFS: skip registering single non-seed device %s (%d:%d)\n",
+ path, MAJOR(bdev_handle->bdev->bd_dev),
+ MINOR(bdev_handle->bdev->bd_dev));

- ret = lookup_bdev(path, &devt);
- if (ret)
- btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
- path, ret);
- else
- btrfs_free_stale_devices(devt, NULL);
+ btrfs_free_stale_devices(bdev_handle->bdev->bd_dev, NULL);

- pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
device = NULL;
goto free_disk_super;
}
--
2.44.0


2024-03-28 05:10:07

by Anand Jain

[permalink] [raw]
Subject: Re: FAILED: Patch "btrfs: do not skip re-registration for the mounted device" failed to apply to 6.8-stable tree



On 3/28/24 05:37, David Sterba wrote:
> On Wed, Mar 27, 2024 at 08:06:40AM -0400, Sasha Levin wrote:
>> The patch below does not apply to the 6.8-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <[email protected]>.
>
> Please use the version below, applies cleanly on 6.7.x and 6.8.x.
>

Thank You!
Anand

> ---
>
> From: Anand Jain <[email protected]>
> Subject: [PATCH] btrfs: do not skip re-registration for the mounted device
>
> [ Upstream commit d565fffa68560ac540bf3d62cc79719da50d5e7a ]
>
> There are reports that since version 6.7 update-grub fails to find the
> device of the root on systems without initrd and on a single device.
>
> This looks like the device name changed in the output of
> /proc/self/mountinfo:
>
> 6.5-rc5 working
>
> 18 1 0:16 / / rw,noatime - btrfs /dev/sda8 ...
>
> 6.7 not working:
>
> 17 1 0:15 / / rw,noatime - btrfs /dev/root ...
>
> and "update-grub" shows this error:
>
> /usr/sbin/grub-probe: error: cannot find a device for / (is /dev mounted?)
>
> This looks like it's related to the device name, but grub-probe
> recognizes the "/dev/root" path and tries to find the underlying device.
> However there's a special case for some filesystems, for btrfs in
> particular.
>
> The generic root device detection heuristic is not done and it all
> relies on reading the device infos by a btrfs specific ioctl. This ioctl
> returns the device name as it was saved at the time of device scan (in
> this case it's /dev/root).
>
> The change in 6.7 for temp_fsid to allow several single device
> filesystem to exist with the same fsid (and transparently generate a new
> UUID at mount time) was to skip caching/registering such devices.
>
> This also skipped mounted device. One step of scanning is to check if
> the device name hasn't changed, and if yes then update the cached value.
>
> This broke the grub-probe as it always read the device /dev/root and
> couldn't find it in the system. A temporary workaround is to create a
> symlink but this does not survive reboot.
>
> The right fix is to allow updating the device path of a mounted
> filesystem even if this is a single device one.
>
> In the fix, check if the device's major:minor number matches with the
> cached device. If they do, then we can allow the scan to happen so that
> device_list_add() can take care of updating the device path. The file
> descriptor remains unchanged.
>
> This does not affect the temp_fsid feature, the UUID of the mounted
> filesystem remains the same and the matching is based on device major:minor
> which is unique per mounted filesystem.
>
> This covers the path when the device (that exists for all mounted
> devices) name changes, updating /dev/root to /dev/sdx. Any other single
> device with filesystem and is not mounted is still skipped.
>
> Note that if a system is booted and initial mount is done on the
> /dev/root device, this will be the cached name of the device. Only after
> the command "btrfs device scan" it will change as it triggers the
> rename.
>
> The fix was verified by users whose systems were affected.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218353
> Link: https://lore.kernel.org/lkml/CAKLYgeJ1tUuqLcsquwuFqjDXPSJpEiokrWK2gisPKDZLs8Y2TQ@mail.gmail.com/
> Fixes: bc27d6f0aa0e ("btrfs: scan but don't register device on single device filesystem")
> CC: [email protected] # 6.7+
> Tested-by: Alex Romosan <[email protected]>
> Tested-by: [email protected]
> Signed-off-by: Anand Jain <[email protected]>
> Reviewed-by: David Sterba <[email protected]>
> Signed-off-by: David Sterba <[email protected]>
> ---
> fs/btrfs/volumes.c | 57 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f627674b37db..6f4dd3ebbf7a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1290,6 +1290,47 @@ int btrfs_forget_devices(dev_t devt)
> return ret;
> }
>
> +static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
> + const char *path, dev_t devt,
> + bool mount_arg_dev)
> +{
> + struct btrfs_fs_devices *fs_devices;
> +
> + /*
> + * Do not skip device registration for mounted devices with matching
> + * maj:min but different paths. Booting without initrd relies on
> + * /dev/root initially, later replaced with the actual root device.
> + * A successful scan ensures grub2-probe selects the correct device.
> + */
> + list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> + struct btrfs_device *device;
> +
> + mutex_lock(&fs_devices->device_list_mutex);
> +
> + if (!fs_devices->opened) {
> + mutex_unlock(&fs_devices->device_list_mutex);
> + continue;
> + }
> +
> + list_for_each_entry(device, &fs_devices->devices, dev_list) {
> + if (device->bdev && (device->bdev->bd_dev == devt) &&
> + strcmp(device->name->str, path) != 0) {
> + mutex_unlock(&fs_devices->device_list_mutex);
> +
> + /* Do not skip registration. */
> + return false;
> + }
> + }
> + mutex_unlock(&fs_devices->device_list_mutex);
> + }
> +
> + if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
> + !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
> + return true;
> +
> + return false;
> +}
> +
> /*
> * Look for a btrfs signature on a device. This may be called out of the mount path
> * and we are not allowed to call set_blocksize during the scan. The superblock
> @@ -1346,18 +1387,14 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
> goto error_bdev_put;
> }
>
> - if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
> - !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)) {
> - dev_t devt;
> + if (btrfs_skip_registration(disk_super, path, bdev_handle->bdev->bd_dev,
> + mount_arg_dev)) {
> + pr_debug("BTRFS: skip registering single non-seed device %s (%d:%d)\n",
> + path, MAJOR(bdev_handle->bdev->bd_dev),
> + MINOR(bdev_handle->bdev->bd_dev));
>
> - ret = lookup_bdev(path, &devt);
> - if (ret)
> - btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> - path, ret);
> - else
> - btrfs_free_stale_devices(devt, NULL);
> + btrfs_free_stale_devices(bdev_handle->bdev->bd_dev, NULL);
>
> - pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
> device = NULL;
> goto free_disk_super;
> }

2024-03-29 12:15:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: FAILED: Patch "btrfs: do not skip re-registration for the mounted device" failed to apply to 6.8-stable tree

On Wed, Mar 27, 2024 at 10:37:51PM +0100, David Sterba wrote:
> On Wed, Mar 27, 2024 at 08:06:40AM -0400, Sasha Levin wrote:
> > The patch below does not apply to the 6.8-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <[email protected]>.
>
> Please use the version below, applies cleanly on 6.7.x and 6.8.x.

Now queued up, thanks.

greg k-h