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

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

[Anand Jain]

We skip device registration for a single device. However, we do not do
that if the device is already mounted, as it might be coming in again
for scanning a different path.

This patch is lightly tested; for verification if it fixes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
I still have some unknowns about the problem. Pls test if this fixes
the problem.

 fs/btrfs/volumes.c | 44 ++++++++++++++++++++++++++++++++++----------
 fs/btrfs/volumes.h |  1 -
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 474ab7ed65ea..192c540a650c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
 	return ret;
 }
 
+static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
+				    dev_t devt, bool mount_arg_dev)
+{
+	struct btrfs_fs_devices *fs_devices;
+
+	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
+		struct btrfs_device *device;
+
+		mutex_lock(&fs_devices->device_list_mutex);
+		list_for_each_entry(device, &fs_devices->devices, dev_list) {
+			if (device->devt == devt) {
+				mutex_unlock(&fs_devices->device_list_mutex);
+				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
@@ -1316,6 +1341,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
 	struct btrfs_device *device = NULL;
 	struct bdev_handle *bdev_handle;
 	u64 bytenr, bytenr_orig;
+	dev_t devt = 0;
 	int ret;
 
 	lockdep_assert_held(&uuid_mutex);
@@ -1355,18 +1381,16 @@ 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;
+	ret = lookup_bdev(path, &devt);
+	if (ret)
+		btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
+			   path, ret);
 
-		ret = lookup_bdev(path, &devt);
-		if (ret)
-			btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
-				   path, ret);
-		else
+	if (btrfs_skip_registration(disk_super, devt, mount_arg_dev)) {
+		pr_debug("BTRFS: skip registering single non-seed device %s\n",
+			  path);
+		if (devt)
 			btrfs_free_stale_devices(devt, NULL);
-
-		pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
 		device = NULL;
 		goto free_disk_super;
 	}
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 53f87f398da7..4ed281f0d1bd 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -792,5 +792,4 @@ bool btrfs_repair_one_zone(struct btrfs_fs_info *fs_info, u64 logical);
 
 bool btrfs_pinned_by_swapfile(struct btrfs_fs_info *fs_info, void *ptr);
 u8 *btrfs_sb_fsid_ptr(struct btrfs_super_block *sb);
-
 #endif
-- 
2.39.3

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

[David Sterba]

On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> We skip device registration for a single device. However, we do not do
> that if the device is already mounted, as it might be coming in again
> for scanning a different path.
> 
> This patch is lightly tested; for verification if it fixes.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> I still have some unknowns about the problem. Pls test if this fixes
> the problem.
> 
>  fs/btrfs/volumes.c | 44 ++++++++++++++++++++++++++++++++++----------
>  fs/btrfs/volumes.h |  1 -
>  2 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 474ab7ed65ea..192c540a650c 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>  	return ret;
>  }
>  
> +static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
> +				    dev_t devt, bool mount_arg_dev)
> +{
> +	struct btrfs_fs_devices *fs_devices;
> +
> +	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> +		struct btrfs_device *device;
> +
> +		mutex_lock(&fs_devices->device_list_mutex);
> +		list_for_each_entry(device, &fs_devices->devices, dev_list) {
> +			if (device->devt == devt) {
> +				mutex_unlock(&fs_devices->device_list_mutex);
> +				return false;
> +			}
> +		}
> +		mutex_unlock(&fs_devices->device_list_mutex);

This is locking and unlocking again before going to device_list_add, so
if something changes regarding the registered device then it's not up to
date.


> +	}
> +
> +	if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
> +	    !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
> +		return true;

The way I implemented it is to check the above conditions as a
prerequisite but leave the heavy work for device_list_add that does all
the uuid and device list locking and we are quite sure it survives all
the races between scanning and mounts.

> +
> +	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
> @@ -1316,6 +1341,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>  	struct btrfs_device *device = NULL;
>  	struct bdev_handle *bdev_handle;
>  	u64 bytenr, bytenr_orig;
> +	dev_t devt = 0;
>  	int ret;
>  
>  	lockdep_assert_held(&uuid_mutex);
> @@ -1355,18 +1381,16 @@ 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;
> +	ret = lookup_bdev(path, &devt);
> +	if (ret)
> +		btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> +			   path, ret);
>  
> -		ret = lookup_bdev(path, &devt);

Do we actually need this check? It was added with the patch skipping the
registration, so it's validating the block device but how can we pass
something that is not a valid block device?

Besides there's a call to bdev_open_by_path() that in turn does the
lookup_bdev so checking it here is redundant. It's not related to the
fix itself but I deleted it in my fix.

> -		if (ret)
> -			btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> -				   path, ret);
> -		else
> +	if (btrfs_skip_registration(disk_super, devt, mount_arg_dev)) {
> +		pr_debug("BTRFS: skip registering single non-seed device %s\n",
> +			  path);
> +		if (devt)
>  			btrfs_free_stale_devices(devt, NULL);
> -
> -		pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
>  		device = NULL;
>  		goto free_disk_super;
>  	}

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

[Anand Jain]

On 2/5/24 18:27, David Sterba wrote:
> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>> We skip device registration for a single device. However, we do not do
>> that if the device is already mounted, as it might be coming in again
>> for scanning a different path.
>>
>> This patch is lightly tested; for verification if it fixes.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> I still have some unknowns about the problem. Pls test if this fixes
>> the problem.
>>
>>   fs/btrfs/volumes.c | 44 ++++++++++++++++++++++++++++++++++----------
>>   fs/btrfs/volumes.h |  1 -
>>   2 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 474ab7ed65ea..192c540a650c 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>>   	return ret;
>>   }
>>   
>> +static bool btrfs_skip_registration(struct btrfs_super_block *disk_super,
>> +				    dev_t devt, bool mount_arg_dev)
>> +{
>> +	struct btrfs_fs_devices *fs_devices;
>> +
>> +	list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>> +		struct btrfs_device *device;
>> +
>> +		mutex_lock(&fs_devices->device_list_mutex);
>> +		list_for_each_entry(device, &fs_devices->devices, dev_list) {
>> +			if (device->devt == devt) {
>> +				mutex_unlock(&fs_devices->device_list_mutex);
>> +				return false;
>> +			}
>> +		}
>> +		mutex_unlock(&fs_devices->device_list_mutex);
> 
> This is locking and unlocking again before going to device_list_add, so
> if something changes regarding the registered device then it's not up to
> date.
> 

Right. A race might happen, but it is not an issue. At worst, there
will be a stale device in the cache, which gets removed or re-used
in the next mkfs or mount of the same device.

However, this is a rough cut that we need to fix. I am reviewing
your approach as well. I'm fine with any fix.


> 
>> +	}
>> +
>> +	if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
>> +	    !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
>> +		return true;
> 
> The way I implemented it is to check the above conditions as a
> prerequisite but leave the heavy work for device_list_add that does all
> the uuid and device list locking and we are quite sure it survives all
> the races between scanning and mounts.
> 

Hm. But isn't that the bug we need to fix? That we skipped the device
scan thread that wanted to replace the device path from /dev/root to
/dev/sdx?

And we skipped, because it was not a mount thread
(%mount_arg_dev=false), and the device is already mounted and the
devt will match?

So my fix also checked if devt is a match, then allow it to scan
(so that the device path can be updated, such as /dev/root to /dev/sdx).

To confirm the bug, I asked for the debug kernel messages, I don't
this we got it. Also, the existing kernel log shows no such issue.


>> +
>> +	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
>> @@ -1316,6 +1341,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>>   	struct btrfs_device *device = NULL;
>>   	struct bdev_handle *bdev_handle;
>>   	u64 bytenr, bytenr_orig;
>> +	dev_t devt = 0;
>>   	int ret;
>>   
>>   	lockdep_assert_held(&uuid_mutex);
>> @@ -1355,18 +1381,16 @@ 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;
>> +	ret = lookup_bdev(path, &devt);
>> +	if (ret)
>> +		btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>> +			   path, ret);
>>   
>> -		ret = lookup_bdev(path, &devt);
> 
> Do we actually need this check? It was added with the patch skipping the
> registration, so it's validating the block device but how can we pass
> something that is not a valid block device?
> 

Do you mean to check if the lookup_bdev() is successful? Hm. It should
be okay not to check, but we do that consistently in other places.

> Besides there's a call to bdev_open_by_path() that in turn does the
> lookup_bdev so checking it here is redundant. It's not related to the
> fix itself but I deleted it in my fix.
> 

Oh no. We need %devt to be set because:

It will match if that device is already mounted/scanned.
It will also free stale entries.

Thx, Anand

>> -		if (ret)
>> -			btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>> -				   path, ret);
>> -		else
>> +	if (btrfs_skip_registration(disk_super, devt, mount_arg_dev)) {
>> +		pr_debug("BTRFS: skip registering single non-seed device %s\n",
>> +			  path);
>> +		if (devt)
>>   			btrfs_free_stale_devices(devt, NULL);
>> -
>> -		pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
>>   		device = NULL;
>>   		goto free_disk_super;
>>   	}

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

[Anand Jain]

On 2/7/24 08:08, Anand Jain wrote:
> 
> 
> 
> On 2/5/24 18:27, David Sterba wrote:
>> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>>> We skip device registration for a single device. However, we do not do
>>> that if the device is already mounted, as it might be coming in again
>>> for scanning a different path.
>>>
>>> This patch is lightly tested; for verification if it fixes.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>> I still have some unknowns about the problem. Pls test if this fixes
>>> the problem.

Successfully tested with fstests (-g volume) and temp-fsid test cases.

Can someone verify if this patch fixes the problem? Also, when problem
occurs please provide kernel messages with Btrfs debugging support
option compiled in.

Thanks, Anand


