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; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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
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
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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