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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
