Re: [Xen-devel] Question about printk implementation.

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

On 15/09/2010 22:15, "Roger Cruz" <roger.cruz@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxx]
> Sent: Wed 9/15/2010 5:03 PM
> To: Roger Cruz; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] Question about printk implementation.
> 
> On 15/09/2010 21:24, "Roger Cruz" <roger.cruz@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel