[PATCH] console: allocate ring buffer earlier

[PATCH] console: allocate ring buffer earlier

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2596 bytes --]

... 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 <jbeulich@suse.com>

--- 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);
     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);
 
     printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
 }
 
+void __init console_init_postirq(void)
+{
+    serial_init_postirq();
+
+    if ( !opt_conring_size )
+        opt_conring_size = num_present_cpus() << (9 + xenlog_lower_thresh);
+
+    if ( conring == _conring )
+        console_init_mem();
+}
+
 void __init console_endboot(void)
 {
     int i, j;
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -14,6 +14,7 @@ struct xen_sysctl_readconsole;
 long read_console_ring(struct xen_sysctl_readconsole *op);
 
 void console_init_preirq(void);
+void console_init_mem(void);
 void console_init_postirq(void);
 void console_endboot(void);
 int console_has(const char *device);




[-- Attachment #2: conring-alloc-earlier.patch --]
[-- Type: text/plain, Size: 2631 bytes --]

console: allocate ring buffer earlier

... 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 <jbeulich@suse.com>

--- 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);
     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);
 
     printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
 }
 
+void __init console_init_postirq(void)
+{
+    serial_init_postirq();
+
+    if ( !opt_conring_size )
+        opt_conring_size = num_present_cpus() << (9 + xenlog_lower_thresh);
+
+    if ( conring == _conring )
+        console_init_mem();
+}
+
 void __init console_endboot(void)
 {
     int i, j;
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -14,6 +14,7 @@ struct xen_sysctl_readconsole;
 long read_console_ring(struct xen_sysctl_readconsole *op);
 
 void console_init_preirq(void);
+void console_init_mem(void);
 void console_init_postirq(void);
 void console_endboot(void);
 int console_has(const char *device);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] console: allocate ring buffer earlier

[Daniel Kiper]

On Fri, Dec 05, 2014 at 04:55:24PM +0000, 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 <jbeulich@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

Re: [PATCH] console: allocate ring buffer earlier

[Daniel Kiper]

On Fri, Dec 05, 2014 at 06:22:57PM +0100, Daniel Kiper wrote:
> On Fri, Dec 05, 2014 at 04:55:24PM +0000, 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 <jbeulich@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

Re: [PATCH] console: allocate ring buffer earlier

[Jan Beulich]

>>> On 05.12.14 at 18:22, <daniel.kiper@oracle.com> wrote:
> On Fri, Dec 05, 2014 at 04:55:24PM +0000, 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

Re: [PATCH] console: allocate ring buffer earlier

[Julien Grall]

Hi Jan,

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.

Regards,

-- 
Julien Grall

Re: [PATCH] console: allocate ring buffer earlier

[Jan Beulich]

>>> On 06.12.14 at 01:05, <julien.grall@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

Re: [PATCH] console: allocate ring buffer earlier

[Julien Grall]

On 08/12/14 08:52, Jan Beulich wrote:
>>>> On 06.12.14 at 01:05, <julien.grall@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