Date: Thu, 28 Mar 2024 13:07:12 +0800
From: Anand Jain <anand.jain@...cle.com>
To: dsterba@...e.cz
Cc: stable@...r.kernel.org, Alex Romosan <aromosan@...il.com>,
        CHECK_1234543212345@...tonmail.com, David Sterba <dsterba@...e.com>,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sasha Levin <sashal@...nel.org>
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 <stable@...r.kernel.org>.
> 
> Please use the version below, applies cleanly on 6.7.x and 6.8.x.
> 

  Thank You!
Anand

> ---
> 
> From: Anand Jain <anand.jain@...cle.com>
> 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: stable@...r.kernel.org # 6.7+
> Tested-by: Alex Romosan <aromosan@...il.com>
> Tested-by: CHECK_1234543212345@...tonmail.com
> Signed-off-by: Anand Jain <anand.jain@...cle.com>
> Reviewed-by: David Sterba <dsterba@...e.com>
> Signed-off-by: David Sterba <dsterba@...e.com>
> ---
>   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;
>   	}

Powered by blists - more mailing lists