Re: [PATCH] 9p: remove CONFIG_NET_9P_DEBUG option

[Eric Van Hensbergen]

On Wed, Aug 10, 2011 at 12:17 PM, Aneesh Kumar K.V
<aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 10 Aug 2011 07:24:56 -0500, Eric Van Hensbergen <ericvh@xxxxxxxxx> wrote:
>> On Wed, Aug 10, 2011 at 4:13 AM, Aneesh Kumar K.V
>> <aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote:
>> > On Mon,  1 Aug 2011 07:14:44 -0500, Alex Ray <alexjray.ncsu@xxxxxxxxx> 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@xxxxxxxx>
>> >
>> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/