Received-SPF: pass (google.com: domain of linux-kernel+bounces-122415-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <2e3ed2fb-8b68-447e-9781-aa6f32a95c2e@oracle.com>
Date: Thu, 28 Mar 2024 13:07:12 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: FAILED: Patch "btrfs: do not skip re-registration for the mounted
 device" failed to apply to 6.8-stable tree
To: dsterba@suse.cz
Cc: stable@vger.kernel.org, Alex Romosan <aromosan@gmail.com>,
        CHECK_1234543212345@protonmail.com, David Sterba <dsterba@suse.com>,
        linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        Sasha Levin <sashal@kernel.org>
References: <20240327120640.2824671-1-sashal@kernel.org>
 <20240327213751.GW14596@twin.jikos.cz>
Content-Language: en-US
From: Anand Jain <anand.jain@oracle.com>
In-Reply-To: <20240327213751.GW14596@twin.jikos.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?MzdUbzk1ZDBKQWpCci91NGoyR0hHZGo4UHphN1BkMFVkaXd1Z1NjN1VmTjEz?=
 =?utf-8?B?TDcreGFsQngwbDNTY0ZYVG1UM2xzMzdtbExseUtJOGhHYVlrREdLVjlBY05P?=
 =?utf-8?B?K3BhSTNqN1dvU2dTUGUvRDdQNEV4MGdncU1hTGRuZ0lobXp0b0J3NkMzSkpQ?=
 =?utf-8?B?YmdEZCtoS1J3R1k5d09HOXF1d3ZOZ21RWWU1QmNCeElRaEZYWXFGRy80QnFM?=
 =?utf-8?B?Tll6c25zd08vcVR2blhRY3BTSnkrWTJ2T3FPc2pDTCtVK1hWZk9yZW1vYnZS?=
 =?utf-8?B?ekV6NHlCWklHbVBGcjhSYmVNYlRCbzVlYzJoRHBSRitnUTBrSUo2UDFieFpB?=
 =?utf-8?B?Q3NNbldoWUdWK1R0eW5VZnhLNWhjN0ZCbXdXT1kyK0Z4K3o5RE9pSlE1dXVO?=
 =?utf-8?B?RW1SSE0vUXNuMG50NldPYTgvU3ZlSzhJOGRUa0MvelNtODFIQnRhM01MN2Fu?=
 =?utf-8?B?VTBqYUc0SEZZK1pBYXJuVHZSS2p6RXg4eUlrSHU4cHJwZVRBakxXbTV5ZkhQ?=
 =?utf-8?B?eWxRL1hoblpxM2RJdXhaWmJPZXZlcmNjeG85OXI4a09lWWVJUThueFNYQTVj?=
 =?utf-8?B?a0VwTU90SUVWb3RqQmlRWWF5bG9vbnZBdHFPSXN6QWxTbTJOenJUK2taYlNG?=
 =?utf-8?B?K1FucTRLUHlPUkF3UEZ4clc4NEVUeG1FbGsxQi9RU2JCNE82bnE1Y0xKREhY?=
 =?utf-8?B?VjFSanFvUnYrNVhmbkttWDIvaW1Nd0U5S1JPeVcrWExTdGRWM0EvSm5qZlFF?=
 =?utf-8?B?MVZCVGt2aUNxUWsyMmJuRitETC9SWFFWY3R4czhLUFMyczJKclRUdUhQMERj?=
 =?utf-8?B?SVRRY0Q5c1JsV3lVa0JoakR0aFhsS3k0MHdtNkRDMW54NmE4bFNQSG5vendY?=
 =?utf-8?B?QjMwVVQ3YU02TzB3eTNOd3RkYmVDb0lUYURWeXVtQWcwQzMzdWE4SHo4Zkd0?=
 =?utf-8?B?NlNqYTZOa2F6cTVBTXFzSWgyVnJYODRIUWN5TEwwNk1PVjFqUG8zYVUxRWt2?=
 =?utf-8?B?UmhSYWZHcWhoZE5uOWF3UGY0MVl3OE9tS1NuakpMYzVxb255SkJXdUYwck1K?=
 =?utf-8?B?MFlvemFNUEZhZVNmVEZoUjV4bUlUdGlUNmh1bWF4Wk1QSnZUdVpNWkNSendW?=
 =?utf-8?B?dXdPcGJaZUdJUndDalZXNEVjaHY5RUNNMElqU2U2bW54NGpCdkZLeXlqZVpT?=
 =?utf-8?B?ME5Id3lzZlF6MkZIVzA0Mkw4cGt2aXZ5dlJZWUQ4U2pwNnZNazBJSnB4a0pQ?=
 =?utf-8?B?WGR3V01OblhFc015a2s1NTIyM09WR1puTCtNV1lFclhEK0tzYnEzZmw1VXI1?=
 =?utf-8?B?aGU5cm5rb1did1NxY0Z3YjhkNEhYZGY2cVBBQzJCRUxMcVFVMWsxV1I4OHVE?=
 =?utf-8?B?Nmd3Q3ZhOUtHQ0Y1aUVacmNucFFSYS9TN0h0Qy9kQkNqRjZHbS9VaVo0Wjlr?=
 =?utf-8?B?amt1dmNMTXVCczEwTHVMTlRZcHVWTEF2NGNaUERNM3VyZ3MwT0psY2RndERy?=
 =?utf-8?B?ME5HRDQ1ay9aOGMvTkJuSFRTY1B1SUQrSUV2UlUyMDMvL1lna3pGeG1xVnI5?=
 =?utf-8?B?TW00QlpLQXZoSUVaSW5jU2htOUlLOUpuckZaZVV2cUFnbEQ0U1o4SENNN0sy?=
 =?utf-8?B?bFRBUUFZenRYYzBFRVRaRXdVZGk1bjM1VDBRbm1wMldTQWNBcUI4VXJxQVNP?=
 =?utf-8?B?VEJ1TG9LT3hnWW83eHRBdGQveCtvK0JlWEdFMkhWazNGcDROMWVMeEFielVG?=
 =?utf-8?B?OWFDT0FRcE1DTnpWOGFFQkJ2WXZScVFjelJFUjFjZ3RCVWE3a2ltdElDQ2wx?=
 =?utf-8?B?OXkwUFVlL0xSVjZkS000SEVjN1prVmZXOGpLYjhNODV5ZS9UZDBXNXNaSHJM?=
 =?utf-8?B?MFRRZDZOTHUyUjFaMyt3Yk80d0p5WnExM0s0UkRZSkJHQzlmL2l4VHJxYXI3?=
 =?utf-8?B?Qmg1NWlEUTMwOGpyWXJnTXIvMG5JdURLTVRmVkRiMDZuMlhxUlV2c1NyNVY5?=
 =?utf-8?B?Z2pQVGtHWVQrcEdGZXFmNnhOUlF6d1NjSTRuMWhPWmtNT0YvVWZYT09HSWRr?=
 =?utf-8?B?R0FpM2w3SnJLSjg2VERCejVMTy8yMjlxQ0pLeFFKaWNhalFQRWRwRnppOXBN?=
 =?utf-8?B?VGVaZDkvWWRDMzRiTks0NkIvV0FYcS9xSnJSaFhzYW5zSEhRaUY3OFo3MUdF?=
 =?utf-8?B?bHc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	uftHrfC0gDdBZXaHvDSdPgDyYnVlx/44sLlG3SKKHua+zVHXWVzf4ksgWIyaKt7n7xfJhjStJE7kIuh+AV3fN4NVnn+9xjS2JtbBJZi6FOyyKyAbItuaIslHUVrzUfy7xKlZy5Fm8amRceXtPYsZJUQxq1ScL9bruC0aFRlbk0eOrmYIRt3PeBq0F8c4RH6CRurqori57q3Ls+DRx12ozf3HvYkKsd7+Be5vxOcyFCiN/U3GEXZbYapKImlJps3VDzE4d4ZQXlhlr3WN64FIvdKVfn39QPz7TqPxujZUo2nHusvm/FxJMElSpxmTwuAOEfzwRgMl9CsQZ4lZHgijumtwXjFmabmIlm4CS63MXnDOI+KYZs8WxQoSj/4dA0iWLxFGFJBKbMWt1c1SdapZgs+qP0sim65Iz7pJvwHqsFAqEKtSj+nFjPmM+Jms7uMfLF+OQ+vs0ag4y05Ii8cOaYdXLThuqb0bZSUaYJjDr5LLd70v3KAz+vMm0RTEx4cOriG9Rh/kagdDUZ577tDFqLqFZ2+sGIwzdBdS0clqaKimhupzEF4lm4GBZtNleSMApjGHCkSghAVFiC3PFJ6vwSNtTpFxHo8qFdjAIBuABJs=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3663a2da-7d46-45b9-2b29-08dc4ee4f20f
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5706.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Mar 2024 05:07:20.3692
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SSwKcpDAE3HohI1FIb9gage+/WOLJ4fUmvXG5/6/yxXZhK5aDoAnsTknU8UGCCTc2fzc3i8k2/oOENidlmE8Rg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR10MB7369
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-28_04,2024-03-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 phishscore=0 suspectscore=0
 mlxlogscore=999 bulkscore=0 spamscore=0 adultscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2403210000
 definitions=main-2403280029
X-Proofpoint-GUID: 3JGomjIwh2Gjc-bdDU3z0eMsW3K_Fw3H
X-Proofpoint-ORIG-GUID: 3JGomjIwh2Gjc-bdDU3z0eMsW3K_Fw3H


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@vger.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@oracle.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@vger.kernel.org # 6.7+
> Tested-by: Alex Romosan <aromosan@gmail.com>
> Tested-by: CHECK_1234543212345@protonmail.com
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.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;
>   	}