[PATCH] cifs: Do not enable debugging code by default

[PATCH] cifs: Do not enable debugging code by default

[Borislav Petkov]

Debugging code is generally disabled in the configurator since it is
only for devs and testers and not for the majority of the installations.

Cc: Joe Perches <joe@perches.com>
Cc: Jeff Layton <jlayton@redhat.com>
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs@vger.kernel.org
Signed-off-by: Borislav Petkov <bp@alien8.de>
---
 fs/cifs/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
index 21ff76c22a17..241a0e2853c5 100644
--- a/fs/cifs/Kconfig
+++ b/fs/cifs/Kconfig
@@ -124,7 +124,6 @@ config CIFS_ACL
 
 config CIFS_DEBUG
 	bool "Enable CIFS debugging routines"
-	default y
 	depends on CIFS
 	help
 	   Enabling this option adds helpful debugging messages to
-- 
1.8.1.rc2

Re: [PATCH] cifs: Do not enable debugging code by default

[Steve French]

I don't think taking out debugging makes sense - we need debugging a
lot more often than expected even to diagnose user errors (usually
relating to session establishment failures, since we are very limited
in how much information can be passed back in mount).   If we had
dynamic trace points for cifs and for common vfs entry points, or if
the distros shipped dual cifs modules (one with debugging and one
without), I wouldn't mind disabling it - but multiple times a month we
have to diagnose user problems (sometimes user errors, sometimes
server bugs etc.) by enabling debugging.

On Mon, Dec 17, 2012 at 3:38 AM, Borislav Petkov <bp@alien8.de> wrote:
> Debugging code is generally disabled in the configurator since it is
> only for devs and testers and not for the majority of the installations.
>
> Cc: Joe Perches <joe@perches.com>
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: Steve French <sfrench@samba.org>
> Cc: linux-cifs@vger.kernel.org
> Signed-off-by: Borislav Petkov <bp@alien8.de>
> ---
>  fs/cifs/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 21ff76c22a17..241a0e2853c5 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -124,7 +124,6 @@ config CIFS_ACL
>
>  config CIFS_DEBUG
>         bool "Enable CIFS debugging routines"
> -       default y
>         depends on CIFS
>         help
>            Enabling this option adds helpful debugging messages to
> --
> 1.8.1.rc2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

Steve

Re: [PATCH] cifs: Do not enable debugging code by default

[Jeff Layton]

On Mon, 17 Dec 2012 09:30:33 -0600
Steve French <smfrench@gmail.com> wrote:

> I don't think taking out debugging makes sense - we need debugging a
> lot more often than expected even to diagnose user errors (usually
> relating to session establishment failures, since we are very limited
> in how much information can be passed back in mount).   If we had
> dynamic trace points for cifs and for common vfs entry points, or if
> the distros shipped dual cifs modules (one with debugging and one
> without), I wouldn't mind disabling it - but multiple times a month we
> have to diagnose user problems (sometimes user errors, sometimes
> server bugs etc.) by enabling debugging.
> 
> On Mon, Dec 17, 2012 at 3:38 AM, Borislav Petkov <bp@alien8.de> wrote:
> > Debugging code is generally disabled in the configurator since it is
> > only for devs and testers and not for the majority of the installations.
> >
> > Cc: Joe Perches <joe@perches.com>
> > Cc: Jeff Layton <jlayton@redhat.com>
> > Cc: Steve French <sfrench@samba.org>
> > Cc: linux-cifs@vger.kernel.org
> > Signed-off-by: Borislav Petkov <bp@alien8.de>
> > ---
> >  fs/cifs/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> > index 21ff76c22a17..241a0e2853c5 100644
> > --- a/fs/cifs/Kconfig
> > +++ b/fs/cifs/Kconfig
> > @@ -124,7 +124,6 @@ config CIFS_ACL
> >
> >  config CIFS_DEBUG
> >         bool "Enable CIFS debugging routines"
> > -       default y
> >         depends on CIFS
> >         help
> >            Enabling this option adds helpful debugging messages to
> > --
> > 1.8.1.rc2
> >

I agree that we often need cifsFYI debug info in order to troubleshoot
problems. The question here though is whether compiling those in ought
to be the default.

In generic distro kernels, I think we do want that turned on even in
non-debug kernels. On an embedded arch, it's less compelling.

Might it make sense to couple this change with a change to the
CONFIG_CIFS_DEBUG help text? Something like:

"This option defaults to 'N', but enabling this is strongly encouraged
unless memory is severely constrained."

Then we can just try to make sure non-embedded distros enable it.
-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] cifs: Do not enable debugging code by default

[Borislav Petkov]

