| Message ID | 80545243dec10a48562bf8a9b5d10b8ba6f16983.1709231441.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: forget removed devices | expand |
On Thu, Feb 29, 2024 at 10:36:55AM -0800, Boris Burkov wrote: > Now that btrfs supports forgetting devices that don't exist, we can add > a udev rule to take advantage of that. This avoids bad edge cases > with cached devices in multi-device filesystems without having to rescan > all the devices on every change. > > Signed-of-by: Boris Burkov <boris@bur.io> > --- > 64-btrfs-rm.rules | 7 +++++++ > Makefile | 2 +- > 2 files changed, 8 insertions(+), 1 deletion(-) > create mode 100644 64-btrfs-rm.rules > > diff --git a/64-btrfs-rm.rules b/64-btrfs-rm.rules > new file mode 100644 > index 000000000..852155d28 > --- /dev/null > +++ b/64-btrfs-rm.rules > @@ -0,0 +1,7 @@ Please add a comment that explains when and why this udev rule should be used. > +SUBSYSTEM!="block", GOTO="btrfs_rm_end" > +ACTION!="remove", GOTO="btrfs_rm_end" > +ENV{ID_FS_TYPE}!="btrfs", GOTO="btrfs_rm_end" > + > +RUN+="/usr/local/bin/btrfs device scan -u $devnode" Is the full path mandatory or is 'btrfs' sufficient? I think systemd uses own tool of the same name. Please use long option name so it's more obvious what it does.
On Thu, Feb 29, 2024 at 08:53:39PM +0100, David Sterba wrote: > On Thu, Feb 29, 2024 at 10:36:55AM -0800, Boris Burkov wrote: > > Now that btrfs supports forgetting devices that don't exist, we can add > > a udev rule to take advantage of that. This avoids bad edge cases > > with cached devices in multi-device filesystems without having to rescan > > all the devices on every change. > > > > Signed-of-by: Boris Burkov <boris@bur.io> > > --- > > 64-btrfs-rm.rules | 7 +++++++ > > Makefile | 2 +- > > 2 files changed, 8 insertions(+), 1 deletion(-) > > create mode 100644 64-btrfs-rm.rules > > > > diff --git a/64-btrfs-rm.rules b/64-btrfs-rm.rules > > new file mode 100644 > > index 000000000..852155d28 > > --- /dev/null > > +++ b/64-btrfs-rm.rules > > @@ -0,0 +1,7 @@ > > Please add a comment that explains when and why this udev rule should be > used. > Definitely happy to add a comment. This is certainly the discussion I was hoping to have, as well, but I thiiink we just always want this? Basically if we don't have it, multi-device users are in danger of accidentally making a stale device cache between mounts. It's probably not that big of a risk in general, but we did hit an easier to hit variant in v5.19 at Meta. OTOH, there is also the problem that this is a no-op unless the kernel has the patch I sent at the same time: btrfs: support device name lookup in forget I don't think there is any downside to running this command which will simply fail on an older kernel. If this becomes ubiquitous, then we can also remove the special case for single device cache clearing from the btrfs unmount code. > > +SUBSYSTEM!="block", GOTO="btrfs_rm_end" > > +ACTION!="remove", GOTO="btrfs_rm_end" > > +ENV{ID_FS_TYPE}!="btrfs", GOTO="btrfs_rm_end" > > + > > +RUN+="/usr/local/bin/btrfs device scan -u $devnode" > > Is the full path mandatory or is 'btrfs' sufficient? I think systemd > uses own tool of the same name. Unfortunately, it did not work for me. I saw logs saying /usr/lib/udev/rules.d/btrfs file not found or something like that in dmesg. I also considered the btrfs udev "builtin" but from experimenting and checking out the code, it looks like that only does device ready, not all device commands. > > Please use long option name so it's more obvious what it does.
diff --git a/64-btrfs-rm.rules b/64-btrfs-rm.rules new file mode 100644 index 000000000..852155d28 --- /dev/null +++ b/64-btrfs-rm.rules @@ -0,0 +1,7 @@ +SUBSYSTEM!="block", GOTO="btrfs_rm_end" +ACTION!="remove", GOTO="btrfs_rm_end" +ENV{ID_FS_TYPE}!="btrfs", GOTO="btrfs_rm_end" + +RUN+="/usr/local/bin/btrfs device scan -u $devnode" + +LABEL="btrfs_rm_end" diff --git a/Makefile b/Makefile index 374f59b99..eaeed9baf 100644 --- a/Makefile +++ b/Makefile @@ -271,7 +271,7 @@ tune_objects = tune/main.o tune/seeding.o tune/change-uuid.o tune/change-metadat all_objects = $(objects) $(cmds_objects) $(libbtrfs_objects) $(convert_objects) \ $(mkfs_objects) $(image_objects) $(tune_objects) $(libbtrfsutil_objects) -udev_rules = 64-btrfs-dm.rules 64-btrfs-zoned.rules +udev_rules = 64-btrfs-dm.rules 64-btrfs-zoned.rules 64-btrfs-rm.rules ifeq ("$(origin V)", "command line") BUILD_VERBOSE = $(V)