Mailing List Archive

Question about printk implementation.
I was looking over the implementation of printk (xenunstable) and I have a question about the spin_lock_recursive use and the static "buf" variable in this routine. Suppose that that in a single processor system the hypervisor is printing using this printk routine and has acquired the console_lock. Is it possible for a high level interrupt, like an NMI (just as an example, any other interrupts that preempt the current work in the hypervisor will do), to come in and then use printk. Because we are in single CPU mode, the spin_lock_recursive will succeed (if I interpreted the code correctly). When the higher level interrupt exits and returns to the hypervisor code that was previously printing, the contents of the static "buf" would have changed. Is this a possibility??

Thanks
Roger R. Cruz

void printk(const char *fmt, ...)
{
static char buf[1024]; <<<<<---------------
static int start_of_line = 1, do_print;

va_list args;
char *p, *q;
unsigned long flags;

/* console_lock can be acquired recursively from __printk_ratelimit(). */
local_irq_save(flags); <<<<-----------------
spin_lock_recursive(&console_lock);

va_start(args, fmt);
(void)vsnprintf(buf, sizeof(buf), fmt, args);
va_end(args);

p = buf;

while ( (q = strchr(p, '\n')) != NULL )
{
*q = '\0';
if ( start_of_line )
do_print = printk_prefix_check(p, &p);
if ( do_print )
{
if ( start_of_line )
printk_start_of_line();
__putstr(p);
__putstr("\n");
}
start_of_line = 1;
p = q + 1;
Re: Question about printk implementation. [ In reply to ]
On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:

> I was looking over the implementation of printk (xenunstable) and I have a
> question about the spin_lock_recursive use and the static "buf" variable in
> this routine. Suppose that that in a single processor system the hypervisor
> is printing using this printk routine and has acquired the console_lock. Is
> it possible for a high level interrupt, like an NMI (just as an example, any
> other interrupts that preempt the current work in the hypervisor will do), to
> come in and then use printk. Because we are in single CPU mode, the
> spin_lock_recursive will succeed (if I interpreted the code correctly). When
> the higher level interrupt exits and returns to the hypervisor code that was
> previously printing, the contents of the static "buf" would have changed. Is
> this a possibility??

Well yeah, you know what? Don't do that then! Our printk (like very many Xen
functions) is not NMI safe.

-- Keir

> Thanks
> Roger R. Cruz
>
> void printk(const char *fmt, ...)
> {
> static char buf[1024]; <<<<<---------------
> static int start_of_line = 1, do_print;
>
> va_list args;
> char *p, *q;
> unsigned long flags;
>
> /* console_lock can be acquired recursively from __printk_ratelimit(). */
> local_irq_save(flags); <<<<-----------------
> spin_lock_recursive(&console_lock);
>
> va_start(args, fmt);
> (void)vsnprintf(buf, sizeof(buf), fmt, args);
> va_end(args);
>
> p = buf;
>
> while ( (q = strchr(p, '\n')) != NULL )
> {
> *q = '\0';
> if ( start_of_line )
> do_print = printk_prefix_check(p, &p);
> if ( do_print )
> {
> if ( start_of_line )
> printk_start_of_line();
> __putstr(p);
> __putstr("\n");
> }
> start_of_line = 1;
> p = q + 1;
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: Question about printk implementation. [ In reply to ]
On 15/09/2010 22:03, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

> On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:
>
>> I was looking over the implementation of printk (xenunstable) and I have a
>> question about the spin_lock_recursive use and the static "buf" variable in
>> this routine. Suppose that that in a single processor system the hypervisor
>> is printing using this printk routine and has acquired the console_lock. Is
>> it possible for a high level interrupt, like an NMI (just as an example, any
>> other interrupts that preempt the current work in the hypervisor will do), to
>> come in and then use printk. Because we are in single CPU mode, the
>> spin_lock_recursive will succeed (if I interpreted the code correctly). When
>> the higher level interrupt exits and returns to the hypervisor code that was
>> previously printing, the contents of the static "buf" would have changed. Is
>> this a possibility??
>
> Well yeah, you know what? Don't do that then! Our printk (like very many Xen
> functions) is not NMI safe.

Oh, I see you are thinking of any arbitrary hardirq. Note that we
local_irq_save() before acquiring the console_lock. This has the effect of
disabling irqs while we hold the console_lock. So we cannot be interrupted
on the local CPU (and as I already said, although NMIs can still interrupt
the local CPU, we shouldn't use printk() in NMI context except in fatal
error circumstances where we are prepared to bust the console_lock first).

-- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
RE: Question about printk implementation. [ In reply to ]
Hi Keir,

Good to know that..the NMI handler (nmi_watchdog_tick) does have a printk but I supposed it is ok since we are going to reboot after, so we are not returning to the other code. I assume that putting a printk in a timer function callback (set up with init_timer & set_timer, etc) or any other interrupt handler would not be safe either then.

Roger



-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
Sent: Wed 9/15/2010 5:03 PM
To: Roger Cruz; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Question about printk implementation.

On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:

> I was looking over the implementation of printk (xenunstable) and I have a
> question about the spin_lock_recursive use and the static "buf" variable in
> this routine. Suppose that that in a single processor system the hypervisor
> is printing using this printk routine and has acquired the console_lock. Is
> it possible for a high level interrupt, like an NMI (just as an example, any
> other interrupts that preempt the current work in the hypervisor will do), to
> come in and then use printk. Because we are in single CPU mode, the
> spin_lock_recursive will succeed (if I interpreted the code correctly). When
> the higher level interrupt exits and returns to the hypervisor code that was
> previously printing, the contents of the static "buf" would have changed. Is
> this a possibility??

Well yeah, you know what? Don't do that then! Our printk (like very many Xen
functions) is not NMI safe.

-- Keir

> Thanks
> Roger R. Cruz
>
> void printk(const char *fmt, ...)
> {
> static char buf[1024]; <<<<<---------------
> static int start_of_line = 1, do_print;
>
> va_list args;
> char *p, *q;
> unsigned long flags;
>
> /* console_lock can be acquired recursively from __printk_ratelimit(). */
> local_irq_save(flags); <<<<-----------------
> spin_lock_recursive(&console_lock);
>
> va_start(args, fmt);
> (void)vsnprintf(buf, sizeof(buf), fmt, args);
> va_end(args);
>
> p = buf;
>
> while ( (q = strchr(p, '\n')) != NULL )
> {
> *q = '\0';
> if ( start_of_line )
> do_print = printk_prefix_check(p, &p);
> if ( do_print )
> {
> if ( start_of_line )
> printk_start_of_line();
> __putstr(p);
> __putstr("\n");
> }
> start_of_line = 1;
> p = q + 1;
>



No virus found in this incoming message.
Checked by AVG - www.avg.com
Version: 9.0.851 / Virus Database: 271.1.1/3134 - Release Date: 09/15/10 02:34:00
RE: Question about printk implementation. [ In reply to ]
OK. We crossed emails. You answered what I asked in the other.

Thanks a bunch.
Roger


-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
Sent: Wed 9/15/2010 5:07 PM
To: Roger Cruz; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Question about printk implementation.

On 15/09/2010 22:03, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

> On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:
>
>> I was looking over the implementation of printk (xenunstable) and I have a
>> question about the spin_lock_recursive use and the static "buf" variable in
>> this routine. Suppose that that in a single processor system the hypervisor
>> is printing using this printk routine and has acquired the console_lock. Is
>> it possible for a high level interrupt, like an NMI (just as an example, any
>> other interrupts that preempt the current work in the hypervisor will do), to
>> come in and then use printk. Because we are in single CPU mode, the
>> spin_lock_recursive will succeed (if I interpreted the code correctly). When
>> the higher level interrupt exits and returns to the hypervisor code that was
>> previously printing, the contents of the static "buf" would have changed. Is
>> this a possibility??
>
> Well yeah, you know what? Don't do that then! Our printk (like very many Xen
> functions) is not NMI safe.

Oh, I see you are thinking of any arbitrary hardirq. Note that we
local_irq_save() before acquiring the console_lock. This has the effect of
disabling irqs while we hold the console_lock. So we cannot be interrupted
on the local CPU (and as I already said, although NMIs can still interrupt
the local CPU, we shouldn't use printk() in NMI context except in fatal
error circumstances where we are prepared to bust the console_lock first).

-- Keir



No virus found in this incoming message.
Checked by AVG - www.avg.com
Version: 9.0.851 / Virus Database: 271.1.1/3134 - Release Date: 09/15/10 02:34:00
Re: Question about printk implementation. [ In reply to ]
On 15/09/2010 22:15, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:

> Hi Keir,
>
> Good to know that..the NMI handler (nmi_watchdog_tick) does have a printk but
> I supposed it is ok since we are going to reboot after, so we are not
> returning to the other code.

Exactly. And note that we bust the console_lock, via console_force_unlock(),
first.

-- Keir

> I assume that putting a printk in a timer
> function callback (set up with init_timer & set_timer, etc) or any other
> interrupt handler would not be safe either then.
>
> Roger
>
>
>
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: Wed 9/15/2010 5:03 PM
> To: Roger Cruz; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] Question about printk implementation.
>
> On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@virtualcomputer.com> wrote:
>
>> I was looking over the implementation of printk (xenunstable) and I have a
>> question about the spin_lock_recursive use and the static "buf" variable in
>> this routine. Suppose that that in a single processor system the hypervisor
>> is printing using this printk routine and has acquired the console_lock. Is
>> it possible for a high level interrupt, like an NMI (just as an example, any
>> other interrupts that preempt the current work in the hypervisor will do), to
>> come in and then use printk. Because we are in single CPU mode, the
>> spin_lock_recursive will succeed (if I interpreted the code correctly). When
>> the higher level interrupt exits and returns to the hypervisor code that was
>> previously printing, the contents of the static "buf" would have changed. Is
>> this a possibility??
>
> Well yeah, you know what? Don't do that then! Our printk (like very many Xen
> functions) is not NMI safe.
>
> -- Keir
>
>> Thanks
>> Roger R. Cruz
>>
>> void printk(const char *fmt, ...)
>> {
>> static char buf[1024]; <<<<<---------------
>> static int start_of_line = 1, do_print;
>>
>> va_list args;
>> char *p, *q;
>> unsigned long flags;
>>
>> /* console_lock can be acquired recursively from __printk_ratelimit(). */
>> local_irq_save(flags); <<<<-----------------
>> spin_lock_recursive(&console_lock);
>>
>> va_start(args, fmt);
>> (void)vsnprintf(buf, sizeof(buf), fmt, args);
>> va_end(args);
>>
>> p = buf;
>>
>> while ( (q = strchr(p, '\n')) != NULL )
>> {
>> *q = '\0';
>> if ( start_of_line )
>> do_print = printk_prefix_check(p, &p);
>> if ( do_print )
>> {
>> if ( start_of_line )
>> printk_start_of_line();
>> __putstr(p);
>> __putstr("\n");
>> }
>> start_of_line = 1;
>> p = q + 1;
>>
>
>
>
> No virus found in this incoming message.
> Checked by AVG - www.avg.com
> Version: 9.0.851 / Virus Database: 271.1.1/3134 - Release Date: 09/15/10
> 02:34:00
>
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel