block: add a sequence number to disks
Subsystems (2)
cover block: add a sequence number to disks
From: Matteo Croce <[email protected]>
With this series a monotonically increasing number is added to disks, precisely in the genhd struct, and it's exported in sysfs and uevent.
This helps the userspace correlate events for devices that reuse the same device, like loop.
The first patch is the core one, the 2..4 expose the information in different ways, while the last one increase the sequence number for loop devices at every attach.
# udevadm monitor -kp |grep -e ^DEVNAME -e ^DISKSEQ &
[1] 523
# losetup -fP 3part
[ 3698.615848] loop0: detected capacity change from 16384 to 0
DEVNAME=/dev/loop0
DISKSEQ=13
[ 3698.647189] loop0: p1 p2 p3
DEVNAME=/dev/loop0
DISKSEQ=13
DEVNAME=/dev/loop0p1
DISKSEQ=13
DEVNAME=/dev/loop0p2
DISKSEQ=13
DEVNAME=/dev/loop0p3
DISKSEQ=13
# losetup -fP 2part
[ 3705.170766] loop1: detected capacity change from 40960 to 0
DEVNAME=/dev/loop1
DISKSEQ=14
[ 3705.247280] loop1: p1 p2
DEVNAME=/dev/loop1
DISKSEQ=14
DEVNAME=/dev/loop1p1
DISKSEQ=14
DEVNAME=/dev/loop1p2
DISKSEQ=14
# ./getdiskseq /dev/loop*
/dev/loop0: 13
/dev/loop0p1: 13
/dev/loop0p2: 13
/dev/loop0p3: 13
/dev/loop1: 14
/dev/loop1p1: 14
/dev/loop1p2: 14
/dev/loop2: 5
/dev/loop3: 6
/dev/loop-control: Function not implemented
# grep . /sys/class/block/*/diskseq
/sys/class/block/loop0/diskseq:13
/sys/class/block/loop1/diskseq:14
/sys/class/block/loop2/diskseq:5
/sys/class/block/loop3/diskseq:6
/sys/class/block/ram0/diskseq:1
/sys/class/block/ram1/diskseq:2
/sys/class/block/vda/diskseq:7If merged, this feature will immediately used by the userspace:
https://github.com/systemd/systemd/issues/17469#issuecomment-762919781
Matteo Croce (5):
block: add disk sequence number
block: add ioctl to read the disk sequence number
block: refactor sysfs code
block: export diskseq in sysfs
loop: increment sequence number
Documentation/ABI/testing/sysfs-block | 12 ++++++++ block/genhd.c | 43 ++++++++++++++++++++++++--- block/ioctl.c | 2 ++ drivers/block/loop.c | 3 ++ include/linux/genhd.h | 2 ++ include/uapi/linux/fs.h | 1 + 6 files changed, 59 insertions(+), 4 deletions(-)
Signature · 1 line
2.29.2
PATCH 1/5 block: add disk sequence number
From: Matteo Croce <[email protected]>
Add a sequence number to the disk devices. This number is put in the uevent so userspace can correlate events when a driver reuses a device, like the loop one.
--- block/genhd.c | 19 +++++++++++++++++++ include/linux/genhd.h | 2 ++ 2 files changed, 21 insertions(+)
block/genhd.c +19 -0
@@ -1425,8 +1425,17 @@ static void disk_release(struct device *dev)blk_put_queue (disk->queue);kfree (disk); } + +static int block_uevent (struct device *dev,struct kobj_uevent_env *env) +{ +struct gendisk *disk =dev_to_disk (dev); + +return add_uevent_var (env,"DISKSEQ=%llu" , disk->diskseq); +} +struct class block_class = { .name ="block" , + .dev_uevent = block_uevent, }; static char *block_devnode(struct device *dev, umode_t *mode,
@@ -1601,6 +1610,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); +return disk; out_bdput:
@@ -2149,3 +2160,11 @@ static void disk_release_events(struct gendisk *disk)WARN_ON_ONCE (disk->ev && disk->ev->block !=1 );kfree (disk->ev); } + +void inc_diskseq (struct gendisk *disk) +{ +static atomic64_t diskseq; + + disk->diskseq =atomic64_inc_return (&diskseq); +} +EXPORT_SYMBOL_GPL (inc_diskseq);
include/linux/genhd.h +2 -0
@@ -177,6 +177,7 @@ struct gendisk {int node_id;struct badblocks *bb;struct lockdep_map lockdep_map; +u64 diskseq; };/*
@@ -335,6 +336,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,#endif /* CONFIG_SYSFS */ extern struct rw_semaphore bdev_lookup_sem; +extern void inc_diskseq (struct gendisk *disk);dev_t blk_lookup_devt (const char *name,int partno);void blk_request_module (dev_t devt);
Signature · 1 line
2.29.2
PATCH 2/5 block: add ioctl to read the disk sequence number
In reply to Matteo CroceFrom: Matteo Croce <[email protected]>
Add a new BLKGETDISKSEQ ioctl which retrieves the disk sequence number from the genhd structure.
--- block/ioctl.c | 2 ++ include/uapi/linux/fs.h | 1 + 2 files changed, 3 insertions(+)
block/ioctl.c +2 -0
@@ -460,6 +460,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode, BLKDEV_DISCARD_SECURE);case BLKZEROOUT :return blk_ioctl_zeroout (bdev, mode, arg); +case BLKGETDISKSEQ : +return put_u64 (argp, bdev->bd_disk->diskseq);case BLKREPORTZONE :return blkdev_report_zones_ioctl (bdev, mode, cmd, arg);case BLKRESETZONE :
include/uapi/linux/fs.h +1 -0
@@ -184,6 +184,7 @@ struct fsxattr {#define BLKSECDISCARD _IO(0x12,125)#define BLKROTATIONAL _IO(0x12,126)#define BLKZEROOUT _IO(0x12,127) +#define BLKGETDISKSEQ _IOR(0x12,128,__u64)/* * A jump here: 130-131 are reserved for zoned block devices * (see uapi/linux/blkzoned.h)
Signature · 1 line
2.29.2
PATCH 3/5 block: refactor sysfs code
In reply to Matteo CroceFrom: Matteo Croce <[email protected]>
Move the sysfs register code from a function named disk_add_events() to a new function named disk_add_sysfs(). Also, rename the attribute list with a more generic name than disk_events_attrs.
--- block/genhd.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
block/genhd.c +9 -4
@@ -41,6 +41,7 @@ static void disk_alloc_events(struct gendisk *disk);static void disk_add_events (struct gendisk *disk);static void disk_del_events (struct gendisk *disk);static void disk_release_events (struct gendisk *disk); +static void disk_add_sysfs (struct gendisk *disk);void set_capacity (struct gendisk *disk,sector_t sectors) {
@@ -760,6 +761,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,*/ WARN_ON_ONCE (!blk_get_queue (disk->queue)); +disk_add_sysfs (disk);disk_add_events (disk);blk_integrity_add (disk); }
@@ -2049,7 +2051,7 @@ static const DEVICE_ATTR(events_poll_msecs, 0644, disk_events_poll_msecs_show, disk_events_poll_msecs_store); -static const struct attribute *disk_events_attrs[] = { +static const struct attribute *disk_sysfs_attrs[] = { &dev_attr_events.attr, &dev_attr_events_async.attr, &dev_attr_events_poll_msecs.attr,
@@ -2120,13 +2122,16 @@ static void disk_alloc_events(struct gendisk *disk) disk->ev = ev; } -static void disk_add_events (struct gendisk *disk) +static void disk_add_sysfs (struct gendisk *disk) {/* FIXME: error handling */ -if (sysfs_create_files (&disk_to_dev (disk)->kobj, disk_events_attrs) <0 ) +if (sysfs_create_files (&disk_to_dev (disk)->kobj, disk_sysfs_attrs) <0 )pr_warn ("%s: failed to create sysfs files for events\n" , disk->disk_name); +} +static void disk_add_events (struct gendisk *disk) +{if (!disk->ev)return ;
@@ -2151,7 +2156,7 @@ static void disk_del_events(struct gendisk *disk)mutex_unlock (&disk_events_mutex); } -sysfs_remove_files (&disk_to_dev (disk)->kobj, disk_events_attrs); +sysfs_remove_files (&disk_to_dev (disk)->kobj, disk_sysfs_attrs); }static void disk_release_events (struct gendisk *disk)
Signature · 1 line
2.29.2
PATCH 4/5 block: export diskseq in sysfs
In reply to Matteo CroceFrom: Matteo Croce <[email protected]>
Add a new sysfs handle to export the new diskseq value. Place it in <sysfs>/block/<disk>/diskseq and document it.
--- Documentation/ABI/testing/sysfs-block | 12 ++++++++++++ block/genhd.c | 11 +++++++++++ 2 files changed, 23 insertions(+)
Documentation/ABI/testing/sysfs-block +12 -0
@@ -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 <[email protected]> +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 <[email protected]>
block/genhd.c +11 -0
@@ -1975,6 +1975,7 @@ static void disk_check_events(struct disk_events *ev,* events_async : list of events which can be detected w/o polling * (always empty, only for backwards compatibility) * events_poll_msecs : polling interval, 0: disable, -1: system default +* diskseq : disk sequence number, since boot */ static ssize_t __disk_events_show (unsigned int events,char *buf) {
@@ -2045,16 +2046,26 @@ static ssize_t disk_events_poll_msecs_store(struct device *dev,return count; } +static ssize_t diskseq_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct gendisk *disk =dev_to_disk (dev); + +return sprintf (buf,"%llu\n" , disk->diskseq); +} + static const DEVICE_ATTR(events, 0444, disk_events_show, NULL); static const DEVICE_ATTR(events_async, 0444, disk_events_async_show, NULL); static const DEVICE_ATTR(events_poll_msecs, 0644, disk_events_poll_msecs_show, disk_events_poll_msecs_store); +static const DEVICE_ATTR(diskseq, 0444, diskseq_show, NULL);static const struct attribute *disk_sysfs_attrs[] = { &dev_attr_events.attr, &dev_attr_events_async.attr, &dev_attr_events_poll_msecs.attr, + &dev_attr_diskseq.attr,NULL , };
Signature · 1 line
2.29.2
PATCH 5/5 loop: increment sequence number
In reply to Matteo CroceFrom: Matteo Croce <[email protected]>
On a very loaded system, if there are many events queued up from multiple attach/detach cycles, it's impossible to match them up with the LOOP_CONFIGURE or LOOP_SET_FD call, since we don't know where the position of our own association in the queue is[1].
Not even an empty uevent queue is a reliable indication that we already received the uevent we were waiting for, since with multi-partition block devices each partition's event is queued asynchronously and might be delivered later.
Increment the disk sequence number when setting or changing the backing file, so the userspace knows which backing file generated the event.
[1] https://github.com/systemd/systemd/issues/17469#issuecomment-762919781
--- drivers/block/loop.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/block/loop.c +3 -0
@@ -734,6 +734,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev, goto out_err;/* and ... switch */ +inc_diskseq (lo->lo_disk);blk_mq_freeze_queue (lo->lo_queue);mapping_set_gfp_mask (old_file->f_mapping, lo->old_gfp_mask); lo->lo_backing_file = file;
@@ -1122,6 +1123,8 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,if (error) goto out_unlock; +inc_diskseq (lo->lo_disk); +if (!(file->f_mode &FMODE_WRITE ) || !(mode &FMODE_WRITE ) || !file->f_op->write_iter) lo->lo_flags |=LO_FLAGS_READ_ONLY ;
Signature · 1 line
2.29.2
Quoted 11 lines · Matteo Croce
From: Matteo Croce <[email protected]>
With this series a monotonically increasing number is added to disks, precisely in the genhd struct, and it's exported in sysfs and uevent.
This helps the userspace correlate events for devices that reuse the same device, like loop.
The first patch is the core one, the 2..4 expose the information in different ways, while the last one increase the sequence number for loop devices at every attach.
Patch set looks excellent to me. This would be great to have for the systems project, as it would allow us to fix some major races around loop device allocation, that are relatively easily triggered on loaded systems.
Lennart
Quoted 77 lines · Matteo Croce
From: Matteo Croce <[email protected]>
With this series a monotonically increasing number is added to disks, precisely in the genhd struct, and it's exported in sysfs and uevent.
This helps the userspace correlate events for devices that reuse the same device, like loop.
The first patch is the core one, the 2..4 expose the information in different ways, while the last one increase the sequence number for loop devices at every attach.
# udevadm monitor -kp |grep -e ^DEVNAME -e ^DISKSEQ & [1] 523 # losetup -fP 3part [ 3698.615848] loop0: detected capacity change from 16384 to 0 DEVNAME=/dev/loop0 DISKSEQ=13 [ 3698.647189] loop0: p1 p2 p3 DEVNAME=/dev/loop0 DISKSEQ=13 DEVNAME=/dev/loop0p1 DISKSEQ=13 DEVNAME=/dev/loop0p2 DISKSEQ=13 DEVNAME=/dev/loop0p3 DISKSEQ=13 # losetup -fP 2part [ 3705.170766] loop1: detected capacity change from 40960 to 0 DEVNAME=/dev/loop1 DISKSEQ=14 [ 3705.247280] loop1: p1 p2 DEVNAME=/dev/loop1 DISKSEQ=14 DEVNAME=/dev/loop1p1 DISKSEQ=14 DEVNAME=/dev/loop1p2 DISKSEQ=14 # ./getdiskseq /dev/loop* /dev/loop0: 13 /dev/loop0p1: 13 /dev/loop0p2: 13 /dev/loop0p3: 13 /dev/loop1: 14 /dev/loop1p1: 14 /dev/loop1p2: 14 /dev/loop2: 5 /dev/loop3: 6 /dev/loop-control: Function not implemented # grep . /sys/class/block/*/diskseq /sys/class/block/loop0/diskseq:13 /sys/class/block/loop1/diskseq:14 /sys/class/block/loop2/diskseq:5 /sys/class/block/loop3/diskseq:6 /sys/class/block/ram0/diskseq:1 /sys/class/block/ram1/diskseq:2 /sys/class/block/vda/diskseq:7If merged, this feature will immediately used by the userspace:
https://github.com/systemd/systemd/issues/17469#issuecomment-762919781Matteo Croce (5):
block: add disk sequence number
block: add ioctl to read the disk sequence number
block: refactor sysfs code
block: export diskseq in sysfs
loop: increment sequence numberDocumentation/ABI/testing/sysfs-block | 12 ++++++++ block/genhd.c | 43 ++++++++++++++++++++++++--- block/ioctl.c | 2 ++ drivers/block/loop.c | 3 ++ include/linux/genhd.h | 2 ++ include/uapi/linux/fs.h | 1 + 6 files changed, 59 insertions(+), 4 deletions(-)
Quoted 3 lines
--
2.29.2
Hi,
Did anyone have a chance to look at this series?
Ideas or suggestions?
Regards,
--
per aspera ad upstream