CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

[Laura Abbott]

On a whim, I decided to turn on CONFIG_DEBUG_TEST_DRIVER_REMOVE on
Fedora rawhide since it sounded harmless enough. It spewed warnings
and panicked some systems. Clearly it's  doing its job
well of finding drivers that can't handle remove properly and I
underestimated it. I was expecting to maybe find a driver or two.
Can we get stronger Kconfig text indicating that this shouldn't be
turned on lightly? I'll be turning the option off in Fedora but sending
out reports from what was found.

Thanks,
Laura

Re: CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

[Greg Kroah-Hartman]

On Thu, Oct 06, 2016 at 04:53:20PM -0700, Laura Abbott wrote:
> On a whim, I decided to turn on CONFIG_DEBUG_TEST_DRIVER_REMOVE on
> Fedora rawhide since it sounded harmless enough. It spewed warnings
> and panicked some systems. Clearly it's  doing its job
> well of finding drivers that can't handle remove properly and I
> underestimated it.

Yes, we knew it was going to find bugs, you were brave :)

> I was expecting to maybe find a driver or two.
> Can we get stronger Kconfig text indicating that this shouldn't be
> turned on lightly? I'll be turning the option off in Fedora but sending
> out reports from what was found.

Care to send a patch with the wording change you would have found better
to warn yourself not to do this?

thanks,

greg k-h

Re: CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

[Laura Abbott]

On 10/06/2016 08:56 PM, Greg Kroah-Hartman wrote:
> On Thu, Oct 06, 2016 at 04:53:20PM -0700, Laura Abbott wrote:
>> On a whim, I decided to turn on CONFIG_DEBUG_TEST_DRIVER_REMOVE on
>> Fedora rawhide since it sounded harmless enough. It spewed warnings
>> and panicked some systems. Clearly it's  doing its job
>> well of finding drivers that can't handle remove properly and I
>> underestimated it.
>
> Yes, we knew it was going to find bugs, you were brave :)
>

Sure, let's go with brave. That makes me sound good ;)

>> I was expecting to maybe find a driver or two.
>> Can we get stronger Kconfig text indicating that this shouldn't be
>> turned on lightly? I'll be turning the option off in Fedora but sending
>> out reports from what was found.
>
> Care to send a patch with the wording change you would have found better
> to warn yourself not to do this?
>

Sure.

> thanks,
>
> greg k-h
>

Thanks,
Laura

Re: CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

[Rob Herring]

On Thu, Oct 6, 2016 at 6:53 PM, Laura Abbott <labbott@redhat.com> wrote:
> On a whim, I decided to turn on CONFIG_DEBUG_TEST_DRIVER_REMOVE on
> Fedora rawhide since it sounded harmless enough. It spewed warnings
> and panicked some systems. Clearly it's  doing its job
> well of finding drivers that can't handle remove properly and I
> underestimated it. I was expecting to maybe find a driver or two.
> Can we get stronger Kconfig text indicating that this shouldn't be
> turned on lightly? I'll be turning the option off in Fedora but sending
> out reports from what was found.

It hides behind CONFIG_DEBUG already. Is there a better option that
distros won't turn on?

Rob

Re: CONFIG_DEBUG_TEST_DRIVER_REMOVE needs a warning

[Laura Abbott]

On 10/06/2016 09:06 PM, Rob Herring wrote:
> On Thu, Oct 6, 2016 at 6:53 PM, Laura Abbott <labbott@redhat.com> wrote:
>> On a whim, I decided to turn on CONFIG_DEBUG_TEST_DRIVER_REMOVE on
>> Fedora rawhide since it sounded harmless enough. It spewed warnings
>> and panicked some systems. Clearly it's  doing its job
>> well of finding drivers that can't handle remove properly and I
>> underestimated it. I was expecting to maybe find a driver or two.
>> Can we get stronger Kconfig text indicating that this shouldn't be
>> turned on lightly? I'll be turning the option off in Fedora but sending
>> out reports from what was found.
>
> It hides behind CONFIG_DEBUG already. Is there a better option that
> distros won't turn on?

For Fedora rawhide, we tend to turn on debug options that don't have
a significant performance impact to hopefully make the kernel better
for Fedora stable releases. Apart from a more explicit Kconfig text,
you could hide it behind CONFIG_EXPERT which Fedora doesn't turn on
and is more of a hint that this should only be used in specific
circumstances.

>
> Rob
>

Thanks,
Laura

[PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Laura Abbott]

The current state of driver removal is not great.
CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
currently undersells exactly how many errors this option will find. Add
a bit more description to indicate this option shouldn't be turned on
unless you actually want to debug driver removal. The text can be
changed later when more drivers are fixed up.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 drivers/base/Kconfig | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index fdf44ca..d02e7c0 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -213,14 +213,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"
+	bool "Test driver remove calls during probe (UNSTABLE)"
 	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.
+	  This option is expected to find errors and may render your system
+	  unusable. You should say N here unless you are explicitly looking to
+	  test this functionality.
 
 config SYS_HYPERVISOR
 	bool
-- 
2.7.4

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Geert Uytterhoeven]

On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
> The current state of driver removal is not great.
> CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
> currently undersells exactly how many errors this option will find. Add
> a bit more description to indicate this option shouldn't be turned on
> unless you actually want to debug driver removal. The text can be
> changed later when more drivers are fixed up.

Indeed, this is failing miserably for e.g. SoC clock drivers using
platform_driver_probe(), which are never retried, rendering the complete
system useless.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Greg Kroah-Hartman]

