Re: [PATCH v5] btrfs: do not skip re-registration for the mounted device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



confirming that update-grub works with v5 of the patch (applied
against current Linus tree HEAD). these are the relevant entries in
the log:

Btrfs loaded, debug=on, zoned=no, fsverity=no
BTRFS: device fsid 695aa7ac-862a-4de3-ae59-c96f784600a0 devid 1
transid 2037166 /dev/root (259:3) scanned by swapper/0 (1)
BTRFS info (device nvme0n1p3): first mount of filesystem
695aa7ac-862a-4de3-ae59-c96f784600a0
BTRFS info (device nvme0n1p3): using crc32c (crc32c-generic) checksum algorithm
BTRFS info (device nvme0n1p3): using free-space-tree
VFS: Mounted root (btrfs filesystem) readonly on device 0:20.
BTRFS info: devid 1 device path /dev/root changed to /dev/nvme0n1p3
scanned by (udev-worker) (278)


On Mon, Mar 18, 2024 at 5:47 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote:
>
> 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.
>
> CC: stable@xxxxxxxxxxxxxxx # 6.7+
> Fixes: bc27d6f0aa0e ("btrfs: scan but don't register device on single device filesystem")
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218353
> Link: https://lore.kernel.org/lkml/CAKLYgeJ1tUuqLcsquwuFqjDXPSJpEiokrWK2gisPKDZLs8Y2TQ@xxxxxxxxxxxxxx/
> Tested-by: Alex Romosan <aromosan@xxxxxxxxx>
> Tested-by: CHECK_1234543212345@xxxxxxxxxxxxxx
> Reviewed-by: David Sterba <dsterba@xxxxxxxx>
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> ---
> v5:
> Fix the linux-next build failure reported here:
>   https://lore.kernel.org/all/20240318091755.1d0f696f@xxxxxxxxxxxxxxxx/
> As the Linux-next branch no longer has the this commit,
> I've sent out the entire patch again.
>
> v4: (based on mainline master)
> I removed CC: stable@xxxxxxxxxxxxxxx # 6.7+ as this is still in the RFC stage.
> I need this patch verified by the bug filer.
> Use devt from bdev->bd_dev
> Rebased on mainline kernel.org master branch
>
> v3:
> https://lore.kernel.org/linux-btrfs/e2add8d54fbbd813305ba014c11d21d297ad87d0.1709782041.git.anand.jain@xxxxxxxxxx/T/#u
>
>  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 a2d07fa3cfdf..813c1c66b2db 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 update-grub 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->devt == devt) &&
> +                           strcmp(device->name->str, path)) {
> +                               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.38.1
>





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux