run_qemu.sh: do not automatically modprobe 'cxl_test' at boot time #185
Conversation
Boot time is not a good time to debug issues like pmem/ndctl#278 or any other really. `git -C ndctl/ grep 'modprobe.*cxl_test'` shows that pretty much every test already loads and unloads cxl_test by itself. If some exotic test managed to avoid doing this until today, then falling back in line is overdue. PS: the "depmod_load_..." variable names were wrong: this was relying on systemd, not depmod. Signed-off-by: Marc Herbert <[email protected]>
|
I don't know about this one. The history has been that when I select --cxl-test on the cmdline I get the cxl-test environment upon boot up and am ready to explore. I may be trying out any number of things in that environment, including, but not limited to running unit tests. Remember cxl-test is an environment, not only a unit test vehicle. |
Even after this PR, you are only one, simple and obvious Before this PR, no choice: pmem/ndctl#278 crash (and others) immediately at boot time and very obscure how to "opt out". For weeks, I didn't even realize run_qemu.sh was responsible and really wondered how to disable this. |
|
The 'why' in this pull request is: "Boot time is not a good time to debug issues like pmem/ndctl#278 or any other really." Following the link to issue #278 the story doesn't get much crisper. Why does this change need to be made? |
Boot time is a very busy time, so it's impossible to focus on one thing at time. This means:
Making environment changes (log levels, tracing, init state,...) is also MUCH harder because it can require rebuilding the image as opposed to simply making the changes in place. Depending on the change desired, this can require advanced There's really nothing specific to this PR there. Applied to |
|
If this is a change to make debugging something else easier, why not do that on a debug branch and only commit a change like this to main when it's deemed required? FWIW adjusting the cxl dyndbg settings is useful for controlling the spew during boot and runtime. |
|
I'd added this as a convenience thing - agree with @AlisonSchofield in that a lot of times you just want to look at something in cxl_test manually, not just run the test suite, and for those times it is nice to have it pre loaded by the time you get to a shell. If it is causing actual problems (I'm not yet sure what the motivation for this is other than general cleanliness?) - sure removing it makes sense, but otherwise I think it's nice to have. I'll leave it to the folks who use this day to day for an ack/nack. |
There is indeed a big convenience difference. Both use cases are valid. But carrying this patch locally on every test system is pretty inconvenient and that's assuming you even know this patch exists. When you don't know it exists, the inconvenience is huge. On the other, |
This comment was marked as resolved.
This comment was marked as resolved.
That's because |
|
Interestingly, loading If I ever get bored I might add add a new run_qemu.sh option... From @davejiang:
Indeed, this warning gets printed 250 times: |
|
Just tested and confirmed that So if you accidentally try the wrong combination then
|
A bit off-topic sorry: this udev hotplug is even more evil than I expected: in my configuration, that rule was somehow enough to corrupt QEMU itself! I was booting with this rule first, deleting it and then restart Linux only (not QEMU) with a |
This file passes the entire ndctl test suite with a couple additional quirks like `meson setup -Ddocs=disabled` and deleting the evil udev hotplug rule discussed in pmem#185 (comment) Signed-off-by: Marc Herbert <[email protected]>
This file passes the entire ndctl test suite with a couple additional quirks like `meson setup -Ddocs=disabled` and deleting the evil udev hotplug rule discussed in #185 (comment) Signed-off-by: Marc Herbert <[email protected]>
3 participants
Boot time is not a good time to debug issues like
pmem/ndctl#278 or any other really.
git -C ndctl/ grep 'modprobe.*cxl_test'shows that pretty much every test already loads and unloads cxl_test by itself. If some exotic test managed to avoid doing this until today, then falling back in line is overdue.PS: the "depmod_load_..." variable names were wrong: this was relying on systemd, not depmod.