On Mon, Oct 31, 2016 at 11:12:45AM +0100, Geert Uytterhoeven wrote:
> On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
> > The current state of driver removal is not great.
> > CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
> > currently undersells exactly how many errors this option will find. Add
> > a bit more description to indicate this option shouldn't be turned on
> > unless you actually want to debug driver removal. The text can be
> > changed later when more drivers are fixed up.
> 
> Indeed, this is failing miserably for e.g. SoC clock drivers using
> platform_driver_probe(), which are never retried, rendering the complete
> system useless.

Why are they never retried?

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Geert Uytterhoeven]

Hi Greg,

On Mon, Oct 31, 2016 at 1:21 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Oct 31, 2016 at 11:12:45AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
>> > The current state of driver removal is not great.
>> > CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
>> > currently undersells exactly how many errors this option will find. Add
>> > a bit more description to indicate this option shouldn't be turned on
>> > unless you actually want to debug driver removal. The text can be
>> > changed later when more drivers are fixed up.
>>
>> Indeed, this is failing miserably for e.g. SoC clock drivers using
>> platform_driver_probe(), which are never retried, rendering the complete
>> system useless.
>
> Why are they never retried?

Because platform_driver_probe() is meant for non-hotpluggable devices,
and unregisters the platform driver immediately if probe fails.
See also the comments for __platform_driver_probe():

 * Note that this is incompatible with deferred probing.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Rob Herring]

On Mon, Oct 31, 2016 at 7:28 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Greg,
>
> On Mon, Oct 31, 2016 at 1:21 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Mon, Oct 31, 2016 at 11:12:45AM +0100, Geert Uytterhoeven wrote:
>>> On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
>>> > The current state of driver removal is not great.
>>> > CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
>>> > currently undersells exactly how many errors this option will find. Add
>>> > a bit more description to indicate this option shouldn't be turned on
>>> > unless you actually want to debug driver removal. The text can be
>>> > changed later when more drivers are fixed up.
>>>
>>> Indeed, this is failing miserably for e.g. SoC clock drivers using
>>> platform_driver_probe(), which are never retried, rendering the complete
>>> system useless.
>>
>> Why are they never retried?
>
> Because platform_driver_probe() is meant for non-hotpluggable devices,
> and unregisters the platform driver immediately if probe fails.
> See also the comments for __platform_driver_probe():

My patch "driver core: skip removal test for non-removable drivers"
fixes this case. It seems to have dropped from Greg's queue, so I'll
resend.

Rob

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Greg Kroah-Hartman]

On Mon, Oct 31, 2016 at 09:10:57AM -0500, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 7:28 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > Hi Greg,
> >
> > On Mon, Oct 31, 2016 at 1:21 PM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> >> On Mon, Oct 31, 2016 at 11:12:45AM +0100, Geert Uytterhoeven wrote:
> >>> On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
> >>> > The current state of driver removal is not great.
> >>> > CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
> >>> > currently undersells exactly how many errors this option will find. Add
> >>> > a bit more description to indicate this option shouldn't be turned on
> >>> > unless you actually want to debug driver removal. The text can be
> >>> > changed later when more drivers are fixed up.
> >>>
> >>> Indeed, this is failing miserably for e.g. SoC clock drivers using
> >>> platform_driver_probe(), which are never retried, rendering the complete
> >>> system useless.
> >>
> >> Why are they never retried?
> >
> > Because platform_driver_probe() is meant for non-hotpluggable devices,
> > and unregisters the platform driver immediately if probe fails.
> > See also the comments for __platform_driver_probe():
> 
> My patch "driver core: skip removal test for non-removable drivers"
> fixes this case. It seems to have dropped from Greg's queue, so I'll
> resend.

Odd, I don't see that anywhere here, sorry about that...

Re: [PATCH] driver core: Make Kconfig text for DEBUG_TEST_DRIVER_REMOVE stronger

[Greg Kroah-Hartman]

On Mon, Oct 31, 2016 at 09:07:24AM -0600, Greg Kroah-Hartman wrote:
> On Mon, Oct 31, 2016 at 09:10:57AM -0500, Rob Herring wrote:
> > On Mon, Oct 31, 2016 at 7:28 AM, Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > Hi Greg,
> > >
> > > On Mon, Oct 31, 2016 at 1:21 PM, Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > >> On Mon, Oct 31, 2016 at 11:12:45AM +0100, Geert Uytterhoeven wrote:
> > >>> On Fri, Oct 7, 2016 at 6:09 PM, Laura Abbott <labbott@redhat.com> wrote:
> > >>> > The current state of driver removal is not great.
> > >>> > CONFIG_DEBUG_TEST_DRIVER_REMOVE finds lots of errors. The help text
> > >>> > currently undersells exactly how many errors this option will find. Add
> > >>> > a bit more description to indicate this option shouldn't be turned on
> > >>> > unless you actually want to debug driver removal. The text can be
> > >>> > changed later when more drivers are fixed up.
> > >>>
> > >>> Indeed, this is failing miserably for e.g. SoC clock drivers using
> > >>> platform_driver_probe(), which are never retried, rendering the complete
> > >>> system useless.
> > >>
> > >> Why are they never retried?
> > >
> > > Because platform_driver_probe() is meant for non-hotpluggable devices,
> > > and unregisters the platform driver immediately if probe fails.
> > > See also the comments for __platform_driver_probe():
> > 
> > My patch "driver core: skip removal test for non-removable drivers"
> > fixes this case. It seems to have dropped from Greg's queue, so I'll
> > resend.
> 
> Odd, I don't see that anywhere here, sorry about that...

Ah, found it, will queue it up now.