2005-11-09 14:08:17

by Jan Beulich

[permalink] [raw]
Subject: [PATCH 14/39] NLKD - kernel trace buffer access

Debug extension implementation for NLKD to access the kernel trace
buffer.

Signed-Off-By: Jan Beulich <[email protected]>

(actual patch attached)


Attachments:
(No filename) (151.00 B)
linux-2.6.14-nlkd-printk.patch (4.88 kB)
Download all attachments

2005-11-10 05:45:00

by Keith Owens

[permalink] [raw]
Subject: Re: [PATCH 14/39] NLKD - kernel trace buffer access

On Wed, 09 Nov 2005 15:09:13 +0100,
"Jan Beulich" <[email protected]> wrote:
>Debug extension implementation for NLKD to access the kernel trace
>buffer.

printk.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 187 insertions(+)

This is complete overkill in printk.c. The only change required to
printk is to add a routine which gets the parameters that define the
buffer, see below from KDB. The rest of the code in your patch belongs
in the debugger, not in printk.

#ifdef CONFIG_KDB
/* kdb dmesg command needs access to the syslog buffer. do_syslog() uses locks
* so it cannot be used during debugging. Just tell kdb where the start and
* end of the physical and logical logs are. This is equivalent to do_syslog(3).
*/
void kdb_syslog_data(char *syslog_data[4])
{
syslog_data[0] = log_buf;
syslog_data[1] = log_buf + log_buf_len;
syslog_data[2] = log_buf + log_end - (logged_chars < log_buf_len ? logged_chars : log_buf_len);
syslog_data[3] = log_buf + log_end;
}
#endif /* CONFIG_KDB */

2005-11-10 08:01:32

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH 14/39] NLKD - kernel trace buffer access

>>> Keith Owens <[email protected]> 10.11.05 06:44:52 >>>
>On Wed, 09 Nov 2005 15:09:13 +0100,
>"Jan Beulich" <[email protected]> wrote:
>>Debug extension implementation for NLKD to access the kernel trace
>>buffer.
>
> printk.c | 187
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 187 insertions(+)
>
>This is complete overkill in printk.c. The only change required to
>printk is to add a routine which gets the parameters that define the
>buffer, see below from KDB. The rest of the code in your patch
belongs
>in the debugger, not in printk.

This depends on the perspective...

>#ifdef CONFIG_KDB
>/* kdb dmesg command needs access to the syslog buffer. do_syslog()
uses locks
> * so it cannot be used during debugging. Just tell kdb where the
start and
> * end of the physical and logical logs are. This is equivalent to
do_syslog(3).
> */
>void kdb_syslog_data(char *syslog_data[4])
>{
> syslog_data[0] = log_buf;
> syslog_data[1] = log_buf + log_buf_len;
> syslog_data[2] = log_buf + log_end - (logged_chars < log_buf_len
? logged_chars : log_buf_len);
> syslog_data[3] = log_buf + log_end;
>}
>#endif /* CONFIG_KDB */

The publishing of this function allows uncontrolled access to the
otherwise (and sure purposefully) static symbols; you could as well
globalize the symbols directly. In order for KDB to be a module, this
symbol would even need to be exported. By keeping the debugger access
code in the same file, nothing gets changed visibility-wise for the
outside world.

Further, the design of the debugger extensions of NLKD calls for the
extension code to live in the place their data gets controlled at.
Consider an extension living in a module - how could the debugger access
that information?

Jan