Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1382530
Summary: | CONFIG_DEBUG_TEST_DRIVER_REMOVE breaks all the things | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Laura Abbott <labbott> |
Component: | kernel | Assignee: | Kernel Maintainer List <kernel-maint> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | gansalmon, ichavero, itamar, jonathan, kernel-maint, lersek, madhu.chinakonda, mchehab, rjones |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2018-04-06 18:17:40 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 910269 |
Description
Laura Abbott
2016-10-07 00:18:24 UTC
*** Bug 1382318 has been marked as a duplicate of this bug. *** The option is going to be turned off in the Oct 07 build This config option, added in commit bea5b158ff0da9c7246ff391f754f5f38e34577a Author: Rob Herring <robh> Date: Thu Aug 11 10:20:58 2016 -0500 driver core: add test of driver remove calls during probe In recent discussions on ksummit-discuss[1], it was suggested to do a sequence of probe, remove, probe for testing driver remove paths. This adds a kconfig option for said test. [1] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003459.html Suggested-by: Arnd Bergmann <arnd> Cc: Greg Kroah-Hartman <gregkh> Signed-off-by: Rob Herring <robh> Signed-off-by: Greg Kroah-Hartman <gregkh> seems to be a valid and useful development / cleanup tool; it's just that it probably shouldn't be enabled in production kernels as long as it exposes critical bugs in commonly used drivers. (Clearly the problems exist in those drivers, not in CONFIG_DEBUG_TEST_DRIVER_REMOVE.) Thank you for pinpointing this, Laura! I'll be sure to retest bug 1366842 with an aarch64 Rawhide nightly ISO after this BZ is addressed. Actually, Ard Biesheuvel pointed out to me that upstream commit bea5b158ff0d may not be correct: On 10/07/16 17:19, Ard Biesheuvel wrote: > they simply probe a driver twice even if it does not have a .remove() > method, so the feature itself may need some work as well. On 10/07/16 17:51, Ard Biesheuvel wrote: > I [...] noticed that the pci-host-generic driver does not have a remove > function, which the test code does not seem to deal with (quoted with permission) Looking at the patch: > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > index 98504ec99c7d..fdf44cac08e6 100644 > --- a/drivers/base/Kconfig > +++ b/drivers/base/Kconfig > @@ -212,6 +212,16 @@ config DEBUG_DEVRES > > If you are unsure about this, Say N here. > > +config DEBUG_TEST_DRIVER_REMOVE > + bool "Test driver remove calls during probe" > + depends on DEBUG_KERNEL > + help > + Say Y here if you want the Driver core to test driver remove functions > + by calling probe, remove, probe. This tests the remove path without > + having to unbind the driver or unload the driver module. > + > + If you are unsure about this, say N here. > + > config SYS_HYPERVISOR > bool > default n > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 16688f50729c..4910e6db2a34 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -329,6 +329,7 @@ static int really_probe(struct device *dev, struct device_driver *drv) > { > int ret = -EPROBE_DEFER; > int local_trigger_count = atomic_read(&deferred_trigger_count); > + bool test_remove = IS_ENABLED(CONFIG_DEBUG_TEST_DRIVER_REMOVE); > > if (defer_all_probes) { > /* > @@ -346,6 +347,7 @@ static int really_probe(struct device *dev, struct device_driver *drv) > drv->bus->name, __func__, drv->name, dev_name(dev)); > WARN_ON(!list_empty(&dev->devres_head)); > > +re_probe: > dev->driver = drv; > > /* If using pinctrl, bind pins now before probing */ > @@ -383,6 +385,25 @@ static int really_probe(struct device *dev, struct device_driver *drv) > goto probe_failed; > } > > + if (test_remove) { > + test_remove = false; > + > + if (dev->bus && dev->bus->remove) > + dev->bus->remove(dev); > + else if (drv->remove) > + drv->remove(dev); > + > + devres_release_all(dev); > + driver_sysfs_remove(dev); > + dev->driver = NULL; > + dev_set_drvdata(dev, NULL); > + if (dev->pm_domain && dev->pm_domain->dismiss) > + dev->pm_domain->dismiss(dev); > + pm_runtime_reinit(dev); > + > + goto re_probe; > + } > + > pinctrl_init_done(dev); > > if (dev->pm_domain && dev->pm_domain->sync) The code checks for the remove() member functions, but if none of those are available, it only skips calling them -- it does not skip releasing other resources, and also doesn't skip calling probe() on the device that is still bound. (In reply to Laura Abbott from comment #2) > The option is going to be turned off in the Oct 07 build dist-git commit cb6ef876a0ac2bdd3c2c3ca929a318919ff04c7d ("Disable CONFIG_DEBUG_TEST_DRIVER_REMOVE") My finest hour. |