On Tue, Dec 18, 2012 at 07:13:17AM -0500, Jeff Layton wrote:
> I agree that we often need cifsFYI debug info in order to troubleshoot
> problems. The question here though is whether compiling those in ought
> to be the default.
>
> In generic distro kernels, I think we do want that turned on even in
> non-debug kernels. On an embedded arch, it's less compelling.
>
> Might it make sense to couple this change with a change to the
> CONFIG_CIFS_DEBUG help text? Something like:
>
> "This option defaults to 'N', but enabling this is strongly encouraged
> unless memory is severely constrained."

... or we make it default n if EMBEDDED, otherwise default y + changing
the help text?

> Then we can just try to make sure non-embedded distros enable it.

Yeah, I don't think distros look at the help text... or?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] cifs: Do not enable debugging code by default

[Jeff Layton]

On Tue, 18 Dec 2012 15:44:52 +0100
Borislav Petkov <bp@alien8.de> wrote:

> On Tue, Dec 18, 2012 at 07:13:17AM -0500, Jeff Layton wrote:
> > I agree that we often need cifsFYI debug info in order to troubleshoot
> > problems. The question here though is whether compiling those in ought
> > to be the default.
> >
> > In generic distro kernels, I think we do want that turned on even in
> > non-debug kernels. On an embedded arch, it's less compelling.
> >
> > Might it make sense to couple this change with a change to the
> > CONFIG_CIFS_DEBUG help text? Something like:
> >
> > "This option defaults to 'N', but enabling this is strongly encouraged
> > unless memory is severely constrained."
> 
> ... or we make it default n if EMBEDDED, otherwise default y + changing
> the help text?
> 
> > Then we can just try to make sure non-embedded distros enable it.
> 
> Yeah, I don't think distros look at the help text... or?
> 

Yeah, not many do, but presumably they'll set it once and forget about
it. Once someone straightens them out, they should rarely break.

OTOH, the value of defaulting to "N" is pretty low here. Maybe it would
just be best to just drop this patch and have the embedded folks turn
this off on their own.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] cifs: Do not enable debugging code by default

[Borislav Petkov]

On Tue, Dec 18, 2012 at 10:07:02AM -0500, Jeff Layton wrote:
> Yeah, not many do, but presumably they'll set it once and forget about
> it. Once someone straightens them out, they should rarely break.
>
> OTOH, the value of defaulting to "N" is pretty low here. Maybe it
> would just be best to just drop this patch and have the embedded folks
> turn this off on their own.

Yeah, sure.

I just thought you have this debugging code forked out of the driver and
it should be enabled only for when there are issues.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] cifs: Do not enable debugging code by default

[Steve French]

On Tue, Dec 18, 2012 at 9:19 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Dec 18, 2012 at 10:07:02AM -0500, Jeff Layton wrote:
>> Yeah, not many do, but presumably they'll set it once and forget about
>> it. Once someone straightens them out, they should rarely break.
>>
>> OTOH, the value of defaulting to "N" is pretty low here. Maybe it
>> would just be best to just drop this patch and have the embedded folks
>> turn this off on their own.
>
> Yeah, sure.

I am ok with disabling it for embedded, but probably easier to drop
it.   The real benefit will come when someone familiar with dynamic
trace points comes up with some good examples for file system(s) and
the VFS so we can reduce our dependency on static tracing.



-- 
Thanks,

Steve

vfs trace points?: was: cifs: Do not enable debugging code by default

[Joe Perches]

On Tue, 2012-12-18 at 09:33 -0600, Steve French wrote:
> On Tue, Dec 18, 2012 at 9:19 AM, Borislav Petkov <bp@alien8.de> wrote:
> > On Tue, Dec 18, 2012 at 10:07:02AM -0500, Jeff Layton wrote:
> >> Yeah, not many do, but presumably they'll set it once and forget about
> >> it. Once someone straightens them out, they should rarely break.
> >>
> >> OTOH, the value of defaulting to "N" is pretty low here. Maybe it
> >> would just be best to just drop this patch and have the embedded folks
> >> turn this off on their own.
> >
> > Yeah, sure.
> 
> I am ok with disabling it for embedded, but probably easier to drop
> it.   The real benefit will come when someone familiar with dynamic
> trace points comes up with some good examples for file system(s) and
> the VFS so we can reduce our dependency on static tracing.

I'm fine with requiring embedded to turn this off
where appropriate.

It seems that keeping cifs KERN_DEBUG output for
normal kernels is pretty cost free.  120KB ain't much
these days.

I do think that Al Viro & Ted Ts'o should be pinged
about their vfs tracing thoughts. (cc'd)

MIT seems to bounce my emails, so maybe you could ask
Ted directly.