mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: Alex Romosan <aromosan@gmail.com>, dsterba@suse.cz
Cc: Thorsten Leemhuis <regressions@leemhuis.info>,
	Qu Wenruo <quwenruo.btrfs@gmx.com>,
	linux-kernel@vger.kernel.org,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	Linux kernel regressions list <regressions@lists.linux.dev>
Subject: Re: commit 108cc873398932af589c295f78c348513b8d70d9 breaks update-grub on btrfs root filesystem
Date: Sat, 12 Sep 2026 09:12:20 +0930	[thread overview]
Message-ID: <8cf9fc40-3316-44c0-bf8e-bd8698a50a33@suse.com> (raw)
In-Reply-To: <CAKLYgeLAXC5qbNAWnS3KNCijuRT9mF_hz4e1fUexx+D-J5Bs4g@mail.gmail.com>



在 2026/9/12 04:20, Alex Romosan 写道:
> i agree it's a regression. one question (and i am not very familiar
> with the underlying principles), how do the other filesystems deal
> with this? shouldn't the registering of devices be abstracted out
> since i would assume every fs does it or is this something btrfs
> specific?

Other fses has the same problem on the reported device path.

The difference is other fses saves the device path at mount time and 
doesn't change.

E.g, the same weird device path can be utilized and reported for other 
fses too:

  # cd /dev/
  # mknod weird b 253 2
                  ^^^^^ The same major/minor number for
                        /dev/test/scratch1
  # mkfs.ext4 /dev/weird -F
  # mount /dev/weird /mnt/test
  # mount | tail - n1
  /dev/weird on /mnt/test type ext4 (rw,relatime)

The btrfs specific part is, btrfs can rename the device path on device 
rescan.

That introduced a new problem, should we trust the device path passed in 
for scan?

Normally a device scan is triggered by udev rules, so normally it's sane 
names like "/dev/sda1".
But end users can easily pass random/weird pass in that ioctl to force 
an update on device path, and we're back to the start point.

Furthermore when LVM gets involved, the same dm device can have multiple 
different softlinks.
And a wrong softlink scanned can easily screw a lot of mount detection, 
aka, screw up the most common tool we use, fstests.

Then we can easily have a weird situation where the same device is 
scanned again and again with different names (e.g. "/dev/dm-2" and 
"/dev/test/scratch1"), causing btrfs to report different device name 
depending on the timing.


Another situation is, someone is passing a completely weird name, which 
may not even be accessible by other processes.
The worst situation here is "/proc/self/fd/3", which can be a softlink 
to a block device, but only accessible by that exact process.


One solution to this complex corner case is, to do a proper path 
resolution, and check if the existing recorded path can be accessible, 
if not replace it with the newer one.

However that is not perfect either, firstly related to namespace, the 
device scan can be triggered inside a namespace, and again the path can 
only be accessible inside a certain namespace.

Secondly such path resolution may involve btrfs itself, e.g. the block 
file is not in devfs, but a directory inside btrfs.
Then scan and path resolution may lock the same inode, causing deadlock.

All the history can be found in this patch:

https://lore.kernel.org/linux-btrfs/5e65d9ba5927b4b6985ce819e9e49d082c6e9b45.1789112424.git.wqu@suse.com/T/#u


Now back to the grub problem, firstly it's not causing anyone unable to 
boot, it is only causing the grub2-probe unable to determine where the 
device is.

The reason is for users who are not using initramfs, but direct kernel 
boot. In that case, the rootfs is always using the name "/dev/root".

The lack of initramfs means we do not have proper devfs at boot time, so 
kernel is using that "/dev/root" for rootfs.

But after the system is fully up, a proper devfs is mounted at "/dev/", 
so the older temporary "/dev/root" is no longer accessible.

I can argue that the user space should not really trust the device path 
reported by mount, but utilize the device number reported for the mount 
point. E.g "mountpoint -d", then go through the "/dev/" or libblkid to 
grab the real device.

In fact, even using that weird name, other tools like lsblk can properly 
detect the real device without being confused by the name:

|-test-scratch1 253:2    0   10G  0 lvm  /mnt/test

