* [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
@ 2011-08-01 12:14 Alex Ray
2011-08-03 4:34 ` Aneesh Kumar K.V
2011-08-10 9:13 ` Aneesh Kumar K.V
0 siblings, 2 replies; 8+ messages in thread
From: Alex Ray @ 2011-08-01 12:14 UTC (permalink / raw)
To: Eric Van Hensbergen; +Cc: v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
functionality from v9fs. Logging is (already) controlled with the
run-time debug= option, this gets rid of the compile-time option (which
was being misunderstood and misused).
Signed-off-by: Alex Ray <ajray@ncsu.edu>
---
fs/9p/v9fs.c | 2 --
include/net/9p/9p.h | 6 ------
net/9p/Kconfig | 5 -----
net/9p/mod.c | 2 --
net/9p/protocol.c | 7 -------
5 files changed, 0 insertions(+), 22 deletions(-)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index ef96618..f1cb2e2 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -148,9 +148,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
switch (token) {
case Opt_debug:
v9ses->debug = option;
-#ifdef CONFIG_NET_9P_DEBUG
p9_debug_level = option;
-#endif
break;
case Opt_dfltuid:
diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
index 342dcf1..17cc437 100644
--- a/include/net/9p/9p.h
+++ b/include/net/9p/9p.h
@@ -61,7 +61,6 @@ enum p9_debug_flags {
P9_DEBUG_VPKT = (1<<12),
};
-#ifdef CONFIG_NET_9P_DEBUG
extern unsigned int p9_debug_level;
#define P9_DPRINTK(level, format, arg...) \
@@ -78,11 +77,6 @@ do { \
#define P9_DUMP_PKT(way, pdu) p9pdu_dump(way, pdu)
-#else
-#define P9_DPRINTK(level, format, arg...) do { } while (0)
-#define P9_DUMP_PKT(way, pdu) do { } while (0)
-#endif
-
#define P9_EPRINTK(level, format, arg...) \
do { \
diff --git a/net/9p/Kconfig b/net/9p/Kconfig
index d9ea09b..e1335e5 100644
--- a/net/9p/Kconfig
+++ b/net/9p/Kconfig
@@ -28,9 +28,4 @@ config NET_9P_RDMA
help
This builds support for an RDMA transport.
-config NET_9P_DEBUG
- bool "Debug information"
- help
- Say Y if you want the 9P subsystem to log debug information.
-
endif
diff --git a/net/9p/mod.c b/net/9p/mod.c
index 2664d12..4a3a303 100644
--- a/net/9p/mod.c
+++ b/net/9p/mod.c
@@ -34,12 +34,10 @@
#include <linux/list.h>
#include <linux/spinlock.h>
-#ifdef CONFIG_NET_9P_DEBUG
unsigned int p9_debug_level = 0; /* feature-rific global debug level */
EXPORT_SYMBOL(p9_debug_level);
module_param_named(debug, p9_debug_level, uint, 0);
MODULE_PARM_DESC(debug, "9P debugging level");
-#endif
/*
* Dynamic Transport Registration Routines
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index df58375..04b0585 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -40,7 +40,6 @@
static int
p9pdu_writef(struct p9_fcall *pdu, int proto_version, const char *fmt, ...);
-#ifdef CONFIG_NET_9P_DEBUG
void
p9pdu_dump(int way, struct p9_fcall *pdu)
{
@@ -63,12 +62,6 @@ p9pdu_dump(int way, struct p9_fcall *pdu)
print_hex_dump_bytes("]9P[ ", DUMP_PREFIX_OFFSET, pdu->sdata,
len);
}
-#else
-void
-p9pdu_dump(int way, struct p9_fcall *pdu)
-{
-}
-#endif
EXPORT_SYMBOL(p9pdu_dump);
void p9stat_free(struct p9_wstat *stbuf)
--
1.7.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-01 12:14 [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option Alex Ray
@ 2011-08-03 4:34 ` Aneesh Kumar K.V
2011-08-10 9:13 ` Aneesh Kumar K.V
1 sibling, 0 replies; 8+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-03 4:34 UTC (permalink / raw)
To: Alex Ray, Eric Van Hensbergen
Cc: v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
> functionality from v9fs. Logging is (already) controlled with the
> run-time debug= option, this gets rid of the compile-time option (which
> was being misunderstood and misused).
It would be nice to get those converted to trace points, that will help
us to enable/disable them easily, have low overhead when disabled.
>
> Signed-off-by: Alex Ray <ajray@ncsu.edu>
> ---
> fs/9p/v9fs.c | 2 --
> include/net/9p/9p.h | 6 ------
> net/9p/Kconfig | 5 -----
> net/9p/mod.c | 2 --
> net/9p/protocol.c | 7 -------
> 5 files changed, 0 insertions(+), 22 deletions(-)
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index ef96618..f1cb2e2 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -148,9 +148,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
> switch (token) {
> case Opt_debug:
> v9ses->debug = option;
> -#ifdef CONFIG_NET_9P_DEBUG
> p9_debug_level = option;
> -#endif
> break;
>
> case Opt_dfltuid:
> diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
> index 342dcf1..17cc437 100644
> --- a/include/net/9p/9p.h
> +++ b/include/net/9p/9p.h
> @@ -61,7 +61,6 @@ enum p9_debug_flags {
> P9_DEBUG_VPKT = (1<<12),
> };
>
> -#ifdef CONFIG_NET_9P_DEBUG
> extern unsigned int p9_debug_level;
>
> #define P9_DPRINTK(level, format, arg...) \
> @@ -78,11 +77,6 @@ do { \
>
> #define P9_DUMP_PKT(way, pdu) p9pdu_dump(way, pdu)
>
> -#else
> -#define P9_DPRINTK(level, format, arg...) do { } while (0)
> -#define P9_DUMP_PKT(way, pdu) do { } while (0)
> -#endif
> -
>
> #define P9_EPRINTK(level, format, arg...) \
> do { \
> diff --git a/net/9p/Kconfig b/net/9p/Kconfig
> index d9ea09b..e1335e5 100644
> --- a/net/9p/Kconfig
> +++ b/net/9p/Kconfig
> @@ -28,9 +28,4 @@ config NET_9P_RDMA
> help
> This builds support for an RDMA transport.
>
> -config NET_9P_DEBUG
> - bool "Debug information"
> - help
> - Say Y if you want the 9P subsystem to log debug information.
> -
> endif
> diff --git a/net/9p/mod.c b/net/9p/mod.c
> index 2664d12..4a3a303 100644
> --- a/net/9p/mod.c
> +++ b/net/9p/mod.c
> @@ -34,12 +34,10 @@
> #include <linux/list.h>
> #include <linux/spinlock.h>
>
> -#ifdef CONFIG_NET_9P_DEBUG
> unsigned int p9_debug_level = 0; /* feature-rific global debug level */
> EXPORT_SYMBOL(p9_debug_level);
> module_param_named(debug, p9_debug_level, uint, 0);
> MODULE_PARM_DESC(debug, "9P debugging level");
> -#endif
>
> /*
> * Dynamic Transport Registration Routines
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index df58375..04b0585 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -40,7 +40,6 @@
> static int
> p9pdu_writef(struct p9_fcall *pdu, int proto_version, const char *fmt, ...);
>
> -#ifdef CONFIG_NET_9P_DEBUG
> void
> p9pdu_dump(int way, struct p9_fcall *pdu)
> {
> @@ -63,12 +62,6 @@ p9pdu_dump(int way, struct p9_fcall *pdu)
> print_hex_dump_bytes("]9P[ ", DUMP_PREFIX_OFFSET, pdu->sdata,
> len);
> }
> -#else
> -void
> -p9pdu_dump(int way, struct p9_fcall *pdu)
> -{
> -}
> -#endif
> EXPORT_SYMBOL(p9pdu_dump);
>
> void p9stat_free(struct p9_wstat *stbuf)
> --
-aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-01 12:14 [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option Alex Ray
2011-08-03 4:34 ` Aneesh Kumar K.V
@ 2011-08-10 9:13 ` Aneesh Kumar K.V
2011-08-10 12:24 ` Eric Van Hensbergen
1 sibling, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-10 9:13 UTC (permalink / raw)
To: Alex Ray, Eric Van Hensbergen
Cc: v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
> functionality from v9fs. Logging is (already) controlled with the
> run-time debug= option, this gets rid of the compile-time option (which
> was being misunderstood and misused).
>
> Signed-off-by: Alex Ray <ajray@ncsu.edu>
I see this merged to for-next. Do we know whether enabling debug always have a
performance impact ?.
-aneesh
> ---
> fs/9p/v9fs.c | 2 --
> include/net/9p/9p.h | 6 ------
> net/9p/Kconfig | 5 -----
> net/9p/mod.c | 2 --
> net/9p/protocol.c | 7 -------
> 5 files changed, 0 insertions(+), 22 deletions(-)
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index ef96618..f1cb2e2 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -148,9 +148,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
> switch (token) {
> case Opt_debug:
> v9ses->debug = option;
> -#ifdef CONFIG_NET_9P_DEBUG
> p9_debug_level = option;
> -#endif
> break;
>
> case Opt_dfltuid:
> diff --git a/include/net/9p/9p.h b/include/net/9p/9p.h
> index 342dcf1..17cc437 100644
> --- a/include/net/9p/9p.h
> +++ b/include/net/9p/9p.h
> @@ -61,7 +61,6 @@ enum p9_debug_flags {
> P9_DEBUG_VPKT = (1<<12),
> };
>
> -#ifdef CONFIG_NET_9P_DEBUG
> extern unsigned int p9_debug_level;
>
> #define P9_DPRINTK(level, format, arg...) \
> @@ -78,11 +77,6 @@ do { \
>
> #define P9_DUMP_PKT(way, pdu) p9pdu_dump(way, pdu)
>
> -#else
> -#define P9_DPRINTK(level, format, arg...) do { } while (0)
> -#define P9_DUMP_PKT(way, pdu) do { } while (0)
> -#endif
> -
>
> #define P9_EPRINTK(level, format, arg...) \
> do { \
> diff --git a/net/9p/Kconfig b/net/9p/Kconfig
> index d9ea09b..e1335e5 100644
> --- a/net/9p/Kconfig
> +++ b/net/9p/Kconfig
> @@ -28,9 +28,4 @@ config NET_9P_RDMA
> help
> This builds support for an RDMA transport.
>
> -config NET_9P_DEBUG
> - bool "Debug information"
> - help
> - Say Y if you want the 9P subsystem to log debug information.
> -
> endif
> diff --git a/net/9p/mod.c b/net/9p/mod.c
> index 2664d12..4a3a303 100644
> --- a/net/9p/mod.c
> +++ b/net/9p/mod.c
> @@ -34,12 +34,10 @@
> #include <linux/list.h>
> #include <linux/spinlock.h>
>
> -#ifdef CONFIG_NET_9P_DEBUG
> unsigned int p9_debug_level = 0; /* feature-rific global debug level */
> EXPORT_SYMBOL(p9_debug_level);
> module_param_named(debug, p9_debug_level, uint, 0);
> MODULE_PARM_DESC(debug, "9P debugging level");
> -#endif
>
> /*
> * Dynamic Transport Registration Routines
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index df58375..04b0585 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -40,7 +40,6 @@
> static int
> p9pdu_writef(struct p9_fcall *pdu, int proto_version, const char *fmt, ...);
>
> -#ifdef CONFIG_NET_9P_DEBUG
> void
> p9pdu_dump(int way, struct p9_fcall *pdu)
> {
> @@ -63,12 +62,6 @@ p9pdu_dump(int way, struct p9_fcall *pdu)
> print_hex_dump_bytes("]9P[ ", DUMP_PREFIX_OFFSET, pdu->sdata,
> len);
> }
> -#else
> -void
> -p9pdu_dump(int way, struct p9_fcall *pdu)
> -{
> -}
> -#endif
> EXPORT_SYMBOL(p9pdu_dump);
>
> void p9stat_free(struct p9_wstat *stbuf)
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-10 9:13 ` Aneesh Kumar K.V
@ 2011-08-10 12:24 ` Eric Van Hensbergen
2011-08-10 17:17 ` Aneesh Kumar K.V
0 siblings, 1 reply; 8+ messages in thread
From: Eric Van Hensbergen @ 2011-08-10 12:24 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Alex Ray, v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
>> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
>> functionality from v9fs. Logging is (already) controlled with the
>> run-time debug= option, this gets rid of the compile-time option (which
>> was being misunderstood and misused).
>>
>> Signed-off-by: Alex Ray <ajray@ncsu.edu>
>
> I see this merged to for-next. Do we know whether enabling debug always have a
> performance impact ?.
>
No clue, but without any debug it makes it impossible for user's to
generate reasonable bug reports. If I understand the tracepoint
collection facility correctly, it incurs exactly the same overhead as
a DPRINT when the debug mount option is set to 0 (although tracepoints
are much lower overhead when actually collecting). Now, one could
make a case that we have too many DPRINT and need to cut back, but if
that's the case, let's just get around to it and cleanup a bit.
All that being said, I welcome anyone to send me performance with and
without CONFIG_NET_9P_DEBUG turned on to convince me differently.
-eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-10 12:24 ` Eric Van Hensbergen
@ 2011-08-10 17:17 ` Aneesh Kumar K.V
2011-08-10 18:36 ` Eric Van Hensbergen
0 siblings, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-10 17:17 UTC (permalink / raw)
To: Eric Van Hensbergen
Cc: Alex Ray, v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Wed, 10 Aug 2011 07:24:56 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >
> > On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
> >> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
> >> functionality from v9fs. Logging is (already) controlled with the
> >> run-time debug= option, this gets rid of the compile-time option (which
> >> was being misunderstood and misused).
> >>
> >> Signed-off-by: Alex Ray <ajray@ncsu.edu>
> >
> > I see this merged to for-next. Do we know whether enabling debug always have a
> > performance impact ?.
> >
>
> No clue, but without any debug it makes it impossible for user's to
> generate reasonable bug reports. If I understand the tracepoint
> collection facility correctly, it incurs exactly the same overhead as
> a DPRINT when the debug mount option is set to 0 (although tracepoints
> are much lower overhead when actually collecting).
I was worried about overhead when we are not collecting any debug info.
> Now, one could
> make a case that we have too many DPRINT and need to cut back, but if
> that's the case, let's just get around to it and cleanup a bit.
>
> All that being said, I welcome anyone to send me performance with and
> without CONFIG_NET_9P_DEBUG turned on to convince me differently.
With tracepoints we should not have much performance impact when tracing is
disabled. So may be the right way to go forward is to convert enough P9_DPRINT
to tracepoint that will aid in better debugging of errors, and retain
CONFIG_NET_9P_DEBUG as it is.
I actually converted protocol dump to tracepoints. Other advantages
of switching to tracepoints is the ability to get stack trace,
filtering debug output per mount points, integration with perf tool.
I guess if we can list out which set of P9_DPRINTK will aid better
reporting of bugs then we can look at converting them to
tracepoints. Protocol dump was the immediate one which I found useful.
-aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-10 17:17 ` Aneesh Kumar K.V
@ 2011-08-10 18:36 ` Eric Van Hensbergen
2011-08-11 15:54 ` Aneesh Kumar K.V
0 siblings, 1 reply; 8+ messages in thread
From: Eric Van Hensbergen @ 2011-08-10 18:36 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Alex Ray, v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Wed, Aug 10, 2011 at 12:17 PM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Wed, 10 Aug 2011 07:24:56 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
>> On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
>> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> > On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
>> >> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
>> >> functionality from v9fs. Logging is (already) controlled with the
>> >> run-time debug= option, this gets rid of the compile-time option (which
>> >> was being misunderstood and misused).
>> >>
>> >> Signed-off-by: Alex Ray <ajray@ncsu.edu>
>> >
>> > I see this merged to for-next. Do we know whether enabling debug always have a
>> > performance impact ?.
>> >
>>
>> No clue, but without any debug it makes it impossible for user's to
>> generate reasonable bug reports. If I understand the tracepoint
>> collection facility correctly, it incurs exactly the same overhead as
>> a DPRINT when the debug mount option is set to 0 (although tracepoints
>> are much lower overhead when actually collecting).
>
> I was worried about overhead when we are not collecting any debug info.
>
I understand that. But the overhead when not collecting is the
conditional branch.
According to Documentation/trace/tracepoints.txt this is the same for the
tracepoints:
"When a tracepoint is "off" it has no effect, except for adding a tiny
time penalty
(checking a condition for a branch) and space penalty (adding a few
bytes for the function call at the end of the instrumented function
and adds a data structure in a separate section)."
So, since DPRINT is essentially if(p9_debug_level & level) == level)
it should roughly amount to the same overhead, no? I suppose we could
get fancy and and prefix it with an unlikely.
> I actually converted protocol dump to tracepoints. Other advantages
> of switching to tracepoints is the ability to get stack trace,
> filtering debug output per mount points, integration with perf tool.
tracepoints definitely seem to be a win with the extra information, but
I worry about the average user being able to actually use the facility
to provide reasonable bug reports. Are tracepoints going to be enabled
in all stock kernels and their tools available by default in a distribution,
or is this something we are going to have to step people through?
The middle ground may be to include DPRINT's for the absolute
minimum info for identifying bugs (maybe just P9_DEBUG_ERROR,
P9_DEBUG_9P and/or P9_DEBUG_VFS class messages) and convert
everything else to tracepoints and maybe prune a bit as well.
So, to demonstrate that I'm not just a lazy maintainer, I actually ran
a sniff test using both TCP and Virtio before/after the debug flag
patch and saw no statistically relevant difference between the two runs
when running the Bonnie benchmark suite. (any variation was so low
as to be attributable to noise as measured by having several successive
runs with each version).
-eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-10 18:36 ` Eric Van Hensbergen
@ 2011-08-11 15:54 ` Aneesh Kumar K.V
2011-08-11 16:05 ` Steven Rostedt
0 siblings, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-11 15:54 UTC (permalink / raw)
To: Eric Van Hensbergen, Steven Rostedt
Cc: Alex Ray, v9fs-developer, linux-fsdevel, linux-kernel, Alex Ray
On Wed, 10 Aug 2011 13:36:52 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> On Wed, Aug 10, 2011 at 12:17 PM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > On Wed, 10 Aug 2011 07:24:56 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> >> On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
> >> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >> > On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
> >> >> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
> >> >> functionality from v9fs. Logging is (already) controlled with the
> >> >> run-time debug= option, this gets rid of the compile-time option (which
> >> >> was being misunderstood and misused).
> >> >>
> >> >> Signed-off-by: Alex Ray <ajray@ncsu.edu>
> >> >
> >> > I see this merged to for-next. Do we know whether enabling debug always have a
> >> > performance impact ?.
> >> >
> >>
> >> No clue, but without any debug it makes it impossible for user's to
> >> generate reasonable bug reports. If I understand the tracepoint
> >> collection facility correctly, it incurs exactly the same overhead as
> >> a DPRINT when the debug mount option is set to 0 (although tracepoints
> >> are much lower overhead when actually collecting).
> >
> > I was worried about overhead when we are not collecting any debug info.
> >
>
> I understand that. But the overhead when not collecting is the
> conditional branch.
> According to Documentation/trace/tracepoints.txt this is the same for the
> tracepoints:
>
> "When a tracepoint is "off" it has no effect, except for adding a tiny
> time penalty
> (checking a condition for a branch) and space penalty (adding a few
> bytes for the function call at the end of the instrumented function
> and adds a data structure in a separate section)."
>
> So, since DPRINT is essentially if(p9_debug_level & level) == level)
> it should roughly amount to the same overhead, no? I suppose we could
> get fancy and and prefix it with an unlikely.
>
Is that true with jump label ? May be we should update tracepoints.txt ?
Upstream commit:
bf5438fca2950b03c21ad868090cc1a8fcd49536
8f7b50c514206211cc282a4247f7b12f18dee674
-aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option
2011-08-11 15:54 ` Aneesh Kumar K.V
@ 2011-08-11 16:05 ` Steven Rostedt
0 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2011-08-11 16:05 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Eric Van Hensbergen, Alex Ray, v9fs-developer, linux-fsdevel,
linux-kernel, Alex Ray
On Thu, 2011-08-11 at 21:24 +0530, Aneesh Kumar K.V wrote:
> On Wed, 10 Aug 2011 13:36:52 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> > On Wed, Aug 10, 2011 at 12:17 PM, Aneesh Kumar K.V
> > <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > > On Wed, 10 Aug 2011 07:24:56 -0500, Eric Van Hensbergen <ericvh@gmail.com> wrote:
> > >> On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
> > >> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > >> > On Mon, 1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@gmail.com> wrote:
> > >> >> Remove the CONFIG_NET_9P_DEBUG option, used to completely remove logging
> > >> >> functionality from v9fs. Logging is (already) controlled with the
> > >> >> run-time debug= option, this gets rid of the compile-time option (which
> > >> >> was being misunderstood and misused).
> > >> >>
> > >> >> Signed-off-by: Alex Ray <ajray@ncsu.edu>
> > >> >
> > >> > I see this merged to for-next. Do we know whether enabling debug always have a
> > >> > performance impact ?.
> > >> >
> > >>
> > >> No clue, but without any debug it makes it impossible for user's to
> > >> generate reasonable bug reports. If I understand the tracepoint
> > >> collection facility correctly, it incurs exactly the same overhead as
> > >> a DPRINT when the debug mount option is set to 0 (although tracepoints
> > >> are much lower overhead when actually collecting).
> > >
> > > I was worried about overhead when we are not collecting any debug info.
> > >
> >
> > I understand that. But the overhead when not collecting is the
> > conditional branch.
> > According to Documentation/trace/tracepoints.txt this is the same for the
> > tracepoints:
> >
> > "When a tracepoint is "off" it has no effect, except for adding a tiny
> > time penalty
> > (checking a condition for a branch) and space penalty (adding a few
> > bytes for the function call at the end of the instrumented function
> > and adds a data structure in a separate section)."
> >
> > So, since DPRINT is essentially if(p9_debug_level & level) == level)
> > it should roughly amount to the same overhead, no? I suppose we could
> > get fancy and and prefix it with an unlikely.
> >
>
> Is that true with jump label ? May be we should update tracepoints.txt ?
Correct, that should be updated. When jump labels are enabled, we have a
nop and an unconditional branch over the trace code, which when enable
will change the nop to call the trace code.
We do have a slight hit in icache, but nothing I have been able to
measure. But if we do, I could even modify it to become even less of an
impact (but will cause tracing to slow down, but we don't really care if
it speeds up non tracing case ;)
-- Steve
>
> Upstream commit:
> bf5438fca2950b03c21ad868090cc1a8fcd49536
> 8f7b50c514206211cc282a4247f7b12f18dee674
>
> -aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-08-11 16:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-01 12:14 [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option Alex Ray
2011-08-03 4:34 ` Aneesh Kumar K.V
2011-08-10 9:13 ` Aneesh Kumar K.V
2011-08-10 12:24 ` Eric Van Hensbergen
2011-08-10 17:17 ` Aneesh Kumar K.V
2011-08-10 18:36 ` Eric Van Hensbergen
2011-08-11 15:54 ` Aneesh Kumar K.V
2011-08-11 16:05 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).