Re: [Xen-devel] [PATCH v2] console: increase initial conring size
On 05.12.14 at 16:50, daniel.ki...@oracle.com wrote: In general initial conring size is sufficient. However, if log level is increased on platforms which have e.g. huge number of memory regions (I have an IBM System x3550 M2 with 8 GiB RAM which has more than 200 entries in EFI memory map) then some of earlier messages in console ring are overwritten. It means that in case of issues deeper analysis can be hindered. Sadly conring_size argument does not help because new console buffer is allocated late on heap. It means that it is not possible to allocate larger ring earlier. So, in this situation initial conring size should be increased. My experiments showed that even on not so big machines more than 26 KiB of free space are needed for initial messages. In theory we could increase conring size buffer to 32 KiB. However, I think that this value could be too small for huge machines with large number of ACPI tables and EFI memory regions. Hence, this patch increases initial conring size to 64 KiB. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Actually meanwhile I spotted a (minor) downside to this approach: It raises the lower limit of the permanent ring size to 64k too. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] console: increase initial conring size
In general initial conring size is sufficient. However, if log level is increased on platforms which have e.g. huge number of memory regions (I have an IBM System x3550 M2 with 8 GiB RAM which has more than 200 entries in EFI memory map) then some of earlier messages in console ring are overwritten. It means that in case of issues deeper analysis can be hindered. Sadly conring_size argument does not help because new console buffer is allocated late on heap. It means that it is not possible to allocate larger ring earlier. So, in this situation initial conring size should be increased. My experiments showed that even on not so big machines more than 26 KiB of free space are needed for initial messages. In theory we could increase conring size buffer to 32 KiB. However, I think that this value could be too small for huge machines with large number of ACPI tables and EFI memory regions. Hence, this patch increases initial conring size to 64 KiB. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- This bug (or lack of feature if you prefer) should be fixed, as it was pointed out by Jan Beulich and Olaf Hering, by allocating conring earlier. I though about that before posting this patch (I did not know beforehand about Olaf's work made in 2011). However, I stated that it is too late to make so intrusive changes. So, I think we should (sadly) apply this band-aid to 4.5 because, as you can see in Xen-devel archive, this bug hits more and more people and they fix this issue in the same way as I did in this patch. On the other hand I agree that we should finally fix this issue in better way. Hence, I am adding this thing to my TODO list. v2 - suggestions/fixes: - update documentation (suggested by Andrew Cooper), - add rationale (suggested by Jan Beulich). --- docs/misc/xen-command-line.markdown |2 +- xen/drivers/char/console.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 0866df2..2ad2340 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -286,7 +286,7 @@ A typical setup for most situations might be `com1=115200,8n1` ### conring\_size `= size` - Default: `conring_size=16k` + Default: `conring_size=64k` Specify the size of the console ring buffer. diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 2f03259..429d296 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -67,7 +67,7 @@ custom_param(console_timestamps, parse_console_timestamps); static uint32_t __initdata opt_conring_size; size_param(conring_size, opt_conring_size); -#define _CONRING_SIZE 16384 +#define _CONRING_SIZE 65536 #define CONRING_IDX_MASK(i) ((i)(conring_size-1)) static char __initdata _conring[_CONRING_SIZE]; static char *__read_mostly conring = _conring; -- 1.7.10.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] console: increase initial conring size
On 05.12.14 at 16:50, daniel.ki...@oracle.com wrote: This bug (or lack of feature if you prefer) should be fixed, as it was pointed out by Jan Beulich and Olaf Hering, by allocating conring earlier. I though about that before posting this patch (I did not know beforehand about Olaf's work made in 2011). However, I stated that it is too late to make so intrusive changes. I continue to disagree. If anything, I'd rather see us hide (e.g. behind opt_cpu_info) some of the worst offenders causing the log to become that large. Even if yielding a bigger patch, that would have less impact functionality wise and likely benefit more people. Nor do I see the change to move the allocation earlier all that intrusive. But then again, considering that all you enlarge is an __initdata item, perhaps this is acceptable. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] console: increase initial conring size
On Fri, Dec 05, 2014 at 04:21:35PM +, Jan Beulich wrote: On 05.12.14 at 16:50, daniel.ki...@oracle.com wrote: This bug (or lack of feature if you prefer) should be fixed, as it was pointed out by Jan Beulich and Olaf Hering, by allocating conring earlier. I though about that before posting this patch (I did not know beforehand about Olaf's work made in 2011). However, I stated that it is too late to make so intrusive changes. I continue to disagree. If anything, I'd rather see us hide (e.g. behind opt_cpu_info) some of the worst offenders causing the log to become that large. Even if yielding a bigger patch, that would have less impact Nowadays the worst offender is the EFI memmap which can be quite big. We could hide it behind 'opt_efi_info' and only print out some rather odd entries. But that would be 4.6 material, while this patch nicely fixes it for 4.5. functionality wise and likely benefit more people. Nor do I see the change to move the allocation earlier all that intrusive. But then again, considering that all you enlarge is an __initdata item, perhaps this is acceptable. This has the other side-benefit that it will help us troubleshoot in the field without having the customer try extra parameters to extend the log data. I am all up for less round-trip to troubleshoot issues and I can't see this causing any regressions (unless we have some hard-coded EFL section data). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