>>>
>>>   fs/btrfs/volumes.c | 44 ++++++++++++++++++++++++++++++++++----------
>>>   fs/btrfs/volumes.h |  1 -
>>>   2 files changed, 34 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 474ab7ed65ea..192c540a650c 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>>>       return ret;
>>>   }
>>> +static bool btrfs_skip_registration(struct btrfs_super_block 
>>> *disk_super,
>>> +                    dev_t devt, bool mount_arg_dev)
>>> +{
>>> +    struct btrfs_fs_devices *fs_devices;
>>> +
>>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>>> +        struct btrfs_device *device;
>>> +
>>> +        mutex_lock(&fs_devices->device_list_mutex);
>>> +        list_for_each_entry(device, &fs_devices->devices, dev_list) {
>>> +            if (device->devt == devt) {
>>> +                mutex_unlock(&fs_devices->device_list_mutex);
>>> +                return false;
>>> +            }
>>> +        }
>>> +        mutex_unlock(&fs_devices->device_list_mutex);
>>
>> This is locking and unlocking again before going to device_list_add, so
>> if something changes regarding the registered device then it's not up to
>> date.
>>
> 

We are in the uuid_mutex, a potentially racing thread will have to
acquire this mutex to delete from the list. So there can't a race.



> Right. A race might happen, but it is not an issue. At worst, there
> will be a stale device in the cache, which gets removed or re-used
> in the next mkfs or mount of the same device.
> 
> However, this is a rough cut that we need to fix. I am reviewing
> your approach as well. I'm fine with any fix.
> 
> 
>>
>>> +    }
>>> +
>>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super) == 1 &&
>>> +        !(btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING))
>>> +        return true;
>>
>> The way I implemented it is to check the above conditions as a
>> prerequisite but leave the heavy work for device_list_add that does all
>> the uuid and device list locking and we are quite sure it survives all
>> the races between scanning and mounts.
>>
> 
> Hm. But isn't that the bug we need to fix? That we skipped the device
> scan thread that wanted to replace the device path from /dev/root to
> /dev/sdx?
> 
> And we skipped, because it was not a mount thread
> (%mount_arg_dev=false), and the device is already mounted and the
> devt will match?
> 
> So my fix also checked if devt is a match, then allow it to scan
> (so that the device path can be updated, such as /dev/root to /dev/sdx).
> 
> To confirm the bug, I asked for the debug kernel messages, I don't
> this we got it. Also, the existing kernel log shows no such issue.
> 
> 
>>> +
>>> +    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
>>> @@ -1316,6 +1341,7 @@ struct btrfs_device 
>>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>>>       struct btrfs_device *device = NULL;
>>>       struct bdev_handle *bdev_handle;
>>>       u64 bytenr, bytenr_orig;
>>> +    dev_t devt = 0;
>>>       int ret;
>>>       lockdep_assert_held(&uuid_mutex);
>>> @@ -1355,18 +1381,16 @@ 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;
>>> +    ret = lookup_bdev(path, &devt);
>>> +    if (ret)
>>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>> +               path, ret);
>>> -        ret = lookup_bdev(path, &devt);
>>
>> Do we actually need this check? It was added with the patch skipping the
>> registration, so it's validating the block device but how can we pass
>> something that is not a valid block device?
>>
> 
> Do you mean to check if the lookup_bdev() is successful? Hm. It should
> be okay not to check, but we do that consistently in other places.
> 
>> Besides there's a call to bdev_open_by_path() that in turn does the
>> lookup_bdev so checking it here is redundant. It's not related to the
>> fix itself but I deleted it in my fix.
>>
> 
> Oh no. We need %devt to be set because:
> 
> It will match if that device is already mounted/scanned.
> It will also free stale entries.
> 
> Thx, Anand
> 
>>> -        if (ret)
>>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>> -                   path, ret);
>>> -        else
>>> +    if (btrfs_skip_registration(disk_super, devt, mount_arg_dev)) {
>>> +        pr_debug("BTRFS: skip registering single non-seed device %s\n",
>>> +              path);
>>> +        if (devt)
>>>               btrfs_free_stale_devices(devt, NULL);
>>> -
>>> -        pr_debug("BTRFS: skip registering single non-seed device 
>>> %s\n", path);
>>>           device = NULL;
>>>           goto free_disk_super;
>>>       }

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

[Anand Jain]

Alex,

Please provide the kernel boot messages with debugging enabled but
no fixes applied. Kindly collect these messages after reproducing
the problem.

We've found issues with the previous fix. Please test this
new patch, as it may address the bug. Keep debugging enabled
during testing.


Thanks ,Anand


On 2/7/24 23:48, Alex Romosan wrote:
> Which version of the patch are we talking about? Let me know and I’ll 
> try it with debugging on. I tried David’s patch and it seemed to work (I 
> just booted into that kernel and ran update-grub) but I’ll try something 
> else…
> 
> On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com 
> <mailto:anand.jain@oracle.com>> wrote:
> 
> 
> 
>     On 2/7/24 08:08, Anand Jain wrote:
>      >
>      >
>      >
>      > On 2/5/24 18:27, David Sterba wrote:
>      >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>      >>> We skip device registration for a single device. However, we do
>     not do
>      >>> that if the device is already mounted, as it might be coming in
>     again
>      >>> for scanning a different path.
>      >>>
>      >>> This patch is lightly tested; for verification if it fixes.
>      >>>
>      >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
>     <mailto:anand.jain@oracle.com>>
>      >>> ---
>      >>> I still have some unknowns about the problem. Pls test if this
>     fixes
>      >>> the problem.
> 
>     Successfully tested with fstests (-g volume) and temp-fsid test cases.
> 
>     Can someone verify if this patch fixes the problem? Also, when problem
>     occurs please provide kernel messages with Btrfs debugging support
>     option compiled in.
> 
>     Thanks, Anand
> 
> 
>      >>>
>      >>>   fs/btrfs/volumes.c | 44
>     ++++++++++++++++++++++++++++++++++----------
>      >>>   fs/btrfs/volumes.h |  1 -
>      >>>   2 files changed, 34 insertions(+), 11 deletions(-)
>      >>>
>      >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>      >>> index 474ab7ed65ea..192c540a650c 100644
>      >>> --- a/fs/btrfs/volumes.c
>      >>> +++ b/fs/btrfs/volumes.c
>      >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>      >>>       return ret;
>      >>>   }
>      >>> +static bool btrfs_skip_registration(struct btrfs_super_block
>      >>> *disk_super,
>      >>> +                    dev_t devt, bool mount_arg_dev)
>      >>> +{
>      >>> +    struct btrfs_fs_devices *fs_devices;
>      >>> +
>      >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>      >>> +        struct btrfs_device *device;
>      >>> +
>      >>> +        mutex_lock(&fs_devices->device_list_mutex);
>      >>> +        list_for_each_entry(device, &fs_devices->devices,
>     dev_list) {
>      >>> +            if (device->devt == devt) {
>      >>> +                mutex_unlock(&fs_devices->device_list_mutex);
>      >>> +                return false;
>      >>> +            }
>      >>> +        }
>      >>> +        mutex_unlock(&fs_devices->device_list_mutex);
>      >>
>      >> This is locking and unlocking again before going to
>     device_list_add, so
>      >> if something changes regarding the registered device then it's
>     not up to
>      >> date.
>      >>
>      >
> 
>     We are in the uuid_mutex, a potentially racing thread will have to
>     acquire this mutex to delete from the list. So there can't a race.
> 
> 
> 
>      > Right. A race might happen, but it is not an issue. At worst, there
>      > will be a stale device in the cache, which gets removed or re-used
>      > in the next mkfs or mount of the same device.
>      >
>      > However, this is a rough cut that we need to fix. I am reviewing
>      > your approach as well. I'm fine with any fix.
>      >
>      >
>      >>
>      >>> +    }
>      >>> +
>      >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
>     == 1 &&
>      >>> +        !(btrfs_super_flags(disk_super) &
>     BTRFS_SUPER_FLAG_SEEDING))
>      >>> +        return true;
>      >>
>      >> The way I implemented it is to check the above conditions as a
>      >> prerequisite but leave the heavy work for device_list_add that
>     does all
>      >> the uuid and device list locking and we are quite sure it
>     survives all
>      >> the races between scanning and mounts.
>      >>
>      >
>      > Hm. But isn't that the bug we need to fix? That we skipped the device
>      > scan thread that wanted to replace the device path from /dev/root to
>      > /dev/sdx?
>      >
>      > And we skipped, because it was not a mount thread
>      > (%mount_arg_dev=false), and the device is already mounted and the
>      > devt will match?
>      >
>      > So my fix also checked if devt is a match, then allow it to scan
>      > (so that the device path can be updated, such as /dev/root to
>     /dev/sdx).
>      >
>      > To confirm the bug, I asked for the debug kernel messages, I don't
>      > this we got it. Also, the existing kernel log shows no such issue.
>      >
>      >
>      >>> +
>      >>> +    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
>      >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
>      >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>      >>>       struct btrfs_device *device = NULL;
>      >>>       struct bdev_handle *bdev_handle;
>      >>>       u64 bytenr, bytenr_orig;
>      >>> +    dev_t devt = 0;
>      >>>       int ret;
>      >>>       lockdep_assert_held(&uuid_mutex);
>      >>> @@ -1355,18 +1381,16 @@ 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;
>      >>> +    ret = lookup_bdev(path, &devt);
>      >>> +    if (ret)
>      >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>      >>> +               path, ret);
>      >>> -        ret = lookup_bdev(path, &devt);
>      >>
>      >> Do we actually need this check? It was added with the patch
>     skipping the
>      >> registration, so it's validating the block device but how can we
>     pass
>      >> something that is not a valid block device?
>      >>
>      >
>      > Do you mean to check if the lookup_bdev() is successful? Hm. It
>     should
>      > be okay not to check, but we do that consistently in other places.
>      >
>      >> Besides there's a call to bdev_open_by_path() that in turn does the
>      >> lookup_bdev so checking it here is redundant. It's not related
>     to the
>      >> fix itself but I deleted it in my fix.
>      >>
>      >
>      > Oh no. We need %devt to be set because:
>      >
>      > It will match if that device is already mounted/scanned.
>      > It will also free stale entries.
>      >
>      > Thx, Anand
>      >
>      >>> -        if (ret)
>      >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>      >>> -                   path, ret);
>      >>> -        else
>      >>> +    if (btrfs_skip_registration(disk_super, devt,
>     mount_arg_dev)) {
>      >>> +        pr_debug("BTRFS: skip registering single non-seed
>     device %s\n",
>      >>> +              path);
>      >>> +        if (devt)
>      >>>               btrfs_free_stale_devices(devt, NULL);
>      >>> -
>      >>> -        pr_debug("BTRFS: skip registering single non-seed device
>      >>> %s\n", path);
>      >>>           device = NULL;
>      >>>           goto free_disk_super;
>      >>>       }
> 

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

[Alex Romosan]

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

i'm attaching my boot log with 6.8.0-rc3 no fixes and btrfs debug
enabled (i assume this is what you wanted). update-grub doesn't work.
there was no patch in your last message. do you want me to try the
patch you sent on monday? confused

On Thu, Feb 8, 2024 at 3:23 AM Anand Jain <anand.jain@oracle.com> wrote:
>
>
> Alex,
>
> Please provide the kernel boot messages with debugging enabled but
> no fixes applied. Kindly collect these messages after reproducing
> the problem.
>
> We've found issues with the previous fix. Please test this
> new patch, as it may address the bug. Keep debugging enabled
> during testing.
>
>
> Thanks ,Anand
>
>
> On 2/7/24 23:48, Alex Romosan wrote:
> > Which version of the patch are we talking about? Let me know and I’ll
> > try it with debugging on. I tried David’s patch and it seemed to work (I
> > just booted into that kernel and ran update-grub) but I’ll try something
> > else…
> >
> > On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com
> > <mailto:anand.jain@oracle.com>> wrote:
> >
> >
> >
> >     On 2/7/24 08:08, Anand Jain wrote:
> >      >
> >      >
> >      >
> >      > On 2/5/24 18:27, David Sterba wrote:
> >      >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> >      >>> We skip device registration for a single device. However, we do
> >     not do
> >      >>> that if the device is already mounted, as it might be coming in
> >     again
> >      >>> for scanning a different path.
> >      >>>
> >      >>> This patch is lightly tested; for verification if it fixes.
> >      >>>
> >      >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
> >     <mailto:anand.jain@oracle.com>>
> >      >>> ---
> >      >>> I still have some unknowns about the problem. Pls test if this
> >     fixes
> >      >>> the problem.
> >
> >     Successfully tested with fstests (-g volume) and temp-fsid test cases.
> >
> >     Can someone verify if this patch fixes the problem? Also, when problem
> >     occurs please provide kernel messages with Btrfs debugging support
> >     option compiled in.
> >
> >     Thanks, Anand
> >
> >
> >      >>>
> >      >>>   fs/btrfs/volumes.c | 44
> >     ++++++++++++++++++++++++++++++++++----------
> >      >>>   fs/btrfs/volumes.h |  1 -
> >      >>>   2 files changed, 34 insertions(+), 11 deletions(-)
> >      >>>
> >      >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> >      >>> index 474ab7ed65ea..192c540a650c 100644
> >      >>> --- a/fs/btrfs/volumes.c
> >      >>> +++ b/fs/btrfs/volumes.c
> >      >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
> >      >>>       return ret;
> >      >>>   }
> >      >>> +static bool btrfs_skip_registration(struct btrfs_super_block
> >      >>> *disk_super,
> >      >>> +                    dev_t devt, bool mount_arg_dev)
> >      >>> +{
> >      >>> +    struct btrfs_fs_devices *fs_devices;
> >      >>> +
> >      >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> >      >>> +        struct btrfs_device *device;
> >      >>> +
> >      >>> +        mutex_lock(&fs_devices->device_list_mutex);
> >      >>> +        list_for_each_entry(device, &fs_devices->devices,
> >     dev_list) {
> >      >>> +            if (device->devt == devt) {
> >      >>> +                mutex_unlock(&fs_devices->device_list_mutex);
> >      >>> +                return false;
> >      >>> +            }
> >      >>> +        }
> >      >>> +        mutex_unlock(&fs_devices->device_list_mutex);
> >      >>
> >      >> This is locking and unlocking again before going to
> >     device_list_add, so
> >      >> if something changes regarding the registered device then it's
> >     not up to
> >      >> date.
> >      >>
> >      >
> >
> >     We are in the uuid_mutex, a potentially racing thread will have to
> >     acquire this mutex to delete from the list. So there can't a race.
> >
> >
> >
> >      > Right. A race might happen, but it is not an issue. At worst, there
> >      > will be a stale device in the cache, which gets removed or re-used
> >      > in the next mkfs or mount of the same device.
> >      >
> >      > However, this is a rough cut that we need to fix. I am reviewing
> >      > your approach as well. I'm fine with any fix.
> >      >
> >      >
> >      >>
> >      >>> +    }
> >      >>> +
> >      >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
> >     == 1 &&
> >      >>> +        !(btrfs_super_flags(disk_super) &
> >     BTRFS_SUPER_FLAG_SEEDING))
> >      >>> +        return true;
> >      >>
> >      >> The way I implemented it is to check the above conditions as a
> >      >> prerequisite but leave the heavy work for device_list_add that
> >     does all
> >      >> the uuid and device list locking and we are quite sure it
> >     survives all
> >      >> the races between scanning and mounts.
> >      >>
> >      >
> >      > Hm. But isn't that the bug we need to fix? That we skipped the device
> >      > scan thread that wanted to replace the device path from /dev/root to
> >      > /dev/sdx?
> >      >
> >      > And we skipped, because it was not a mount thread
> >      > (%mount_arg_dev=false), and the device is already mounted and the
> >      > devt will match?
> >      >
> >      > So my fix also checked if devt is a match, then allow it to scan
> >      > (so that the device path can be updated, such as /dev/root to
> >     /dev/sdx).
> >      >
> >      > To confirm the bug, I asked for the debug kernel messages, I don't
> >      > this we got it. Also, the existing kernel log shows no such issue.
> >      >
> >      >
> >      >>> +
> >      >>> +    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
> >      >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
> >      >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
> >      >>>       struct btrfs_device *device = NULL;
> >      >>>       struct bdev_handle *bdev_handle;
> >      >>>       u64 bytenr, bytenr_orig;
> >      >>> +    dev_t devt = 0;
> >      >>>       int ret;
> >      >>>       lockdep_assert_held(&uuid_mutex);
> >      >>> @@ -1355,18 +1381,16 @@ 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;
> >      >>> +    ret = lookup_bdev(path, &devt);
> >      >>> +    if (ret)
> >      >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >      >>> +               path, ret);
> >      >>> -        ret = lookup_bdev(path, &devt);
> >      >>
> >      >> Do we actually need this check? It was added with the patch
> >     skipping the
> >      >> registration, so it's validating the block device but how can we
> >     pass
> >      >> something that is not a valid block device?
> >      >>
> >      >
> >      > Do you mean to check if the lookup_bdev() is successful? Hm. It
> >     should
> >      > be okay not to check, but we do that consistently in other places.
> >      >
> >      >> Besides there's a call to bdev_open_by_path() that in turn does the
> >      >> lookup_bdev so checking it here is redundant. It's not related
> >     to the
> >      >> fix itself but I deleted it in my fix.
> >      >>
> >      >
> >      > Oh no. We need %devt to be set because:
> >      >
> >      > It will match if that device is already mounted/scanned.
> >      > It will also free stale entries.
> >      >
> >      > Thx, Anand
> >      >
> >      >>> -        if (ret)
> >      >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >      >>> -                   path, ret);
> >      >>> -        else
> >      >>> +    if (btrfs_skip_registration(disk_super, devt,
> >     mount_arg_dev)) {
> >      >>> +        pr_debug("BTRFS: skip registering single non-seed
> >     device %s\n",
> >      >>> +              path);
> >      >>> +        if (devt)
> >      >>>               btrfs_free_stale_devices(devt, NULL);
> >      >>> -
> >      >>> -        pr_debug("BTRFS: skip registering single non-seed device
> >      >>> %s\n", path);
> >      >>>           device = NULL;
> >      >>>           goto free_disk_super;
> >      >>>       }
> >

[-- Attachment #2: log.6.8.0-rc3.gz --]
[-- Type: application/gzip, Size: 32585 bytes --]

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

[Anand Jain]

Thanks for the kernel messages with debug enabled.

I don't see the message to skip scannaing for
the mounted device. So it's not what we thought
was the issue.

   pr_debug("BTRFS: skip registering single non-seed device %s\n", path);


Based on the assumption above, I have a fix below,
but I doubt its effectiveness.

 
https://patchwork.kernel.org/project/linux-btrfs/patch/8dd1990114aabb775d4631969f1beabeadaac5b7.1707132247.git.anand.jain@oracle.com/

-Anand


On 2/8/24 18:05, Alex Romosan wrote:
> i'm attaching my boot log with 6.8.0-rc3 no fixes and btrfs debug
> enabled (i assume this is what you wanted). update-grub doesn't work.
> there was no patch in your last message. do you want me to try the
> patch you sent on monday? confused
> 
> On Thu, Feb 8, 2024 at 3:23 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>>
>> Alex,
>>
>> Please provide the kernel boot messages with debugging enabled but
>> no fixes applied. Kindly collect these messages after reproducing
>> the problem.
>>
>> We've found issues with the previous fix. Please test this
>> new patch, as it may address the bug. Keep debugging enabled
>> during testing.
>>
>>
>> Thanks ,Anand
>>
>>
>> On 2/7/24 23:48, Alex Romosan wrote:
>>> Which version of the patch are we talking about? Let me know and I’ll
>>> try it with debugging on. I tried David’s patch and it seemed to work (I
>>> just booted into that kernel and ran update-grub) but I’ll try something
>>> else…
>>>
>>> On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com
>>> <mailto:anand.jain@oracle.com>> wrote:
>>>
>>>
>>>
>>>      On 2/7/24 08:08, Anand Jain wrote:
>>>       >
>>>       >
>>>       >
>>>       > On 2/5/24 18:27, David Sterba wrote:
>>>       >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>>>       >>> We skip device registration for a single device. However, we do
>>>      not do
>>>       >>> that if the device is already mounted, as it might be coming in
>>>      again
>>>       >>> for scanning a different path.
>>>       >>>
>>>       >>> This patch is lightly tested; for verification if it fixes.
>>>       >>>
>>>       >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
>>>      <mailto:anand.jain@oracle.com>>
>>>       >>> ---
>>>       >>> I still have some unknowns about the problem. Pls test if this
>>>      fixes
>>>       >>> the problem.
>>>
>>>      Successfully tested with fstests (-g volume) and temp-fsid test cases.
>>>
>>>      Can someone verify if this patch fixes the problem? Also, when problem
>>>      occurs please provide kernel messages with Btrfs debugging support
>>>      option compiled in.
>>>
>>>      Thanks, Anand
>>>
>>>
>>>       >>>
>>>       >>>   fs/btrfs/volumes.c | 44
>>>      ++++++++++++++++++++++++++++++++++----------
>>>       >>>   fs/btrfs/volumes.h |  1 -
>>>       >>>   2 files changed, 34 insertions(+), 11 deletions(-)
>>>       >>>
>>>       >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>       >>> index 474ab7ed65ea..192c540a650c 100644
>>>       >>> --- a/fs/btrfs/volumes.c
>>>       >>> +++ b/fs/btrfs/volumes.c
>>>       >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>>>       >>>       return ret;
>>>       >>>   }
>>>       >>> +static bool btrfs_skip_registration(struct btrfs_super_block
>>>       >>> *disk_super,
>>>       >>> +                    dev_t devt, bool mount_arg_dev)
>>>       >>> +{
>>>       >>> +    struct btrfs_fs_devices *fs_devices;
>>>       >>> +
>>>       >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>>>       >>> +        struct btrfs_device *device;
>>>       >>> +
>>>       >>> +        mutex_lock(&fs_devices->device_list_mutex);
>>>       >>> +        list_for_each_entry(device, &fs_devices->devices,
>>>      dev_list) {
>>>       >>> +            if (device->devt == devt) {
>>>       >>> +                mutex_unlock(&fs_devices->device_list_mutex);
>>>       >>> +                return false;
>>>       >>> +            }
>>>       >>> +        }
>>>       >>> +        mutex_unlock(&fs_devices->device_list_mutex);
>>>       >>
>>>       >> This is locking and unlocking again before going to
>>>      device_list_add, so
>>>       >> if something changes regarding the registered device then it's
>>>      not up to
>>>       >> date.
>>>       >>
>>>       >
>>>
>>>      We are in the uuid_mutex, a potentially racing thread will have to
>>>      acquire this mutex to delete from the list. So there can't a race.
>>>
>>>
>>>
>>>       > Right. A race might happen, but it is not an issue. At worst, there
>>>       > will be a stale device in the cache, which gets removed or re-used
>>>       > in the next mkfs or mount of the same device.
>>>       >
>>>       > However, this is a rough cut that we need to fix. I am reviewing
>>>       > your approach as well. I'm fine with any fix.
>>>       >
>>>       >
>>>       >>
>>>       >>> +    }
>>>       >>> +
>>>       >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
>>>      == 1 &&
>>>       >>> +        !(btrfs_super_flags(disk_super) &
>>>      BTRFS_SUPER_FLAG_SEEDING))
>>>       >>> +        return true;
>>>       >>
>>>       >> The way I implemented it is to check the above conditions as a
>>>       >> prerequisite but leave the heavy work for device_list_add that
>>>      does all
>>>       >> the uuid and device list locking and we are quite sure it
>>>      survives all
>>>       >> the races between scanning and mounts.
>>>       >>
>>>       >
>>>       > Hm. But isn't that the bug we need to fix? That we skipped the device
>>>       > scan thread that wanted to replace the device path from /dev/root to
>>>       > /dev/sdx?
>>>       >
>>>       > And we skipped, because it was not a mount thread
>>>       > (%mount_arg_dev=false), and the device is already mounted and the
>>>       > devt will match?
>>>       >
>>>       > So my fix also checked if devt is a match, then allow it to scan
>>>       > (so that the device path can be updated, such as /dev/root to
>>>      /dev/sdx).
>>>       >
>>>       > To confirm the bug, I asked for the debug kernel messages, I don't
>>>       > this we got it. Also, the existing kernel log shows no such issue.
>>>       >
>>>       >
>>>       >>> +
>>>       >>> +    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
>>>       >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
>>>       >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>>>       >>>       struct btrfs_device *device = NULL;
>>>       >>>       struct bdev_handle *bdev_handle;
>>>       >>>       u64 bytenr, bytenr_orig;
>>>       >>> +    dev_t devt = 0;
>>>       >>>       int ret;
>>>       >>>       lockdep_assert_held(&uuid_mutex);
>>>       >>> @@ -1355,18 +1381,16 @@ 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;
>>>       >>> +    ret = lookup_bdev(path, &devt);
>>>       >>> +    if (ret)
>>>       >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>>       >>> +               path, ret);
>>>       >>> -        ret = lookup_bdev(path, &devt);
>>>       >>
>>>       >> Do we actually need this check? It was added with the patch
>>>      skipping the
>>>       >> registration, so it's validating the block device but how can we
>>>      pass
>>>       >> something that is not a valid block device?
>>>       >>
>>>       >
>>>       > Do you mean to check if the lookup_bdev() is successful? Hm. It
>>>      should
>>>       > be okay not to check, but we do that consistently in other places.
>>>       >
>>>       >> Besides there's a call to bdev_open_by_path() that in turn does the
>>>       >> lookup_bdev so checking it here is redundant. It's not related
>>>      to the
>>>       >> fix itself but I deleted it in my fix.
>>>       >>
>>>       >
>>>       > Oh no. We need %devt to be set because:
>>>       >
>>>       > It will match if that device is already mounted/scanned.
>>>       > It will also free stale entries.
>>>       >
>>>       > Thx, Anand
>>>       >
>>>       >>> -        if (ret)
>>>       >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>>       >>> -                   path, ret);
>>>       >>> -        else
>>>       >>> +    if (btrfs_skip_registration(disk_super, devt,
>>>      mount_arg_dev)) {
>>>       >>> +        pr_debug("BTRFS: skip registering single non-seed
>>>      device %s\n",
>>>       >>> +              path);
>>>       >>> +        if (devt)
>>>       >>>               btrfs_free_stale_devices(devt, NULL);
>>>       >>> -
>>>       >>> -        pr_debug("BTRFS: skip registering single non-seed device
>>>       >>> %s\n", path);
>>>       >>>           device = NULL;
>>>       >>>           goto free_disk_super;
>>>       >>>       }
>>>

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

[Alex Romosan]

okay, so this is the original patch from Monday. I tried the version
David committed to to the btrfs tree and that fixed the problem but
I'll try this one as well (with debugging now enabled) and let you
know.

On Thu, Feb 8, 2024 at 6:22 PM Anand Jain <anand.jain@oracle.com> wrote:
>
>
> Thanks for the kernel messages with debug enabled.
>
> I don't see the message to skip scannaing for
> the mounted device. So it's not what we thought
> was the issue.
>
>    pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
>
>
> Based on the assumption above, I have a fix below,
> but I doubt its effectiveness.
>
>
> https://patchwork.kernel.org/project/linux-btrfs/patch/8dd1990114aabb775d4631969f1beabeadaac5b7.1707132247.git.anand.jain@oracle.com/
>
> -Anand
>
>
> On 2/8/24 18:05, Alex Romosan wrote:
> > i'm attaching my boot log with 6.8.0-rc3 no fixes and btrfs debug
> > enabled (i assume this is what you wanted). update-grub doesn't work.
> > there was no patch in your last message. do you want me to try the
> > patch you sent on monday? confused
> >
> > On Thu, Feb 8, 2024 at 3:23 AM Anand Jain <anand.jain@oracle.com> wrote:
> >>
> >>
> >> Alex,
> >>
> >> Please provide the kernel boot messages with debugging enabled but
> >> no fixes applied. Kindly collect these messages after reproducing
> >> the problem.
> >>
> >> We've found issues with the previous fix. Please test this
> >> new patch, as it may address the bug. Keep debugging enabled
> >> during testing.
> >>
> >>
> >> Thanks ,Anand
> >>
> >>
> >> On 2/7/24 23:48, Alex Romosan wrote:
> >>> Which version of the patch are we talking about? Let me know and I’ll
> >>> try it with debugging on. I tried David’s patch and it seemed to work (I
> >>> just booted into that kernel and ran update-grub) but I’ll try something
> >>> else…
> >>>
> >>> On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com
> >>> <mailto:anand.jain@oracle.com>> wrote:
> >>>
> >>>
> >>>
> >>>      On 2/7/24 08:08, Anand Jain wrote:
> >>>       >
> >>>       >
> >>>       >
> >>>       > On 2/5/24 18:27, David Sterba wrote:
> >>>       >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> >>>       >>> We skip device registration for a single device. However, we do
> >>>      not do
> >>>       >>> that if the device is already mounted, as it might be coming in
> >>>      again
> >>>       >>> for scanning a different path.
> >>>       >>>
> >>>       >>> This patch is lightly tested; for verification if it fixes.
> >>>       >>>
> >>>       >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
> >>>      <mailto:anand.jain@oracle.com>>
> >>>       >>> ---
> >>>       >>> I still have some unknowns about the problem. Pls test if this
> >>>      fixes
> >>>       >>> the problem.
> >>>
> >>>      Successfully tested with fstests (-g volume) and temp-fsid test cases.
> >>>
> >>>      Can someone verify if this patch fixes the problem? Also, when problem
> >>>      occurs please provide kernel messages with Btrfs debugging support
> >>>      option compiled in.
> >>>
> >>>      Thanks, Anand
> >>>
> >>>
> >>>       >>>
> >>>       >>>   fs/btrfs/volumes.c | 44
> >>>      ++++++++++++++++++++++++++++++++++----------
> >>>       >>>   fs/btrfs/volumes.h |  1 -
> >>>       >>>   2 files changed, 34 insertions(+), 11 deletions(-)
> >>>       >>>
> >>>       >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> >>>       >>> index 474ab7ed65ea..192c540a650c 100644
> >>>       >>> --- a/fs/btrfs/volumes.c
> >>>       >>> +++ b/fs/btrfs/volumes.c
> >>>       >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
> >>>       >>>       return ret;
> >>>       >>>   }
> >>>       >>> +static bool btrfs_skip_registration(struct btrfs_super_block
> >>>       >>> *disk_super,
> >>>       >>> +                    dev_t devt, bool mount_arg_dev)
> >>>       >>> +{
> >>>       >>> +    struct btrfs_fs_devices *fs_devices;
> >>>       >>> +
> >>>       >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> >>>       >>> +        struct btrfs_device *device;
> >>>       >>> +
> >>>       >>> +        mutex_lock(&fs_devices->device_list_mutex);
> >>>       >>> +        list_for_each_entry(device, &fs_devices->devices,
> >>>      dev_list) {
> >>>       >>> +            if (device->devt == devt) {
> >>>       >>> +                mutex_unlock(&fs_devices->device_list_mutex);
> >>>       >>> +                return false;
> >>>       >>> +            }
> >>>       >>> +        }
> >>>       >>> +        mutex_unlock(&fs_devices->device_list_mutex);
> >>>       >>
> >>>       >> This is locking and unlocking again before going to
> >>>      device_list_add, so
> >>>       >> if something changes regarding the registered device then it's
> >>>      not up to
> >>>       >> date.
> >>>       >>
> >>>       >
> >>>
> >>>      We are in the uuid_mutex, a potentially racing thread will have to
> >>>      acquire this mutex to delete from the list. So there can't a race.
> >>>
> >>>
> >>>
> >>>       > Right. A race might happen, but it is not an issue. At worst, there
> >>>       > will be a stale device in the cache, which gets removed or re-used
> >>>       > in the next mkfs or mount of the same device.
> >>>       >
> >>>       > However, this is a rough cut that we need to fix. I am reviewing
> >>>       > your approach as well. I'm fine with any fix.
> >>>       >
> >>>       >
> >>>       >>
> >>>       >>> +    }
> >>>       >>> +
> >>>       >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
> >>>      == 1 &&
> >>>       >>> +        !(btrfs_super_flags(disk_super) &
> >>>      BTRFS_SUPER_FLAG_SEEDING))
> >>>       >>> +        return true;
> >>>       >>
> >>>       >> The way I implemented it is to check the above conditions as a
> >>>       >> prerequisite but leave the heavy work for device_list_add that
> >>>      does all
> >>>       >> the uuid and device list locking and we are quite sure it
> >>>      survives all
> >>>       >> the races between scanning and mounts.
> >>>       >>
> >>>       >
> >>>       > Hm. But isn't that the bug we need to fix? That we skipped the device
> >>>       > scan thread that wanted to replace the device path from /dev/root to
> >>>       > /dev/sdx?
> >>>       >
> >>>       > And we skipped, because it was not a mount thread
> >>>       > (%mount_arg_dev=false), and the device is already mounted and the
> >>>       > devt will match?
> >>>       >
> >>>       > So my fix also checked if devt is a match, then allow it to scan
> >>>       > (so that the device path can be updated, such as /dev/root to
> >>>      /dev/sdx).
> >>>       >
> >>>       > To confirm the bug, I asked for the debug kernel messages, I don't
> >>>       > this we got it. Also, the existing kernel log shows no such issue.
> >>>       >
> >>>       >
> >>>       >>> +
> >>>       >>> +    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
> >>>       >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
> >>>       >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
> >>>       >>>       struct btrfs_device *device = NULL;
> >>>       >>>       struct bdev_handle *bdev_handle;
> >>>       >>>       u64 bytenr, bytenr_orig;
> >>>       >>> +    dev_t devt = 0;
> >>>       >>>       int ret;
> >>>       >>>       lockdep_assert_held(&uuid_mutex);
> >>>       >>> @@ -1355,18 +1381,16 @@ 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;
> >>>       >>> +    ret = lookup_bdev(path, &devt);
> >>>       >>> +    if (ret)
> >>>       >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >>>       >>> +               path, ret);
> >>>       >>> -        ret = lookup_bdev(path, &devt);
> >>>       >>
> >>>       >> Do we actually need this check? It was added with the patch
> >>>      skipping the
> >>>       >> registration, so it's validating the block device but how can we
> >>>      pass
> >>>       >> something that is not a valid block device?
> >>>       >>
> >>>       >
> >>>       > Do you mean to check if the lookup_bdev() is successful? Hm. It
> >>>      should
> >>>       > be okay not to check, but we do that consistently in other places.
> >>>       >
> >>>       >> Besides there's a call to bdev_open_by_path() that in turn does the
> >>>       >> lookup_bdev so checking it here is redundant. It's not related
> >>>      to the
> >>>       >> fix itself but I deleted it in my fix.
> >>>       >>
> >>>       >
> >>>       > Oh no. We need %devt to be set because:
> >>>       >
> >>>       > It will match if that device is already mounted/scanned.
> >>>       > It will also free stale entries.
> >>>       >
> >>>       > Thx, Anand
> >>>       >
> >>>       >>> -        if (ret)
> >>>       >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >>>       >>> -                   path, ret);
> >>>       >>> -        else
> >>>       >>> +    if (btrfs_skip_registration(disk_super, devt,
> >>>      mount_arg_dev)) {
> >>>       >>> +        pr_debug("BTRFS: skip registering single non-seed
> >>>      device %s\n",
> >>>       >>> +              path);
> >>>       >>> +        if (devt)
> >>>       >>>               btrfs_free_stale_devices(devt, NULL);
> >>>       >>> -
> >>>       >>> -        pr_debug("BTRFS: skip registering single non-seed device
> >>>       >>> %s\n", path);
> >>>       >>>           device = NULL;
> >>>       >>>           goto free_disk_super;
> >>>       >>>       }
> >>>

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

[Alex Romosan]

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

i'm attaching the boot log from 6.8-rc3 with your patch. update-grub
works. i took a quick look at the log and i can see this (which wasn't
in the unpatched kernel):

BTRFS info: devid 1 device path /dev/root changed to /dev/nvme0n1p3
scanned by (udev-worker)

On Thu, Feb 8, 2024 at 6:22 PM Anand Jain <anand.jain@oracle.com> wrote:
>
>
> Thanks for the kernel messages with debug enabled.
>
> I don't see the message to skip scannaing for
> the mounted device. So it's not what we thought
> was the issue.
>
>    pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
>
>
> Based on the assumption above, I have a fix below,
> but I doubt its effectiveness.
>
>
> https://patchwork.kernel.org/project/linux-btrfs/patch/8dd1990114aabb775d4631969f1beabeadaac5b7.1707132247.git.anand.jain@oracle.com/
>
> -Anand
>
>
> On 2/8/24 18:05, Alex Romosan wrote:
> > i'm attaching my boot log with 6.8.0-rc3 no fixes and btrfs debug
> > enabled (i assume this is what you wanted). update-grub doesn't work.
> > there was no patch in your last message. do you want me to try the
> > patch you sent on monday? confused
> >
> > On Thu, Feb 8, 2024 at 3:23 AM Anand Jain <anand.jain@oracle.com> wrote:
> >>
> >>
> >> Alex,
> >>
> >> Please provide the kernel boot messages with debugging enabled but
> >> no fixes applied. Kindly collect these messages after reproducing
> >> the problem.
> >>
> >> We've found issues with the previous fix. Please test this
> >> new patch, as it may address the bug. Keep debugging enabled
> >> during testing.
> >>
> >>
> >> Thanks ,Anand
> >>
> >>
> >> On 2/7/24 23:48, Alex Romosan wrote:
> >>> Which version of the patch are we talking about? Let me know and I’ll
> >>> try it with debugging on. I tried David’s patch and it seemed to work (I
> >>> just booted into that kernel and ran update-grub) but I’ll try something
> >>> else…
> >>>
> >>> On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com
> >>> <mailto:anand.jain@oracle.com>> wrote:
> >>>
> >>>
> >>>
> >>>      On 2/7/24 08:08, Anand Jain wrote:
> >>>       >
> >>>       >
> >>>       >
> >>>       > On 2/5/24 18:27, David Sterba wrote:
> >>>       >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> >>>       >>> We skip device registration for a single device. However, we do
> >>>      not do
> >>>       >>> that if the device is already mounted, as it might be coming in
> >>>      again
> >>>       >>> for scanning a different path.
> >>>       >>>
> >>>       >>> This patch is lightly tested; for verification if it fixes.
> >>>       >>>
> >>>       >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
> >>>      <mailto:anand.jain@oracle.com>>
> >>>       >>> ---
> >>>       >>> I still have some unknowns about the problem. Pls test if this
> >>>      fixes
> >>>       >>> the problem.
> >>>
> >>>      Successfully tested with fstests (-g volume) and temp-fsid test cases.
> >>>
> >>>      Can someone verify if this patch fixes the problem? Also, when problem
> >>>      occurs please provide kernel messages with Btrfs debugging support
> >>>      option compiled in.
> >>>
> >>>      Thanks, Anand
> >>>
> >>>
> >>>       >>>
> >>>       >>>   fs/btrfs/volumes.c | 44
> >>>      ++++++++++++++++++++++++++++++++++----------
> >>>       >>>   fs/btrfs/volumes.h |  1 -
> >>>       >>>   2 files changed, 34 insertions(+), 11 deletions(-)
> >>>       >>>
> >>>       >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> >>>       >>> index 474ab7ed65ea..192c540a650c 100644
> >>>       >>> --- a/fs/btrfs/volumes.c
> >>>       >>> +++ b/fs/btrfs/volumes.c
> >>>       >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
> >>>       >>>       return ret;
> >>>       >>>   }
> >>>       >>> +static bool btrfs_skip_registration(struct btrfs_super_block
> >>>       >>> *disk_super,
> >>>       >>> +                    dev_t devt, bool mount_arg_dev)
> >>>       >>> +{
> >>>       >>> +    struct btrfs_fs_devices *fs_devices;
> >>>       >>> +
> >>>       >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
> >>>       >>> +        struct btrfs_device *device;
> >>>       >>> +
> >>>       >>> +        mutex_lock(&fs_devices->device_list_mutex);
> >>>       >>> +        list_for_each_entry(device, &fs_devices->devices,
> >>>      dev_list) {
> >>>       >>> +            if (device->devt == devt) {
> >>>       >>> +                mutex_unlock(&fs_devices->device_list_mutex);
> >>>       >>> +                return false;
> >>>       >>> +            }
> >>>       >>> +        }
> >>>       >>> +        mutex_unlock(&fs_devices->device_list_mutex);
> >>>       >>
> >>>       >> This is locking and unlocking again before going to
> >>>      device_list_add, so
> >>>       >> if something changes regarding the registered device then it's
> >>>      not up to
> >>>       >> date.
> >>>       >>
> >>>       >
> >>>
> >>>      We are in the uuid_mutex, a potentially racing thread will have to
> >>>      acquire this mutex to delete from the list. So there can't a race.
> >>>
> >>>
> >>>
> >>>       > Right. A race might happen, but it is not an issue. At worst, there
> >>>       > will be a stale device in the cache, which gets removed or re-used
> >>>       > in the next mkfs or mount of the same device.
> >>>       >
> >>>       > However, this is a rough cut that we need to fix. I am reviewing
> >>>       > your approach as well. I'm fine with any fix.
> >>>       >
> >>>       >
> >>>       >>
> >>>       >>> +    }
> >>>       >>> +
> >>>       >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
> >>>      == 1 &&
> >>>       >>> +        !(btrfs_super_flags(disk_super) &
> >>>      BTRFS_SUPER_FLAG_SEEDING))
> >>>       >>> +        return true;
> >>>       >>
> >>>       >> The way I implemented it is to check the above conditions as a
> >>>       >> prerequisite but leave the heavy work for device_list_add that
> >>>      does all
> >>>       >> the uuid and device list locking and we are quite sure it
> >>>      survives all
> >>>       >> the races between scanning and mounts.
> >>>       >>
> >>>       >
> >>>       > Hm. But isn't that the bug we need to fix? That we skipped the device
> >>>       > scan thread that wanted to replace the device path from /dev/root to
> >>>       > /dev/sdx?
> >>>       >
> >>>       > And we skipped, because it was not a mount thread
> >>>       > (%mount_arg_dev=false), and the device is already mounted and the
> >>>       > devt will match?
> >>>       >
> >>>       > So my fix also checked if devt is a match, then allow it to scan
> >>>       > (so that the device path can be updated, such as /dev/root to
> >>>      /dev/sdx).
> >>>       >
> >>>       > To confirm the bug, I asked for the debug kernel messages, I don't
> >>>       > this we got it. Also, the existing kernel log shows no such issue.
> >>>       >
> >>>       >
> >>>       >>> +
> >>>       >>> +    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
> >>>       >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
> >>>       >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
> >>>       >>>       struct btrfs_device *device = NULL;
> >>>       >>>       struct bdev_handle *bdev_handle;
> >>>       >>>       u64 bytenr, bytenr_orig;
> >>>       >>> +    dev_t devt = 0;
> >>>       >>>       int ret;
> >>>       >>>       lockdep_assert_held(&uuid_mutex);
> >>>       >>> @@ -1355,18 +1381,16 @@ 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;
> >>>       >>> +    ret = lookup_bdev(path, &devt);
> >>>       >>> +    if (ret)
> >>>       >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >>>       >>> +               path, ret);
> >>>       >>> -        ret = lookup_bdev(path, &devt);
> >>>       >>
> >>>       >> Do we actually need this check? It was added with the patch
> >>>      skipping the
> >>>       >> registration, so it's validating the block device but how can we
> >>>      pass
> >>>       >> something that is not a valid block device?
> >>>       >>
> >>>       >
> >>>       > Do you mean to check if the lookup_bdev() is successful? Hm. It
> >>>      should
> >>>       > be okay not to check, but we do that consistently in other places.
> >>>       >
> >>>       >> Besides there's a call to bdev_open_by_path() that in turn does the
> >>>       >> lookup_bdev so checking it here is redundant. It's not related
> >>>      to the
> >>>       >> fix itself but I deleted it in my fix.
> >>>       >>
> >>>       >
> >>>       > Oh no. We need %devt to be set because:
> >>>       >
> >>>       > It will match if that device is already mounted/scanned.
> >>>       > It will also free stale entries.
> >>>       >
> >>>       > Thx, Anand
> >>>       >
> >>>       >>> -        if (ret)
> >>>       >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
> >>>       >>> -                   path, ret);
> >>>       >>> -        else
> >>>       >>> +    if (btrfs_skip_registration(disk_super, devt,
> >>>      mount_arg_dev)) {
> >>>       >>> +        pr_debug("BTRFS: skip registering single non-seed
> >>>      device %s\n",
> >>>       >>> +              path);
> >>>       >>> +        if (devt)
> >>>       >>>               btrfs_free_stale_devices(devt, NULL);
> >>>       >>> -
> >>>       >>> -        pr_debug("BTRFS: skip registering single non-seed device
> >>>       >>> %s\n", path);
> >>>       >>>           device = NULL;
> >>>       >>>           goto free_disk_super;
> >>>       >>>       }
> >>>

[-- Attachment #2: log.6.8.0-rc3-dirty.gz --]
[-- Type: application/gzip, Size: 32383 bytes --]

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

[Anand Jain]

On 2/9/24 01:34, Alex Romosan wrote:
> i'm attaching the boot log from 6.8-rc3 with your patch. update-grub
> works. i took a quick look at the log and i can see this (which wasn't
> in the unpatched kernel):
> 
> BTRFS info: devid 1 device path /dev/root changed to /dev/nvme0n1p3
> scanned by (udev-worker)
> 

  That's nice. Thanks for verifying.

Anand


> On Thu, Feb 8, 2024 at 6:22 PM Anand Jain <anand.jain@oracle.com> wrote:
>>
>>
>> Thanks for the kernel messages with debug enabled.
>>
>> I don't see the message to skip scannaing for
>> the mounted device. So it's not what we thought
>> was the issue.
>>
>>     pr_debug("BTRFS: skip registering single non-seed device %s\n", path);
>>
>>
>> Based on the assumption above, I have a fix below,
>> but I doubt its effectiveness.
>>
>>
>> https://patchwork.kernel.org/project/linux-btrfs/patch/8dd1990114aabb775d4631969f1beabeadaac5b7.1707132247.git.anand.jain@oracle.com/
>>
>> -Anand
>>
>>
>> On 2/8/24 18:05, Alex Romosan wrote:
>>> i'm attaching my boot log with 6.8.0-rc3 no fixes and btrfs debug
>>> enabled (i assume this is what you wanted). update-grub doesn't work.
>>> there was no patch in your last message. do you want me to try the
>>> patch you sent on monday? confused
>>>
>>> On Thu, Feb 8, 2024 at 3:23 AM Anand Jain <anand.jain@oracle.com> wrote:
>>>>
>>>>
>>>> Alex,
>>>>
>>>> Please provide the kernel boot messages with debugging enabled but
>>>> no fixes applied. Kindly collect these messages after reproducing
>>>> the problem.
>>>>
>>>> We've found issues with the previous fix. Please test this
>>>> new patch, as it may address the bug. Keep debugging enabled
>>>> during testing.
>>>>
>>>>
>>>> Thanks ,Anand
>>>>
>>>>
>>>> On 2/7/24 23:48, Alex Romosan wrote:
>>>>> Which version of the patch are we talking about? Let me know and I’ll
>>>>> try it with debugging on. I tried David’s patch and it seemed to work (I
>>>>> just booted into that kernel and ran update-grub) but I’ll try something
>>>>> else…
>>>>>
>>>>> On Wed, Feb 7, 2024 at 19:08 Anand Jain <anand.jain@oracle.com
>>>>> <mailto:anand.jain@oracle.com>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>       On 2/7/24 08:08, Anand Jain wrote:
>>>>>        >
>>>>>        >
>>>>>        >
>>>>>        > On 2/5/24 18:27, David Sterba wrote:
>>>>>        >> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>>>>>        >>> We skip device registration for a single device. However, we do
>>>>>       not do
>>>>>        >>> that if the device is already mounted, as it might be coming in
>>>>>       again
>>>>>        >>> for scanning a different path.
>>>>>        >>>
>>>>>        >>> This patch is lightly tested; for verification if it fixes.
>>>>>        >>>
>>>>>        >>> Signed-off-by: Anand Jain <anand.jain@oracle.com
>>>>>       <mailto:anand.jain@oracle.com>>
>>>>>        >>> ---
>>>>>        >>> I still have some unknowns about the problem. Pls test if this
>>>>>       fixes
>>>>>        >>> the problem.
>>>>>
>>>>>       Successfully tested with fstests (-g volume) and temp-fsid test cases.
>>>>>
>>>>>       Can someone verify if this patch fixes the problem? Also, when problem
>>>>>       occurs please provide kernel messages with Btrfs debugging support
>>>>>       option compiled in.
>>>>>
>>>>>       Thanks, Anand
>>>>>
>>>>>
>>>>>        >>>
>>>>>        >>>   fs/btrfs/volumes.c | 44
>>>>>       ++++++++++++++++++++++++++++++++++----------
>>>>>        >>>   fs/btrfs/volumes.h |  1 -
>>>>>        >>>   2 files changed, 34 insertions(+), 11 deletions(-)
>>>>>        >>>
>>>>>        >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>>>        >>> index 474ab7ed65ea..192c540a650c 100644
>>>>>        >>> --- a/fs/btrfs/volumes.c
>>>>>        >>> +++ b/fs/btrfs/volumes.c
>>>>>        >>> @@ -1299,6 +1299,31 @@ int btrfs_forget_devices(dev_t devt)
>>>>>        >>>       return ret;
>>>>>        >>>   }
>>>>>        >>> +static bool btrfs_skip_registration(struct btrfs_super_block
>>>>>        >>> *disk_super,
>>>>>        >>> +                    dev_t devt, bool mount_arg_dev)
>>>>>        >>> +{
>>>>>        >>> +    struct btrfs_fs_devices *fs_devices;
>>>>>        >>> +
>>>>>        >>> +    list_for_each_entry(fs_devices, &fs_uuids, fs_list) {
>>>>>        >>> +        struct btrfs_device *device;
>>>>>        >>> +
>>>>>        >>> +        mutex_lock(&fs_devices->device_list_mutex);
>>>>>        >>> +        list_for_each_entry(device, &fs_devices->devices,
>>>>>       dev_list) {
>>>>>        >>> +            if (device->devt == devt) {
>>>>>        >>> +                mutex_unlock(&fs_devices->device_list_mutex);
>>>>>        >>> +                return false;
>>>>>        >>> +            }
>>>>>        >>> +        }
>>>>>        >>> +        mutex_unlock(&fs_devices->device_list_mutex);
>>>>>        >>
>>>>>        >> This is locking and unlocking again before going to
>>>>>       device_list_add, so
>>>>>        >> if something changes regarding the registered device then it's
>>>>>       not up to
>>>>>        >> date.
>>>>>        >>
>>>>>        >
>>>>>
>>>>>       We are in the uuid_mutex, a potentially racing thread will have to
>>>>>       acquire this mutex to delete from the list. So there can't a race.
>>>>>
>>>>>
>>>>>
>>>>>        > Right. A race might happen, but it is not an issue. At worst, there
>>>>>        > will be a stale device in the cache, which gets removed or re-used
>>>>>        > in the next mkfs or mount of the same device.
>>>>>        >
>>>>>        > However, this is a rough cut that we need to fix. I am reviewing
>>>>>        > your approach as well. I'm fine with any fix.
>>>>>        >
>>>>>        >
>>>>>        >>
>>>>>        >>> +    }
>>>>>        >>> +
>>>>>        >>> +    if (!mount_arg_dev && btrfs_super_num_devices(disk_super)
>>>>>       == 1 &&
>>>>>        >>> +        !(btrfs_super_flags(disk_super) &
>>>>>       BTRFS_SUPER_FLAG_SEEDING))
>>>>>        >>> +        return true;
>>>>>        >>
>>>>>        >> The way I implemented it is to check the above conditions as a
>>>>>        >> prerequisite but leave the heavy work for device_list_add that
>>>>>       does all
>>>>>        >> the uuid and device list locking and we are quite sure it
>>>>>       survives all
>>>>>        >> the races between scanning and mounts.
>>>>>        >>
>>>>>        >
>>>>>        > Hm. But isn't that the bug we need to fix? That we skipped the device
>>>>>        > scan thread that wanted to replace the device path from /dev/root to
>>>>>        > /dev/sdx?
>>>>>        >
>>>>>        > And we skipped, because it was not a mount thread
>>>>>        > (%mount_arg_dev=false), and the device is already mounted and the
>>>>>        > devt will match?
>>>>>        >
>>>>>        > So my fix also checked if devt is a match, then allow it to scan
>>>>>        > (so that the device path can be updated, such as /dev/root to
>>>>>       /dev/sdx).
>>>>>        >
>>>>>        > To confirm the bug, I asked for the debug kernel messages, I don't
>>>>>        > this we got it. Also, the existing kernel log shows no such issue.
>>>>>        >
>>>>>        >
>>>>>        >>> +
>>>>>        >>> +    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
>>>>>        >>> @@ -1316,6 +1341,7 @@ struct btrfs_device
>>>>>        >>> *btrfs_scan_one_device(const char *path, blk_mode_t flags,
>>>>>        >>>       struct btrfs_device *device = NULL;
>>>>>        >>>       struct bdev_handle *bdev_handle;
>>>>>        >>>       u64 bytenr, bytenr_orig;
>>>>>        >>> +    dev_t devt = 0;
>>>>>        >>>       int ret;
>>>>>        >>>       lockdep_assert_held(&uuid_mutex);
>>>>>        >>> @@ -1355,18 +1381,16 @@ 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;
>>>>>        >>> +    ret = lookup_bdev(path, &devt);
>>>>>        >>> +    if (ret)
>>>>>        >>> +        btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>>>>        >>> +               path, ret);
>>>>>        >>> -        ret = lookup_bdev(path, &devt);
>>>>>        >>
>>>>>        >> Do we actually need this check? It was added with the patch
>>>>>       skipping the
>>>>>        >> registration, so it's validating the block device but how can we
>>>>>       pass
>>>>>        >> something that is not a valid block device?
>>>>>        >>
>>>>>        >
>>>>>        > Do you mean to check if the lookup_bdev() is successful? Hm. It
>>>>>       should
>>>>>        > be okay not to check, but we do that consistently in other places.
>>>>>        >
>>>>>        >> Besides there's a call to bdev_open_by_path() that in turn does the
>>>>>        >> lookup_bdev so checking it here is redundant. It's not related
>>>>>       to the
>>>>>        >> fix itself but I deleted it in my fix.
>>>>>        >>
>>>>>        >
>>>>>        > Oh no. We need %devt to be set because:
>>>>>        >
>>>>>        > It will match if that device is already mounted/scanned.
>>>>>        > It will also free stale entries.
>>>>>        >
>>>>>        > Thx, Anand
>>>>>        >
>>>>>        >>> -        if (ret)
>>>>>        >>> -            btrfs_warn(NULL, "lookup bdev failed for path %s: %d",
>>>>>        >>> -                   path, ret);
>>>>>        >>> -        else
>>>>>        >>> +    if (btrfs_skip_registration(disk_super, devt,
>>>>>       mount_arg_dev)) {
>>>>>        >>> +        pr_debug("BTRFS: skip registering single non-seed
>>>>>       device %s\n",
>>>>>        >>> +              path);
>>>>>        >>> +        if (devt)
>>>>>        >>>               btrfs_free_stale_devices(devt, NULL);
>>>>>        >>> -
>>>>>        >>> -        pr_debug("BTRFS: skip registering single non-seed device
>>>>>        >>> %s\n", path);
>>>>>        >>>           device = NULL;
>>>>>        >>>           goto free_disk_super;
>>>>>        >>>       }
>>>>>

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

[Dr. Bernd Feige]

QW0gRnJlaXRhZywgZGVtIDA5LjAyLjIwMjQgdW0gMTM6NDEgKzA1MzAgc2NocmllYiBBbmFuZCBK
YWluOgo+IAo+IAo+IE9uIDIvOS8yNCAwMTozNCwgQWxleCBSb21vc2FuIHdyb3RlOgo+ID4gaSdt
IGF0dGFjaGluZyB0aGUgYm9vdCBsb2cgZnJvbSA2LjgtcmMzIHdpdGggeW91ciBwYXRjaC4gdXBk
YXRlLQo+ID4gZ3J1Ygo+ID4gd29ya3MuIGkgdG9vayBhIHF1aWNrIGxvb2sgYXQgdGhlIGxvZyBh
bmQgaSBjYW4gc2VlIHRoaXMgKHdoaWNoCj4gPiB3YXNuJ3QKPiA+IGluIHRoZSB1bnBhdGNoZWQg
a2VybmVsKToKPiA+IAo+ID4gQlRSRlMgaW5mbzogZGV2aWQgMSBkZXZpY2UgcGF0aCAvZGV2L3Jv
b3QgY2hhbmdlZCB0byAvZGV2L252bWUwbjFwMwo+ID4gc2Nhbm5lZCBieSAodWRldi13b3JrZXIp
Cj4gPiAKPiAKPiDCoCBUaGF0J3MgbmljZS4gVGhhbmtzIGZvciB2ZXJpZnlpbmcuCj4gCgpUZXN0
ZWQgaGVyZSBhcyB3ZWxsLCBmaXhlcyB0aGUgcHJvYmxlbSBmb3IgbWUgdG9vLgpUaGFua3MhCkJl
cm5kCgoKPiBBbmFuZAo+IAo+IAo+ID4gT24gVGh1LCBGZWIgOCwgMjAyNCBhdCA2OjIy4oCvUE0g
QW5hbmQgSmFpbiA8YW5hbmQuamFpbkBvcmFjbGUuY29tPgo+ID4gd3JvdGU6Cj4gPiA+IAo+ID4g
PiAKPiA+ID4gVGhhbmtzIGZvciB0aGUga2VybmVsIG1lc3NhZ2VzIHdpdGggZGVidWcgZW5hYmxl
ZC4KPiA+ID4gCj4gPiA+IEkgZG9uJ3Qgc2VlIHRoZSBtZXNzYWdlIHRvIHNraXAgc2Nhbm5haW5n
IGZvcgo+ID4gPiB0aGUgbW91bnRlZCBkZXZpY2UuIFNvIGl0J3Mgbm90IHdoYXQgd2UgdGhvdWdo
dAo+ID4gPiB3YXMgdGhlIGlzc3VlLgo+ID4gPiAKPiA+ID4gwqDCoMKgIHByX2RlYnVnKCJCVFJG
Uzogc2tpcCByZWdpc3RlcmluZyBzaW5nbGUgbm9uLXNlZWQgZGV2aWNlCj4gPiA+ICVzXG4iLCBw
YXRoKTsKPiA+ID4gCj4gPiA+IAo+ID4gPiBCYXNlZCBvbiB0aGUgYXNzdW1wdGlvbiBhYm92ZSwg
SSBoYXZlIGEgZml4IGJlbG93LAo+ID4gPiBidXQgSSBkb3VidCBpdHMgZWZmZWN0aXZlbmVzcy4K
PiA+ID4gCj4gPiA+IAo+ID4gPiBodHRwczovL3BhdGNod29yay5rZXJuZWwub3JnL3Byb2plY3Qv
bGludXgtYnRyZnMvcGF0Y2gvOGRkMTk5MDExNGFhYmI3NzVkNDYzMTk2OWYxYmVhYmVhZGFhYzVi
Ny4xNzA3MTMyMjQ3LmdpdC5hbmFuZC5qYWluQG9yYWNsZS5jb20vCj4gPiA+IAo+ID4gPiAtQW5h
bmQKPiA+ID4gCj4gPiA+IAo+ID4gPiBPbiAyLzgvMjQgMTg6MDUsIEFsZXggUm9tb3NhbiB3cm90
ZToKPiA+ID4gPiBpJ20gYXR0YWNoaW5nIG15IGJvb3QgbG9nIHdpdGggNi44LjAtcmMzIG5vIGZp
eGVzIGFuZCBidHJmcwo+ID4gPiA+IGRlYnVnCj4gPiA+ID4gZW5hYmxlZCAoaSBhc3N1bWUgdGhp
cyBpcyB3aGF0IHlvdSB3YW50ZWQpLiB1cGRhdGUtZ3J1YiBkb2Vzbid0Cj4gPiA+ID4gd29yay4K
PiA+ID4gPiB0aGVyZSB3YXMgbm8gcGF0Y2ggaW4geW91ciBsYXN0IG1lc3NhZ2UuIGRvIHlvdSB3
YW50IG1lIHRvIHRyeQo+ID4gPiA+IHRoZQo+ID4gPiA+IHBhdGNoIHlvdSBzZW50IG9uIG1vbmRh
eT8gY29uZnVzZWQKPiA+ID4gPiAKPiA+ID4gPiBPbiBUaHUsIEZlYiA4LCAyMDI0IGF0IDM6MjPi
gK9BTSBBbmFuZCBKYWluCj4gPiA+ID4gPGFuYW5kLmphaW5Ab3JhY2xlLmNvbT4gd3JvdGU6Cj4g
PiA+ID4gPiAKPiA+ID4gPiA+IAo+ID4gPiA+ID4gQWxleCwKPiA+ID4gPiA+IAo+ID4gPiA+ID4g
UGxlYXNlIHByb3ZpZGUgdGhlIGtlcm5lbCBib290IG1lc3NhZ2VzIHdpdGggZGVidWdnaW5nCj4g
PiA+ID4gPiBlbmFibGVkIGJ1dAo+ID4gPiA+ID4gbm8gZml4ZXMgYXBwbGllZC4gS2luZGx5IGNv
bGxlY3QgdGhlc2UgbWVzc2FnZXMgYWZ0ZXIKPiA+ID4gPiA+IHJlcHJvZHVjaW5nCj4gPiA+ID4g
PiB0aGUgcHJvYmxlbS4KPiA+ID4gPiA+IAo+ID4gPiA+ID4gV2UndmUgZm91bmQgaXNzdWVzIHdp
dGggdGhlIHByZXZpb3VzIGZpeC4gUGxlYXNlIHRlc3QgdGhpcwo+ID4gPiA+ID4gbmV3IHBhdGNo
LCBhcyBpdCBtYXkgYWRkcmVzcyB0aGUgYnVnLiBLZWVwIGRlYnVnZ2luZyBlbmFibGVkCj4gPiA+
ID4gPiBkdXJpbmcgdGVzdGluZy4KPiA+ID4gPiA+IAo+ID4gPiA+ID4gCj4gPiA+ID4gPiBUaGFu
a3MgLEFuYW5kCj4gPiA+ID4gPiAKPiA+ID4gPiA+IAo+ID4gPiA+ID4gT24gMi83LzI0IDIzOjQ4
LCBBbGV4IFJvbW9zYW4gd3JvdGU6Cj4gPiA+ID4gPiA+IFdoaWNoIHZlcnNpb24gb2YgdGhlIHBh
dGNoIGFyZSB3ZSB0YWxraW5nIGFib3V0PyBMZXQgbWUKPiA+ID4gPiA+ID4ga25vdyBhbmQgSeKA
mWxsCj4gPiA+ID4gPiA+IHRyeSBpdCB3aXRoIGRlYnVnZ2luZyBvbi4gSSB0cmllZCBEYXZpZOKA
mXMgcGF0Y2ggYW5kIGl0Cj4gPiA+ID4gPiA+IHNlZW1lZCB0byB3b3JrIChJCj4gPiA+ID4gPiA+
IGp1c3QgYm9vdGVkIGludG8gdGhhdCBrZXJuZWwgYW5kIHJhbiB1cGRhdGUtZ3J1YikgYnV0IEni
gJlsbAo+ID4gPiA+ID4gPiB0cnkgc29tZXRoaW5nCj4gPiA+ID4gPiA+IGVsc2XigKYKPiA+ID4g
PiA+ID4gCj4gPiA+ID4gPiA+IE9uIFdlZCwgRmViIDcsIDIwMjQgYXQgMTk6MDggQW5hbmQgSmFp
bgo+ID4gPiA+ID4gPiA8YW5hbmQuamFpbkBvcmFjbGUuY29tCj4gPiA+ID4gPiA+IDxtYWlsdG86
YW5hbmQuamFpbkBvcmFjbGUuY29tPj4gd3JvdGU6Cj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiAK
PiA+ID4gPiA+ID4gCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgT24gMi83LzI0IDA4OjA4LCBBbmFu
ZCBKYWluIHdyb3RlOgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4gPiA+ID4gPiDCoMKg
wqDCoMKgwqAgPgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4gPiA+ID4gPiDCoMKgwqDC
oMKgwqAgPiBPbiAyLzUvMjQgMTg6MjcsIERhdmlkIFN0ZXJiYSB3cm90ZToKPiA+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgID4+IE9uIE1vbiwgRmViIDA1LCAyMDI0IGF0IDA3OjQ1OjA1UE0gKzA4MDAs
IEFuYW5kCj4gPiA+ID4gPiA+IEphaW4gd3JvdGU6Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+
Pj4gV2Ugc2tpcCBkZXZpY2UgcmVnaXN0cmF0aW9uIGZvciBhIHNpbmdsZSBkZXZpY2UuCj4gPiA+
ID4gPiA+IEhvd2V2ZXIsIHdlIGRvCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgbm90IGRvCj4gPiA+
ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gdGhhdCBpZiB0aGUgZGV2aWNlIGlzIGFscmVhZHkgbW91
bnRlZCwgYXMgaXQKPiA+ID4gPiA+ID4gbWlnaHQgYmUgY29taW5nIGluCj4gPiA+ID4gPiA+IMKg
wqDCoMKgwqAgYWdhaW4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiBmb3Igc2Nhbm5pbmcg
YSBkaWZmZXJlbnQgcGF0aC4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+Pgo+ID4gPiA+ID4g
PiDCoMKgwqDCoMKgwqAgPj4+IFRoaXMgcGF0Y2ggaXMgbGlnaHRseSB0ZXN0ZWQ7IGZvciB2ZXJp
ZmljYXRpb24KPiA+ID4gPiA+ID4gaWYgaXQgZml4ZXMuCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDC
oCA+Pj4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiBTaWduZWQtb2ZmLWJ5OiBBbmFuZCBK
YWluIDxhbmFuZC5qYWluQG9yYWNsZS5jb20KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoCA8bWFpbHRv
OmFuYW5kLmphaW5Ab3JhY2xlLmNvbT4+Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gLS0t
Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gSSBzdGlsbCBoYXZlIHNvbWUgdW5rbm93bnMg
YWJvdXQgdGhlIHByb2JsZW0uCj4gPiA+ID4gPiA+IFBscyB0ZXN0IGlmIHRoaXMKPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoCBmaXhlcwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IHRoZSBwcm9i
bGVtLgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoCBTdWNjZXNzZnVsbHkgdGVz
dGVkIHdpdGggZnN0ZXN0cyAoLWcgdm9sdW1lKSBhbmQKPiA+ID4gPiA+ID4gdGVtcC1mc2lkIHRl
c3QgY2FzZXMuCj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIENhbiBzb21lb25l
IHZlcmlmeSBpZiB0aGlzIHBhdGNoIGZpeGVzIHRoZSBwcm9ibGVtPwo+ID4gPiA+ID4gPiBBbHNv
LCB3aGVuIHByb2JsZW0KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoCBvY2N1cnMgcGxlYXNlIHByb3Zp
ZGUga2VybmVsIG1lc3NhZ2VzIHdpdGggQnRyZnMKPiA+ID4gPiA+ID4gZGVidWdnaW5nIHN1cHBv
cnQKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoCBvcHRpb24gY29tcGlsZWQgaW4uCj4gPiA+ID4gPiA+
IAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIFRoYW5rcywgQW5hbmQKPiA+ID4gPiA+ID4gCj4gPiA+
ID4gPiA+IAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+Cj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoCA+Pj7CoMKgIGZzL2J0cmZzL3ZvbHVtZXMuYyB8IDQ0Cj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqAgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0KPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoMKgID4+PsKgwqAgZnMvYnRyZnMvdm9sdW1lcy5oIHzCoCAxIC0KPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4+PsKgwqAgMiBmaWxlcyBjaGFuZ2VkLCAzNCBpbnNlcnRpb25z
KCspLCAxMQo+ID4gPiA+ID4gPiBkZWxldGlvbnMoLSkKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKg
ID4+Pgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IGRpZmYgLS1naXQgYS9mcy9idHJmcy92
b2x1bWVzLmMKPiA+ID4gPiA+ID4gYi9mcy9idHJmcy92b2x1bWVzLmMKPiA+ID4gPiA+ID4gwqDC
oMKgwqDCoMKgID4+PiBpbmRleCA0NzRhYjdlZDY1ZWEuLjE5MmM1NDBhNjUwYyAxMDA2NDQKPiA+
ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiAtLS0gYS9mcy9idHJmcy92b2x1bWVzLmMKPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4+PiArKysgYi9mcy9idHJmcy92b2x1bWVzLmMKPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoMKgID4+PiBAQCAtMTI5OSw2ICsxMjk5LDMxIEBAIGludAo+ID4gPiA+ID4g
PiBidHJmc19mb3JnZXRfZGV2aWNlcyhkZXZfdCBkZXZ0KQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKg
wqAgPj4+wqDCoMKgwqDCoMKgIHJldHVybiByZXQ7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+
Pj7CoMKgIH0KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiArc3RhdGljIGJvb2wgYnRyZnNf
c2tpcF9yZWdpc3RyYXRpb24oc3RydWN0Cj4gPiA+ID4gPiA+IGJ0cmZzX3N1cGVyX2Jsb2NrCj4g
PiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gKmRpc2tfc3VwZXIsCj4gPiA+ID4gPiA+IMKgwqDC
oMKgwqDCoCA+Pj4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgIGRldl90
IGRldnQsIGJvb2wKPiA+ID4gPiA+ID4gbW91bnRfYXJnX2RldikKPiA+ID4gPiA+ID4gwqDCoMKg
wqDCoMKgID4+PiArewo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqAgc3RydWN0
IGJ0cmZzX2ZzX2RldmljZXMgKmZzX2RldmljZXM7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+
Pj4gKwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqAgbGlzdF9mb3JfZWFjaF9l
bnRyeShmc19kZXZpY2VzLCAmZnNfdXVpZHMsCj4gPiA+ID4gPiA+IGZzX2xpc3QpIHsKPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4+PiArwqDCoMKgwqDCoMKgwqAgc3RydWN0IGJ0cmZzX2Rldmlj
ZSAqZGV2aWNlOwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICsKPiA+ID4gPiA+ID4gwqDC
oMKgwqDCoMKgID4+PiArwqDCoMKgwqDCoMKgwqAgbXV0ZXhfbG9jaygmZnNfZGV2aWNlcy0KPiA+
ID4gPiA+ID4gPmRldmljZV9saXN0X211dGV4KTsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+
PiArwqDCoMKgwqDCoMKgwqAgbGlzdF9mb3JfZWFjaF9lbnRyeShkZXZpY2UsCj4gPiA+ID4gPiA+
ICZmc19kZXZpY2VzLT5kZXZpY2VzLAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIGRldl9saXN0KSB7
Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqAgaWYg
KGRldmljZS0+ZGV2dCA9PSBkZXZ0KSB7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gK8Kg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCBtdXRleF91bmxvY2soJmZzX2RldmljZXMtCj4g
PiA+ID4gPiA+ID5kZXZpY2VfbGlzdF9tdXRleCk7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+
Pj4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCByZXR1cm4gZmFsc2U7Cj4gPiA+ID4g
PiA+IMKgwqDCoMKgwqDCoCA+Pj4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqAgfQo+ID4gPiA+ID4g
PiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqDCoMKgwqDCoCB9Cj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoCA+Pj4gK8KgwqDCoMKgwqDCoMKgIG11dGV4X3VubG9jaygmZnNfZGV2aWNlcy0KPiA+ID4g
PiA+ID4gPmRldmljZV9saXN0X211dGV4KTsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+Cj4g
PiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+PiBUaGlzIGlzIGxvY2tpbmcgYW5kIHVubG9ja2luZyBh
Z2FpbiBiZWZvcmUgZ29pbmcKPiA+ID4gPiA+ID4gdG8KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoCBk
ZXZpY2VfbGlzdF9hZGQsIHNvCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+PiBpZiBzb21ldGhp
bmcgY2hhbmdlcyByZWdhcmRpbmcgdGhlIHJlZ2lzdGVyZWQKPiA+ID4gPiA+ID4gZGV2aWNlIHRo
ZW4gaXQncwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIG5vdCB1cCB0bwo+ID4gPiA+ID4gPiDCoMKg
wqDCoMKgwqAgPj4gZGF0ZS4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+Cj4gPiA+ID4gPiA+
IMKgwqDCoMKgwqDCoCA+Cj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIFdlIGFy
ZSBpbiB0aGUgdXVpZF9tdXRleCwgYSBwb3RlbnRpYWxseSByYWNpbmcgdGhyZWFkCj4gPiA+ID4g
PiA+IHdpbGwgaGF2ZSB0bwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgIGFjcXVpcmUgdGhpcyBtdXRl
eCB0byBkZWxldGUgZnJvbSB0aGUgbGlzdC4gU28gdGhlcmUKPiA+ID4gPiA+ID4gY2FuJ3QgYSBy
YWNlLgo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gCj4gPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiDC
oMKgwqDCoMKgwqAgPiBSaWdodC4gQSByYWNlIG1pZ2h0IGhhcHBlbiwgYnV0IGl0IGlzIG5vdCBh
bgo+ID4gPiA+ID4gPiBpc3N1ZS4gQXQgd29yc3QsIHRoZXJlCj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoCA+IHdpbGwgYmUgYSBzdGFsZSBkZXZpY2UgaW4gdGhlIGNhY2hlLCB3aGljaCBnZXRzCj4g
PiA+ID4gPiA+IHJlbW92ZWQgb3IgcmUtdXNlZAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPiBp
biB0aGUgbmV4dCBta2ZzIG9yIG1vdW50IG9mIHRoZSBzYW1lIGRldmljZS4KPiA+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4gSG93ZXZlciwgdGhpcyBp
cyBhIHJvdWdoIGN1dCB0aGF0IHdlIG5lZWQgdG8gZml4Lgo+ID4gPiA+ID4gPiBJIGFtIHJldmll
d2luZwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPiB5b3VyIGFwcHJvYWNoIGFzIHdlbGwuIEkn
bSBmaW5lIHdpdGggYW55IGZpeC4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+Cj4gPiA+ID4gPiA+
IMKgwqDCoMKgwqDCoCA+Pj4gK8KgwqDCoCB9Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4g
Kwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqAgaWYgKCFtb3VudF9hcmdfZGV2
ICYmCj4gPiA+ID4gPiA+IGJ0cmZzX3N1cGVyX251bV9kZXZpY2VzKGRpc2tfc3VwZXIpCj4gPiA+
ID4gPiA+IMKgwqDCoMKgwqAgPT0gMSAmJgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvC
oMKgwqDCoMKgwqDCoCAhKGJ0cmZzX3N1cGVyX2ZsYWdzKGRpc2tfc3VwZXIpICYKPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoCBCVFJGU19TVVBFUl9GTEFHX1NFRURJTkcpKQo+ID4gPiA+ID4gPiDCoMKg
wqDCoMKgwqAgPj4+ICvCoMKgwqDCoMKgwqDCoCByZXR1cm4gdHJ1ZTsKPiA+ID4gPiA+ID4gwqDC
oMKgwqDCoMKgID4+Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+PiBUaGUgd2F5IEkgaW1wbGVt
ZW50ZWQgaXQgaXMgdG8gY2hlY2sgdGhlIGFib3ZlCj4gPiA+ID4gPiA+IGNvbmRpdGlvbnMgYXMg
YQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4gcHJlcmVxdWlzaXRlIGJ1dCBsZWF2ZSB0aGUg
aGVhdnkgd29yayBmb3IKPiA+ID4gPiA+ID4gZGV2aWNlX2xpc3RfYWRkIHRoYXQKPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoCBkb2VzIGFsbAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4gdGhlIHV1
aWQgYW5kIGRldmljZSBsaXN0IGxvY2tpbmcgYW5kIHdlIGFyZSBxdWl0ZQo+ID4gPiA+ID4gPiBz
dXJlIGl0Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgc3Vydml2ZXMgYWxsCj4gPiA+ID4gPiA+IMKg
wqDCoMKgwqDCoCA+PiB0aGUgcmFjZXMgYmV0d2VlbiBzY2FubmluZyBhbmQgbW91bnRzLgo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4KPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4gSG0uIEJ1dCBpc24ndCB0aGF0IHRoZSBidWcgd2UgbmVlZCB0
byBmaXg/IFRoYXQgd2UKPiA+ID4gPiA+ID4gc2tpcHBlZCB0aGUgZGV2aWNlCj4gPiA+ID4gPiA+
IMKgwqDCoMKgwqDCoCA+IHNjYW4gdGhyZWFkIHRoYXQgd2FudGVkIHRvIHJlcGxhY2UgdGhlIGRl
dmljZSBwYXRoCj4gPiA+ID4gPiA+IGZyb20gL2Rldi9yb290IHRvCj4gPiA+ID4gPiA+IMKgwqDC
oMKgwqDCoCA+IC9kZXYvc2R4Pwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4gPiA+ID4g
PiDCoMKgwqDCoMKgwqAgPiBBbmQgd2Ugc2tpcHBlZCwgYmVjYXVzZSBpdCB3YXMgbm90IGEgbW91
bnQgdGhyZWFkCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+ICglbW91bnRfYXJnX2Rldj1mYWxz
ZSksIGFuZCB0aGUgZGV2aWNlIGlzIGFscmVhZHkKPiA+ID4gPiA+ID4gbW91bnRlZCBhbmQgdGhl
Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+IGRldnQgd2lsbCBtYXRjaD8KPiA+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4gU28gbXkgZml4IGFsc28g
Y2hlY2tlZCBpZiBkZXZ0IGlzIGEgbWF0Y2gsIHRoZW4KPiA+ID4gPiA+ID4gYWxsb3cgaXQgdG8g
c2Nhbgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPiAoc28gdGhhdCB0aGUgZGV2aWNlIHBhdGgg
Y2FuIGJlIHVwZGF0ZWQsIHN1Y2ggYXMKPiA+ID4gPiA+ID4gL2Rldi9yb290IHRvCj4gPiA+ID4g
PiA+IMKgwqDCoMKgwqAgL2Rldi9zZHgpLgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPiBUbyBjb25maXJtIHRoZSBidWcsIEkgYXNrZWQgZm9yIHRo
ZSBkZWJ1ZyBrZXJuZWwKPiA+ID4gPiA+ID4gbWVzc2FnZXMsIEkgZG9uJ3QKPiA+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgID4gdGhpcyB3ZSBnb3QgaXQuIEFsc28sIHRoZSBleGlzdGluZyBrZXJuZWwg
bG9nCj4gPiA+ID4gPiA+IHNob3dzIG5vIHN1Y2ggaXNzdWUuCj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoCA+Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDC
oCA+Pj4gKwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqAgcmV0dXJuIGZhbHNl
Owo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICt9Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDC
oCA+Pj4gKwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+wqDCoCAvKgo+ID4gPiA+ID4gPiDC
oMKgwqDCoMKgwqAgPj4+wqDCoMKgICogTG9vayBmb3IgYSBidHJmcyBzaWduYXR1cmUgb24gYSBk
ZXZpY2UuCj4gPiA+ID4gPiA+IFRoaXMgbWF5IGJlIGNhbGxlZAo+ID4gPiA+ID4gPiDCoMKgwqDC
oMKgIG91dAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IG9mIHRoZSBtb3VudCBwYXRoCj4g
PiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj7CoMKgwqAgKiBhbmQgd2UgYXJlIG5vdCBhbGxvd2Vk
IHRvIGNhbGwKPiA+ID4gPiA+ID4gc2V0X2Jsb2Nrc2l6ZSBkdXJpbmcgdGhlIHNjYW4uCj4gPiA+
ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gVGhlIHN1cGVyYmxvY2sKPiA+ID4gPiA+ID4gwqDCoMKg
wqDCoMKgID4+PiBAQCAtMTMxNiw2ICsxMzQxLDcgQEAgc3RydWN0IGJ0cmZzX2RldmljZQo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICpidHJmc19zY2FuX29uZV9kZXZpY2UoY29uc3QgY2hh
ciAqcGF0aCwKPiA+ID4gPiA+ID4gYmxrX21vZGVfdCBmbGFncywKPiA+ID4gPiA+ID4gwqDCoMKg
wqDCoMKgID4+PsKgwqDCoMKgwqDCoCBzdHJ1Y3QgYnRyZnNfZGV2aWNlICpkZXZpY2UgPSBOVUxM
Owo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+wqDCoMKgwqDCoMKgIHN0cnVjdCBiZGV2X2hh
bmRsZSAqYmRldl9oYW5kbGU7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj7CoMKgwqDCoMKg
wqAgdTY0IGJ5dGVuciwgYnl0ZW5yX29yaWc7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4g
K8KgwqDCoCBkZXZfdCBkZXZ0ID0gMDsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PsKgwqDC
oMKgwqDCoCBpbnQgcmV0Owo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+wqDCoMKgwqDCoMKg
IGxvY2tkZXBfYXNzZXJ0X2hlbGQoJnV1aWRfbXV0ZXgpOwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKg
wqAgPj4+IEBAIC0xMzU1LDE4ICsxMzgxLDE2IEBAIHN0cnVjdCBidHJmc19kZXZpY2UKPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4+PiAqYnRyZnNfc2Nhbl9vbmVfZGV2aWNlKGNvbnN0IGNoYXIg
KnBhdGgsCj4gPiA+ID4gPiA+IGJsa19tb2RlX3QgZmxhZ3MsCj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqDCoCA+Pj7CoMKgwqDCoMKgwqDCoMKgwqDCoCBnb3RvIGVycm9yX2JkZXZfcHV0Owo+ID4gPiA+
ID4gPiDCoMKgwqDCoMKgwqAgPj4+wqDCoMKgwqDCoMKgIH0KPiA+ID4gPiA+ID4gwqDCoMKgwqDC
oMKgID4+PiAtwqDCoMKgIGlmICghbW91bnRfYXJnX2RldiAmJgo+ID4gPiA+ID4gPiBidHJmc19z
dXBlcl9udW1fZGV2aWNlcyhkaXNrX3N1cGVyKQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgID09IDEg
JiYKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiAtwqDCoMKgwqDCoMKgwqAgIShidHJmc19z
dXBlcl9mbGFncyhkaXNrX3N1cGVyKSAmCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgQlRSRlNfU1VQ
RVJfRkxBR19TRUVESU5HKSkgewo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IC3CoMKgwqDC
oMKgwqDCoCBkZXZfdCBkZXZ0Owo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICvCoMKgwqAg
cmV0ID0gbG9va3VwX2JkZXYocGF0aCwgJmRldnQpOwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAg
Pj4+ICvCoMKgwqAgaWYgKHJldCkKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiArwqDCoMKg
wqDCoMKgwqAgYnRyZnNfd2FybihOVUxMLCAibG9va3VwIGJkZXYgZmFpbGVkCj4gPiA+ID4gPiA+
IGZvciBwYXRoICVzOiAlZCIsCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj4gK8KgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqAgcGF0aCwgcmV0KTsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKg
ID4+PiAtwqDCoMKgwqDCoMKgwqAgcmV0ID0gbG9va3VwX2JkZXYocGF0aCwgJmRldnQpOwo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+IERvIHdl
IGFjdHVhbGx5IG5lZWQgdGhpcyBjaGVjaz8gSXQgd2FzIGFkZGVkIHdpdGgKPiA+ID4gPiA+ID4g
dGhlIHBhdGNoCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgc2tpcHBpbmcgdGhlCj4gPiA+ID4gPiA+
IMKgwqDCoMKgwqDCoCA+PiByZWdpc3RyYXRpb24sIHNvIGl0J3MgdmFsaWRhdGluZyB0aGUgYmxv
Y2sgZGV2aWNlCj4gPiA+ID4gPiA+IGJ1dCBob3cgY2FuIHdlCj4gPiA+ID4gPiA+IMKgwqDCoMKg
wqAgcGFzcwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4gc29tZXRoaW5nIHRoYXQgaXMgbm90
IGEgdmFsaWQgYmxvY2sgZGV2aWNlPwo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4KPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4gRG8geW91IG1l
YW4gdG8gY2hlY2sgaWYgdGhlIGxvb2t1cF9iZGV2KCkgaXMKPiA+ID4gPiA+ID4gc3VjY2Vzc2Z1
bD8gSG0uIEl0Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgc2hvdWxkCj4gPiA+ID4gPiA+IMKgwqDC
oMKgwqDCoCA+IGJlIG9rYXkgbm90IHRvIGNoZWNrLCBidXQgd2UgZG8gdGhhdCBjb25zaXN0ZW50
bHkKPiA+ID4gPiA+ID4gaW4gb3RoZXIgcGxhY2VzLgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAg
Pgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4gQmVzaWRlcyB0aGVyZSdzIGEgY2FsbCB0byBi
ZGV2X29wZW5fYnlfcGF0aCgpCj4gPiA+ID4gPiA+IHRoYXQgaW4gdHVybiBkb2VzIHRoZQo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4gbG9va3VwX2JkZXYgc28gY2hlY2tpbmcgaXQgaGVyZSBp
cyByZWR1bmRhbnQuCj4gPiA+ID4gPiA+IEl0J3Mgbm90IHJlbGF0ZWQKPiA+ID4gPiA+ID4gwqDC
oMKgwqDCoCB0byB0aGUKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+IGZpeCBpdHNlbGYgYnV0
IEkgZGVsZXRlZCBpdCBpbiBteSBmaXguCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pgo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPiBPaCBuby4g
V2UgbmVlZCAlZGV2dCB0byBiZSBzZXQgYmVjYXVzZToKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKg
ID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4gSXQgd2lsbCBtYXRjaCBpZiB0aGF0IGRldmlj
ZSBpcyBhbHJlYWR5Cj4gPiA+ID4gPiA+IG1vdW50ZWQvc2Nhbm5lZC4KPiA+ID4gPiA+ID4gwqDC
oMKgwqDCoMKgID4gSXQgd2lsbCBhbHNvIGZyZWUgc3RhbGUgZW50cmllcy4KPiA+ID4gPiA+ID4g
wqDCoMKgwqDCoMKgID4KPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4gVGh4LCBBbmFuZAo+ID4g
PiA+ID4gPiDCoMKgwqDCoMKgwqAgPgo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IC3CoMKg
wqDCoMKgwqDCoCBpZiAocmV0KQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IC3CoMKgwqDC
oMKgwqDCoMKgwqDCoMKgIGJ0cmZzX3dhcm4oTlVMTCwgImxvb2t1cCBiZGV2Cj4gPiA+ID4gPiA+
IGZhaWxlZCBmb3IgcGF0aCAlczogJWQiLAo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IC3C
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAgcGF0aCwgcmV0KTsKPiA+ID4gPiA+
ID4gwqDCoMKgwqDCoMKgID4+PiAtwqDCoMKgwqDCoMKgwqAgZWxzZQo+ID4gPiA+ID4gPiDCoMKg
wqDCoMKgwqAgPj4+ICvCoMKgwqAgaWYgKGJ0cmZzX3NraXBfcmVnaXN0cmF0aW9uKGRpc2tfc3Vw
ZXIsCj4gPiA+ID4gPiA+IGRldnQsCj4gPiA+ID4gPiA+IMKgwqDCoMKgwqAgbW91bnRfYXJnX2Rl
dikpIHsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PiArwqDCoMKgwqDCoMKgwqAgcHJfZGVi
dWcoIkJUUkZTOiBza2lwIHJlZ2lzdGVyaW5nCj4gPiA+ID4gPiA+IHNpbmdsZSBub24tc2VlZAo+
ID4gPiA+ID4gPiDCoMKgwqDCoMKgIGRldmljZSAlc1xuIiwKPiA+ID4gPiA+ID4gwqDCoMKgwqDC
oMKgID4+PiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAgcGF0aCk7Cj4gPiA+ID4gPiA+IMKg
wqDCoMKgwqDCoCA+Pj4gK8KgwqDCoMKgwqDCoMKgIGlmIChkZXZ0KQo+ID4gPiA+ID4gPiDCoMKg
wqDCoMKgwqAgPj4+wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCBidHJmc19mcmVlX3N0YWxl
X2RldmljZXMoZGV2dCwKPiA+ID4gPiA+ID4gTlVMTCk7Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDC
oCA+Pj4gLQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+IC3CoMKgwqDCoMKgwqDCoCBwcl9k
ZWJ1ZygiQlRSRlM6IHNraXAgcmVnaXN0ZXJpbmcKPiA+ID4gPiA+ID4gc2luZ2xlIG5vbi1zZWVk
IGRldmljZQo+ID4gPiA+ID4gPiDCoMKgwqDCoMKgwqAgPj4+ICVzXG4iLCBwYXRoKTsKPiA+ID4g
PiA+ID4gwqDCoMKgwqDCoMKgID4+PsKgwqDCoMKgwqDCoMKgwqDCoMKgIGRldmljZSA9IE5VTEw7
Cj4gPiA+ID4gPiA+IMKgwqDCoMKgwqDCoCA+Pj7CoMKgwqDCoMKgwqDCoMKgwqDCoCBnb3RvIGZy
ZWVfZGlza19zdXBlcjsKPiA+ID4gPiA+ID4gwqDCoMKgwqDCoMKgID4+PsKgwqDCoMKgwqDCoCB9
Cj4gPiA+ID4gPiA+IAoK

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

[David Sterba]

On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> We skip device registration for a single device. However, we do not do
> that if the device is already mounted, as it might be coming in again
> for scanning a different path.
> 
> This patch is lightly tested; for verification if it fixes.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> I still have some unknowns about the problem. Pls test if this fixes
> the problem.

Seems that we have that tested by some reportes, I checked it in my VM
setup that it works (fstests and grub2-probe) so let's add it to
for-next. Please use the changelog from my patch that describes the
problem and then add description of how you fixed it. Thanks.

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

[Anand Jain]

On 2/12/24 20:29, David Sterba wrote:
> On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
>> We skip device registration for a single device. However, we do not do
>> that if the device is already mounted, as it might be coming in again
>> for scanning a different path.
>>
>> This patch is lightly tested; for verification if it fixes.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---

>> I still have some unknowns about the problem. Pls test if this fixes
>> the problem.

It is a mystery- we didn't see
    "BTRFS: skip registering single non-seed device %s\n",
in the kernel messages.

Do you have any clue?

> Seems that we have that tested by some reportes, I checked it in my VM
> setup that it works (fstests and grub2-probe) so let's add it to
> for-next. Please use the changelog from my patch that describes the
> problem and then add description of how you fixed it. Thanks.

Thxs for the verification. I'll revise, send out.

Thx, Anand

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

[David Sterba]

On Tue, Feb 13, 2024 at 06:05:53AM +0530, Anand Jain wrote:
> 
> 
> On 2/12/24 20:29, David Sterba wrote:
> > On Mon, Feb 05, 2024 at 07:45:05PM +0800, Anand Jain wrote:
> >> We skip device registration for a single device. However, we do not do
> >> that if the device is already mounted, as it might be coming in again
> >> for scanning a different path.
> >>
> >> This patch is lightly tested; for verification if it fixes.
> >>
> >> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> >> ---
> 
> >> I still have some unknowns about the problem. Pls test if this fixes
> >> the problem.
> 
> It is a mystery- we didn't see
>     "BTRFS: skip registering single non-seed device %s\n",
> in the kernel messages.
> 
> Do you have any clue?

It's printed by pr_debug(), so it may not be on by default. For my
testing I changed it to printk(KERN_ERR "...") as I do for all debugging
messages just to be sure, the message was printed.

It may be actually useful to print that on the 'info' level as we do for
the other scanning messages.