Thank you for looking in to this.
On 11/24/20 2:02 PM, Cornelia Huck wrote:
On Tue, 24 Nov 2020 10:34:07 +0100
Vineeth Vijayan <vneethv@xxxxxxxxxxxxx> wrote:
'commit fa1a8c23eb7d ("s390: cio: Delay uevents for subchannels")'
introduced the uevent suppression of subchannels. Even though there
are reasons for wanting to delay the uevent, it also introduces
problems. i.e On some platforms (qemu), where the udev-rule for the
subchannel needs to do driver_override to bind the vfio-ccw driver
instead of io_subchannel driver, but the suppressed uevent is
generated only when the device is found on the subchannel. By the
time it generates the uevent, it makes it difficult for the vfio-ccw
udev-rules to work.
This patch removes the uevent-suppress logic from the css driver.
The ADD uevent will be generated when there is a valid subchannel
and not after binding the valid device. The uevent generates while
device_add() during css_sch_device_register() function.
Signed-off-by: Vineeth Vijayan <vneethv@xxxxxxxxxxxxx>
---
drivers/s390/cio/chsc_sch.c | 5 -----
drivers/s390/cio/css.c | 19 -------------------
drivers/s390/cio/device.c | 18 ------------------
drivers/s390/cio/eadm_sch.c | 5 -----
drivers/s390/cio/vfio_ccw_drv.c | 5 -----
5 files changed, 52 deletions(-)
While I really like that diffstat, I hope that this is actually safe
for userspace programs processing uevents. Previously, we generated
the ADD uevent only when all parts were setup and ready to use
(including a child ccw_device, for example). Now, the ADD uevent is
created earlier, before drivers have done their setup. Do existing
udev rules still work as expected? (...)
I am still working on this. I have done minimal tests as of now. Mostly
with the available LPARs/zVMs. And it looks ok. But, i know there are
many fragile setups out there and the change in the "timing" (The
uevents generated earlier) could be a potential issue on that. I will
keep you updated on my findings.
About this RFC, i just wanted make sure that we are on same page with
regards to the RFD that you shared.
@@ -1055,16 +1047,6 @@ static int io_subchannel_probe(struct subchannel *sch)
"attributes for subchannel "
"0.%x.%04x (rc=%d)\n",
sch->schid.ssid, sch->schid.sch_no, rc);
- /*
- * The console subchannel already has an associated ccw_device.
- * Throw the delayed uevent for the subchannel, register
- * the ccw_device and exit.
I would keep the comment that we already have a ccw_device here. I.e.
/*
* The console subchannel already has an associated ccw_device.
* Register it and exit.
*/
- */
- if (dev_get_uevent_suppress(&sch->dev)) {
- /* should always be the case for the console */
- dev_set_uevent_suppress(&sch->dev, 0);
- kobject_uevent(&sch->dev.kobj, KOBJ_ADD);
- }
cdev = sch_get_cdev(sch);
rc = ccw_device_add(cdev);
if (rc) {
(...)