> 
> On Fri, Sep 11, 2026 at 8:21 PM David Sterba <dsterba@suse.cz> wrote:
>>
>> On Thu, Sep 10, 2026 at 12:39:25PM +0200, Thorsten Leemhuis wrote:
>>> On 9/9/26 03:13, Qu Wenruo wrote:
>>>> 在 2026/9/9 09:55, David Sterba 写道:
>>>>> On Tue, Sep 08, 2026 at 11:59:43PM +0200, Alex Romosan wrote:
>>>>>>
>>>>>> [...] Since version 7.3-rc1 i haven't been able to to a grub-update,
>>>>>> instead i get this error:
>>>>>>
>>>>>> /usr/sbin/grub-probe: error: cannot find a device for / (is /dev
>>>>>> mounted?).
>>>>>>
>>>>>> 7.2 is fine. a git-bisect identified commit
>>>>>> 108cc873398932af589c295f78c348513b8d70d9 as being the culprit.
>>>>>> reverting this commit from 7.3-rc2 allowed me to run grub-update
>>>>>> again.
>>>>>>
>>>>>> this is not the first time i reported grub-update being broken on
>>>>>> btrfs. i reported exactly the same problem on jan 8, 2024
>>>>>> (https://lkml.iu.edu/hypermail/linux/kernel/2401.1/00596.html). maybe
>>>>>> the discussion that followed would help come up with a fix that will
>>>>>> make everybody happy.
>>>>>
>>>>> I remember debugging that one, https://bugzilla.kernel.org/
>>>>> show_bug.cgi?id=218353
>>>>> Reverting 108cc8733989 ("btrfs: fix a lockdep caused by path resolution
>>>>> during device scan") would bring back the lockdep warning and there is a
>>>>> locking problem.
>>>>>
>>>>> The commit says it's fixing 2e8b6bc0ab41 ("btrfs: avoid unnecessary
>>>>> device path update for the same device"), the difference is in lines
>>>>>
>>>>> (https://bugzilla.suse.com/show_bug.cgi?id=1230641)
>>>>>
>>>>> -       } else if (!device->name || strcmp(device->name->str, path)) {
>>>>> +    } else if (!device->name || !is_same_device(device, path)) {
>>>>>
>>>>> Which gets changed to (by 108cc8733989):
>>>>>
>>>>> -       } else if (!device->name || !is_same_device(device, path)) {
>>>>> +       } else if (!device->name || device->devt != path_devt) {
>>>>>
>>>>> Each change is reaction to a bug, I don't see a clear fix which will
>>>>> make it work in all cases.
>>>>
>>>> And I want to add that, the previous path based comparison is also
>>>> problematic for namespaces/weird block device names.
>>>>
>>>> Although not common, it's definitely possible to map weird block file
>>>> name into a namespace.
>>>>
>>>> Thus the path based comparison is not reliable in the first place, no to
>>>> mention the later lockdep problems.
>>>
>>> Well, but our #1 is "no regressions". And the recent change while fixing
>>> bugs clearly causes one, as Alex's report is afaics at least the third
>>> about it; the two earlier ones can be found here:
>>>
>>> https://lore.kernel.org/linux-btrfs/dfbe1e27-dab8-4d55-8cf3-0b28eeac5df4@gmail.com/
>>> https://lore.kernel.org/linux-btrfs/018a9738-1d4a-43a0-9352-a56d1e541364@gmail.com/
>>> Plus a repost of the latter here:
>>> https://lore.kernel.org/linux-btrfs/dfbe1e27-dab8-4d55-8cf3-0b28eeac5df4@gmail.com/
>>
>> I'm going to treat this as a regression. And for the record Qu and me
>> are in disagreement on that. My target is to make the systems boot again
>> first, the fix may leave some problematic case (like mentioned, devices
>> in namespaces), but that is probably a lesser problem.

It's not not-booting, but just grub2-probe unable to determine which 
device it really has, and so far only affects users without a initramfs.

But if you really want to revert back to fix this particular case, then 
allow the old bad random device rename, back to the starting point of 
the cat-mice game:


diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4ddabadc9188..f4b36ad1282d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -749,6 +749,18 @@ const u8 *btrfs_sb_fsid_ptr(const struct 
btrfs_super_block *sb)
         return has_metadata_uuid ? sb->metadata_uuid : sb->fsid;
  }

+static int device_name_cmp(struct btrfs_device *device, const char *path)
+{
+       const char *old_name;
+       int ret;
+
+       rcu_read_lock();
+       old_name = rcu_dereference(device->name);
+       ret = strcmp(old_name, path);
+       rcu_read_unlock();
+       return ret;
+}
+
  /*
   * Add new device to list of registered devices
   *
@@ -869,7 +881,7 @@ static noinline struct btrfs_device 
*device_list_add(const char *path,
                                 MAJOR(path_devt), MINOR(path_devt),
                                 current->comm, task_pid_nr(current));

-       } else if (!device->name || device->devt != path_devt) {
+       } else if (!device->name || device_name_cmp(device, path)) {
                 const char *old_name;

                 /*

  reply	other threads:[~2026-09-11 23:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 21:59 Alex Romosan
2026-09-08 22:22 ` Hanabishi
2026-09-09  0:25 ` David Sterba
2026-09-09  1:13   ` Qu Wenruo
2026-09-10 10:39     ` Thorsten Leemhuis
2026-09-11 18:21       ` David Sterba
2026-09-11 18:50         ` Alex Romosan
2026-09-11 23:42           ` Qu Wenruo [this message]
2026-09-09 14:18   ` Hanabishi
2026-09-09 21:58     ` Qu Wenruo
2026-09-09 23:35       ` Hanabishi
2026-09-10  0:02         ` Qu Wenruo
2026-09-11 18:11       ` David Sterba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8cf9fc40-3316-44c0-bf8e-bd8698a50a33@suse.com \
    --to=wqu@suse.com \
    --cc=aromosan@gmail.com \
    --cc=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.com \
    --cc=regressions@leemhuis.info \
    --cc=regressions@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®