Hello Tejun,
I'm adding Rob (author of TEST_DRIVER_REMOVE) to Cc in case if he
has interest in the topic.
On 11/29/2016 10:44 PM, Tejun Heo wrote:
> Hello, Vladimir.
>
> On Tue, Nov 29, 2016 at 10:04:14PM +0200, Vladimir Zapolskiy wrote:
>>> Not really. Is this from the unloading test config?
>>
>> Correct, I always get the warning with CONFIG_DEBUG_TEST_DRIVER_REMOVE
>> option enabled.
>>
>> I understand that a working solution might be just to disable the
>> option rather than handle this case, however because it is wanted
>> to test other drivers for potential errors also (e.g. the same ATA
>> controller driver regardless of the false positive in the ATA core),
>> in my opinion the issue should not be ignored.
>
> So, the problem is that CONFIG_DEBUG_TEST_DRIVER_REMOVE is introducing
> a condition which isn't otherwise possible, so it's triggering pseudo
> bugs. The solution here is to make CONFIG_DEBUG_TEST_DRIVER_REMOVE
> flush async calls before trying to remove the driver.
>
in case if I haven't tired you out yet, I verified you solution and it
works perfectly, there is no problem in ATA subsystem which I can point
out:
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d76cd97..a4feecf 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -384,6 +384,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
if (test_remove) {
test_remove = false;
+ async_synchronize_full();
+
if (dev->bus->remove)
dev->bus->remove(dev);
else if (drv->remove)
In my understanding the code under "if (test_remove)" branch should be
close to the code of __device_release_driver() function, but here it
is slightly different on purpose --- driver_allows_async_probing(drv)
returns false in case of the ATA controller driver(s), here async
probing is not a functional part of a driver, but it is embedded into
the ATA subsystem by means of generic async_port_probe(). Not sure if
__device_release_driver() or driver_allows_async_probing() should be
corrected respecting this case, and hence I'm not going to touch it.
>>> Control doesn't get passed to userland until async probings are
>>> flushed, so this shouldn't normally be possible.
>>>
>>
>> I'm not an expert in ATA, can you please show me the synchronization
>> point?
>
> async_synchronize_full() call in kernel_init() for booting and in
> delete_module() for module unloading.
>
Thank you for discussion and support.
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
