[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-nilfs
Subject:    Re: [PATCH 00/24] nilfs2: introduce debugging subsystem implementation
From:       Vyacheslav Dubeyko <slava () dubeyko ! com>
Date:       2013-06-18 9:43:30
Message-ID: 1371548610.2265.63.camel () slavad-ubuntu
[Download RAW message or body]

Hi Ryusuke,

On Tue, 2013-06-18 at 02:02 +0900, Ryusuke Konishi wrote:

[snip]
> 
> Looks like you are trying to reinvent the wheel.
> 

I don't think so. :) But trying to reinvent the wheel is in background
of the technical progress at whole, from my viewpoint. And old way
doesn't mean bad way. :)

> Please consider using trace events kernel API (See
> Documentation/trace/events.txt and include/trace/events/xxxx.h etc),
> and do not try to add own debug/tracing framework.  With the trace
> events framekwork, you will be able to add flexible and switchable
> debug functionalities without abusing NILFS2_DEBUG_XXXX kernel options
> and printk variants.
> 

So, I suppose that I need to describe my vision and my intentions in
more details.

Yes, I agree that tracepoints framework should be used. But I think that
tracepoints framework is more suitable for performance analysis than for
investigation of an issue by means of debugging. Debugging and
tracepoints frameworks have different goals, from my understanding. So,
I think that NILFS2 should have as debugging as tracepoints
opportunities. The debugging framework can be used for gathering
information about an issue's environment on reporters' side and for deep
issue investigation. The tracepoint framework can be used for
performance analysis. And, moreover, I am thinking about adding
tracepoints into NILFS2 and I am going to do it.

The main goal of this patch set is to provide a simple way to gather
information about the issue on reporter's side. In my current approach
it is possible to ask a reporter about collecting additional details by
means of such simple steps: (1) Enable kernel configuration options for
necessary subsystems and debugging output opportunities; (2) Rebuild the
kernel; (3) Restart kernel and reproduce the issue; (4) Send content of
system log by e-mail. I think that such way is really simple and
efficient.

Ok, you dislike the way of using kernel configuration options for
subsystems and using printk variants. One of the suggested way of
debugging output in this patch set is using
pr_debug()/print_hex_dump_bytes() methods. The behaviour of these
methods are controlled via writing to a control file in the 'debugfs'
filesystem. So, you can configure in very flexible way what will be
printed out (please, see Documentation/dynamic-debug-howto.txt). As a
result, it is possible to use only pr_debug()/print_hex_dump_bytes()
instead of printk() variants. Moreover, most of the suggested kernel
configuration options will be not necessary in such case.

This patch set suggests such debugging output opportunities: (1) output
with information about internal errors; (2) output with current values
of variables/function arguments; (3) output with hexdumps of on-disk or
in-core data structures; (4) dump stack output.

I think that information about internal errors that takes place in
NILFS2 driver is very important output. Such information can provide a
clue for understanding a possible reason of an issue and a way of
further investigation. But information about internal errors is
preliminary and rough knowledge that can be used for localization of the
error. So, from my point of view, it doesn't make sense to restrict such
output by any subsystem. As a result, I use simple printk() for this
output.

Debugging output and hexdump output are very important information also.
These outputs should be made by pr_debug()/print_hex_dump_bytes(), from
my point of view. I think that dump stack can be useful information in
some situations. But if it will be used only dynamic version of printk()
then flags of for subsystems will be unnecessary. As a result, there
isn't opportunity for restricting of the dump stack output.

So, as a resume, I think that NILFS2 needs in debugging subsystem and
tracepoints subsystem. It is a complementary subsystems, from my
viewpoint.

With the best regards,
Vyacheslav Dubeyko.

> As I usually comment to you, start small, with a simple but powerful
> change, and try to keep your patchset as simple as you can.

> Regards,
> Ryusuke Konishi
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]