From: Matteo Croce <redacted>
Associating uevents with block devices in userspace is difficult and racy:
the uevent netlink socket is lossy, and on slow and overloaded systems has
a very high latency. Block devices do not have exclusive owners in
userspace, any process can set one up (e.g. loop devices). Moreover, device
names can be reused (e.g. loop0 can be reused again and again). A userspace
process setting up a block device and watching for its events cannot thus
reliably tell whether an event relates to the device it just set up or
another earlier instance with the same name.
Being able to set a UUID on a loop device would solve the race conditions.
But it does not allow to derive orderings from uevents: if you see a uevent
with a UUID that does not match the device you are waiting for, you cannot
tell whether it's because the right uevent has not arrived yet, or it was
already sent and you missed it. So you cannot tell whether you should wait
for it or not.
Being able to set devices up in a namespace would solve the race conditions
too, but it can work only if being namespaced is feasible in the first
place. Many userspace processes need to set devices up for the root
namespace, so this solution cannot always work.
Changing the loop devices naming implementation to always use
monotonically increasing device numbers, instead of reusing the lowest
free number, would also solve the problem, but it would be very disruptive
to userspace and likely break many existing use cases. It would also be
quite awkward to use on long-running machines, as the loop device name
would quickly grow to many-digits length.
Furthermore, this problem does not affect only loop devices - partition
probing is asynchronous and very slow on busy systems. It is very easy to
enter races when using LO_FLAGS_PARTSCAN and watching for the partitions to
show up, as it can take a long time for the uevents to be delivered after
setting them up.
Associating a unique, monotonically increasing sequential number to the
lifetime of each block device, which can be retrieved with an ioctl
immediately upon setting it up, allows to solve the race conditions with
uevents, and also allows userspace processes to know whether they should
wait for the uevent they need or if it was dropped and thus they should
move on.
This does not benefit only loop devices and block devices with multiple
partitions, but for example also removable media such as USB sticks or
cdroms/dvdroms/etc.
The first patch is the core one, the 2..4 expose the information in
different ways, and the last one makes the loop device generate a media
changed event upon attach, detach or reconfigure, so the sequence number
is increased.
If merged, this feature will immediately used by the userspace:
https://github.com/systemd/systemd/issues/17469#issuecomment-762919781
v3 -> v4:
- rebased on top of 5.13
- hook the seqnum increase into the media change event
- make the loop device raise media change events
- merge 1/6 and 5/6
- move the uevent part of 1/6 into a separate one
- drop the now unneeded sysfs refactor
- change 'diskseq' to a global static variable
- add more comments
- refactor commit messages
v2 -> v3:
- rebased on top of 5.13-rc7
- resend because it appeared archived on patchwork
v1 -> v2:
- increase seqnum on media change
- increase on loop detach
Matteo Croce (6):
block: add disk sequence number
block: add ioctl to read the disk sequence number
block: refactor sysfs code
block: export diskseq in sysfs
block: increment sequence number
loop: increment sequence number
Matteo Croce (5):
block: add disk sequence number
block: export the diskseq in uevents
block: add ioctl to read the disk sequence number
block: export diskseq in sysfs
loop: raise media_change event
Documentation/ABI/testing/sysfs-block | 12 ++++++++
block/disk-events.c | 3 ++
block/genhd.c | 44 +++++++++++++++++++++++++++
block/ioctl.c | 2 ++
drivers/block/loop.c | 20 ++++++++++++
drivers/block/loop.h | 1 +
include/linux/genhd.h | 2 ++
include/uapi/linux/fs.h | 1 +
8 files changed, 85 insertions(+)
--
2.31.1
From: Matteo Croce <redacted>
Associating uevents with block devices in userspace is difficult and racy:
the uevent netlink socket is lossy, and on slow and overloaded systems
has a very high latency.
Block devices do not have exclusive owners in userspace, any process can
set one up (e.g. loop devices). Moreover, device names can be reused
(e.g. loop0 can be reused again and again). A userspace process setting
up a block device and watching for its events cannot thus reliably tell
whether an event relates to the device it just set up or another earlier
instance with the same name.
Being able to set a UUID on a loop device would solve the race conditions.
But it does not allow to derive orderings from uevents: if you see a
uevent with a UUID that does not match the device you are waiting for,
you cannot tell whether it's because the right uevent has not arrived yet,
or it was already sent and you missed it. So you cannot tell whether you
should wait for it or not.
Associating a unique, monotonically increasing sequential number to the
lifetime of each block device, which can be retrieved with an ioctl
immediately upon setting it up, allows to solve the race conditions with
uevents, and also allows userspace processes to know whether they should
wait for the uevent they need or if it was dropped and thus they should
move on.
Additionally, increment the disk sequence number when the media change,
i.e. on DISK_EVENT_MEDIA_CHANGE event.
Signed-off-by: Matteo Croce <redacted>
---
block/disk-events.c | 3 +++
block/genhd.c | 25 +++++++++++++++++++++++++
include/linux/genhd.h | 2 ++
3 files changed, 30 insertions(+)
@@ -29,6 +29,23 @@staticstructkobject*block_depr;+/*+*Unique,monotonicallyincreasingsequentialnumberassociatedwithblock+*devicesinstances(i.e.incrementedeachtimeadeviceisattached).+*Associatingueventswithblockdevicesinuserspaceisdifficultandracy:+*theueventnetlinksocketislossy,andonslowandoverloadedsystemshas+*averyhighlatency.+*Blockdevicesdonothaveexclusiveownersinuserspace,anyprocesscanset+*oneup(e.g.loopdevices).Moreover,devicenamescanbereused(e.g.loop0+*canbereusedagainandagain).+*Auserspaceprocesssettingupablockdeviceandwatchingforitsevents+*cannotthusreliablytellwhetheraneventrelatestothedeviceitjustset+*uporanotherearlierinstancewiththesamename.+*Thissequentialnumberallowsuserspaceprocessestosolvethisproblem,and+*uniquelyassociateanueventtothelifetimetoadevice.+*/+staticatomic64_tdiskseq;+/* for extended dynamic devt allocation, currently only one major is used */#define NR_EXT_DEVT (1 << MINORBITS)staticDEFINE_IDA(ext_devt_ida);
@@ -1263,6 +1280,8 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)disk_to_dev(disk)->class=&block_class;disk_to_dev(disk)->type=&disk_type;device_initialize(disk_to_dev(disk));+inc_diskseq(disk);+returndisk;out_destroy_part_tbl:
@@ -1363,3 +1382,9 @@ int bdev_read_only(struct block_device *bdev)returnbdev->bd_read_only||get_disk_ro(bdev->bd_disk);}EXPORT_SYMBOL(bdev_read_only);++voidinc_diskseq(structgendisk*disk)+{+disk->diskseq=atomic64_inc_return(&diskseq);+}+EXPORT_SYMBOL_GPL(inc_diskseq);
@@ -28,6 +28,18 @@ Description: For more details refer Documentation/admin-guide/iostats.rst+What: /sys/block/<disk>/diskseq+Date: February 2021+Contact: Matteo Croce <mcroce@microsoft.com>+Description:+ The /sys/block/<disk>/diskseq files reports the disk+ sequence number, which is a monotonically increasing+ number assigned to every drive.+ Some devices, like the loop device, refresh such number+ every time the backing file is changed.+ The value type is 64 bit unsigned.++ What: /sys/block/<disk>/<part>/stat Date: February 2008 Contact: Jerome Marchand <jmarchan@redhat.com>
From: Christoph Hellwig <hch@infradead.org> Date: 2021-07-12 07:09:54
So when I said plug into the disk events I did not mean that you need
to use all the polling stuff - we can just do the parts required
which are the uevents and invalidation with a new little helper
like this (needs the actual prototype and kerneldoc of course):
Subject: [PATCH v4 3/5] block: add ioctl to read the disk sequence number
From: Matteo Croce <redacted>
Add a new BLKGETDISKSEQ ioctl which retrieves the disk sequence number
from the genhd structure.
Subject: [PATCH v4 3/5] block: add ioctl to read the disk sequence
number
From: Matteo Croce <redacted>
Add a new BLKGETDISKSEQ ioctl which retrieves the disk sequence
number from the genhd structure.
+ case BLKGETDISKSEQ:
+ return put_u64(argp, bdev->bd_disk->diskseq);
How does that work on a system in which int is 32 bits?
Hi,
what's the difference with this?
case BLKGETSIZE64:
return put_u64(argp, i_size_read(bdev->bd_inode));
The returned int is an error code, not the diskseq value itself, e.g.
case BLKFRAGET:
if (!argp)
return -EINVAL;
Regards,
--
per aspera ad upstream