Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier
On 06.12.14 at 01:05, julien.gr...@linaro.org wrote: On 05/12/2014 16:55, Jan Beulich wrote: I didn't change ARM, as I wasn't sure how far ahead this call could be pulled. AFAIU, the new function only requires that the page table are setup (because of the alloc_xenheap_pages). So console_init_mem could be called right after console_init_preirq. No, it requires that alloc_xenheap_pages() works, i.e. it surely can't be placed before the end_boot_allocator() invocation. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier
On 05.12.14 at 18:22, daniel.ki...@oracle.com wrote: On Fri, Dec 05, 2014 at 04:55:24PM +, Jan Beulich wrote: @@ -763,17 +762,28 @@ void __init console_init_postirq(void) } opt_conring_size = PAGE_SIZE order; -spin_lock_irq(console_lock); +spin_lock_irqsave(console_lock, flags); I am not sure why are you change spin_lock_irq() to spin_lock_irqsave() here. Could you explain this in commit message? I think this is quite obvious in the context here: Previously this was guaranteed to be called after interrupts were already enabled (hence the _postirq name). That's not the case anymore, and thus we can't blindly enable interrupts when releasing the lock. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier
On 08/12/14 08:52, Jan Beulich wrote: On 06.12.14 at 01:05, julien.gr...@linaro.org wrote: On 05/12/2014 16:55, Jan Beulich wrote: I didn't change ARM, as I wasn't sure how far ahead this call could be pulled. AFAIU, the new function only requires that the page table are setup (because of the alloc_xenheap_pages). So console_init_mem could be called right after console_init_preirq. No, it requires that alloc_xenheap_pages() works, i.e. it surely can't be placed before the end_boot_allocator() invocation. Which is the case with the place I was suggesting... On ARM, end_boot_allocator is called in setup_mm which is called before console_init_preirq. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier
On Fri, Dec 05, 2014 at 06:22:57PM +0100, Daniel Kiper wrote: On Fri, Dec 05, 2014 at 04:55:24PM +, Jan Beulich wrote: ... when conring_size= was specified on the command line. We can't really do this as early as we would want to when the option was not specified, as the default depends on knowing the system CPU count. Yet the parsing of the ACPI tables is one of the things that generates a lot of output especially on large systems. I didn't change ARM, as I wasn't sure how far ahead this call could be pulled. Signed-off-by: Jan Beulich jbeul...@suse.com Make sense for me but I think that we should have the same thing for ARM too. Hmmm... Even it is better I still think that your proposal is too intrusive at this stage of 4.5 development. However, I think that Konrad has the last word in that case. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier
On Fri, Dec 05, 2014 at 04:55:24PM +, Jan Beulich wrote: ... when conring_size= was specified on the command line. We can't really do this as early as we would want to when the option was not specified, as the default depends on knowing the system CPU count. Yet the parsing of the ACPI tables is one of the things that generates a lot of output especially on large systems. I didn't change ARM, as I wasn't sure how far ahead this call could be pulled. Signed-off-by: Jan Beulich jbeul...@suse.com Make sense for me but I think that we should have the same thing for ARM too. --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1187,6 +1187,7 @@ void __init noreturn __start_xen(unsigne } vm_init(); +console_init_mem(); vesa_init(); softirq_init(); --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -744,15 +744,14 @@ void __init console_init_preirq(void) } } -void __init console_init_postirq(void) +void __init console_init_mem(void) { char *ring; unsigned int i, order, memflags; - -serial_init_postirq(); +unsigned long flags; if ( !opt_conring_size ) -opt_conring_size = num_present_cpus() (9 + xenlog_lower_thresh); +return; order = get_order_from_bytes(max(opt_conring_size, conring_size)); memflags = MEMF_bits(crashinfo_maxaddr_bits); @@ -763,17 +762,28 @@ void __init console_init_postirq(void) } opt_conring_size = PAGE_SIZE order; -spin_lock_irq(console_lock); +spin_lock_irqsave(console_lock, flags); I am not sure why are you change spin_lock_irq() to spin_lock_irqsave() here. Could you explain this in commit message? for ( i = conringc ; i != conringp; i++ ) ring[i (opt_conring_size - 1)] = conring[i (conring_size - 1)]; conring = ring; smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */ conring_size = opt_conring_size; -spin_unlock_irq(console_lock); +spin_unlock_irqrestore(console_lock, flags); Ditto. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel