Re: [Xen-devel] [PATCH] console: allocate ring buffer earlier

2014-12-08 Thread Jan Beulich
 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

2014-12-08 Thread Jan Beulich
 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

2014-12-08 Thread Julien Grall
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

2014-12-07 Thread Daniel Kiper
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

2014-12-05 Thread Daniel Kiper
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