On 3/3/22 18:52, Uday Shankar wrote:
Userspace learns of disk ro state via the change event emitted by
set_disk_ro_uevent. This function has cyclic dependency with
device_add_disk: the latter performs kobject initialization that is
necessary for uevents to go through, but we want to set up properties
like ro state before exposing the disk to userspace via device_add_disk.
The usual workaround is to call set_disk_ro both before and after
device_add_disk; the purpose of the "after" call is just to emit the
uevent. Moreover, because set_disk_ro only emits a uevent when the ro
state changes, set_disk_ro needs to be called twice in the "after"
position to ensure that the ro state flips. See drivers/scsi/sd.c for an
example of this pattern.
The nvme driver does not implement this pattern. It only calls
set_disk_ro before device_add_disk, and so the ro uevent is never
emitted. This breaks applications such as dm-multipath. To avoid
introducing the messy pattern above into the nvme driver, emit the disk
ro uevent immediately after announcing addition of the disk.
Signed-off-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx>
---
block/genhd.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 11c761afd64f..89a110f0b002 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -394,6 +394,16 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
return 0;
}
+static void set_disk_ro_uevent(struct gendisk *gd, int ro)
+{
+ char event[] = "DISK_RO=1";
+ char *envp[] = { event, NULL };
+
+ if (!ro)
+ event[8] = '0';
+ kobject_uevent_env(&disk_to_dev(gd)->kobj, KOBJ_CHANGE, envp);
+}
+
/**
* device_add_disk - add disk information to kernel list
* @parent: parent device for the disk
@@ -522,6 +532,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
*/
dev_set_uevent_suppress(ddev, 0);
disk_uevent(disk, KOBJ_ADD);
+ set_disk_ro_uevent(disk, get_disk_ro(disk));
}
disk_update_readahead(disk);
@@ -1419,16 +1430,6 @@ void blk_cleanup_disk(struct gendisk *disk)
}
EXPORT_SYMBOL(blk_cleanup_disk);
-static void set_disk_ro_uevent(struct gendisk *gd, int ro)
-{
- char event[] = "DISK_RO=1";
- char *envp[] = { event, NULL };
-
- if (!ro)
- event[8] = '0';
- kobject_uevent_env(&disk_to_dev(gd)->kobj, KOBJ_CHANGE, envp);
-}
-
/**
* set_disk_ro - set a gendisk read-only
* @disk: gendisk to operate on
How very odd.
Why not add the 'DISK_RO=1' setting directly to the 'add' event?
That would be the logical thing to do, no?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
