Re: [Xen-devel] [PATCH v3 1/3] xen: add warning infrastructure

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.06.16 at 18:30, <wei.liu2@xxxxxxxxxx> wrote:
> @@ -1582,6 +1583,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      init_constructors();
>  
> +    warning_print();
> +
>      console_endboot();

What about an ARM equivalent? Perhaps put this in console_endboot()?

> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -59,6 +59,7 @@ obj-y += vm_event.o
>  obj-y += vmap.o
>  obj-y += vsprintf.o
>  obj-y += wait.o
> +obj-y += warning.o

And perhaps better to put the new code into the console code too?
In any event if you want to keep this in a separate file, considering
that all of it is code/data contributions to .init.*, this wants to be

obj-bin-y += warning.init.o

so that namely string literals get moved to .init.* too.

> --- /dev/null
> +++ b/xen/common/warning.c
> @@ -0,0 +1,50 @@
> +#include <xen/delay.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/softirq.h>
> +#include <xen/warning.h>
> +
> +#define WARNING_ARRAY_SIZE 20
> +static unsigned int __initdata nr_warnings;
> +static const char __initdata *warnings[WARNING_ARRAY_SIZE];

The __initdata belongs after the *.

> +void __init warning_add(const char *warning)
> +{
> +    if ( nr_warnings >= WARNING_ARRAY_SIZE )
> +        panic("Too many pieces of warning text.");

panic() seems a bit harsh, but this shouldn't trigger anyway.

> +    warnings[nr_warnings] = warning;
> +    nr_warnings++;
> +}
> +
> +void __init warning_print(void)
> +{
> +    unsigned int i, j;
> +
> +    if ( !nr_warnings )
> +        return;

Perhaps a single instance of the ***** separators could be printed
here ...

> +    for ( i = 0; i < nr_warnings; i++ )
> +        printk("%s", warnings[i]);

... and here, avoiding each caller to add such?

Jan


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