Hi Kaihenfeng,
Thanks for your patch suggestion! I'm semantically not sure it is the right
thing - to clarify your theory is that before it checked !resuming and before
had the check for !cdev maybe just to avoid a deference error. And now you
assume that instead of !cdev it should check if there is a cdev there.
I'm unsure - if !cdev was indeed just to protect the dereference then maybe no
check at all might be better. Which would then read "if the event is
IO_SCH_ORPH_UNREG or IO_SCH_UNREG then do css_sch_device_unregister.
But that I'm not immediately convinced doesn't mean much and it is easy
to test and surely worth a try, so I ran v5.11 (bad) plus your patch and
the result will be useful to know in any case. It is working fine, that
much I can tell you.
But if my thought above was right (it was only there to avoid the potential
deference error), then why check it at all. If the condition cdev==NULL is
possible it would now skip to to fully remove it - we might not need that at
all.
And Since I brought up the idea of dropping the cdev check entirely that was
worth a try as well. So now the third check of this morning is for:
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -1525,8 +1525,7 @@ static int io_subchannel_sch_event(struct subchannel
*sch, int process)
switch (action) {
case IO_SCH_ORPH_UNREG:
case IO_SCH_UNREG:
- if (!cdev)
- css_sch_device_unregister(sch);
+ css_sch_device_unregister(sch);
break;
case IO_SCH_ORPH_ATTACH:
case IO_SCH_UNREG_ATTACH:
My patch with that change - in my test - is working as well.
Neither of the solutions has triggered other regressions in my setup - but then
there are so many potential use-cases that I can't be sure without a further
revew by subject matter experts.
So a summary of the recent tests:
5.11.0-16-generic #17+lp1925211v202104201520 (Seths full revert) - working
5.11.0lp1925211-patch-kaihengfeng-dirty - working
5.11.0nocdevcheck-paelzer-dirty - working
I think we'd want an answer from the IBM devs which solution (full
revert, kaihenfeng patch, cpaelzer patch, another approach) they would
prefer - then we can submit it upstream for them to include officially
and we can carry it as delta until we rebase onto a version that has it
applied anyway.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8cc0dcfdc1c0e0be107d0288f9c0cf1f4201be62
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1925211
Title:
Hot-unplug of disks leaves broken block devices around in Hirsute on
s390x
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu-z-systems/+bug/1925211/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
