Re: [PATCH v6 09/24] xsplice: Implement payload loading

["Jan Beulich" <JBeulich@suse.com>]

>>> On 11.04.16 at 18:34, <konrad.wilk@oracle.com> wrote:
> On Mon, Apr 11, 2016 at 12:03:49PM -0400, Konrad Rzeszutek Wilk wrote:
>> On Mon, Apr 11, 2016 at 09:53:06AM -0600, Jan Beulich wrote:
>> > >>> On 09.04.16 at 02:37, <konrad.wilk@oracle.com> wrote:
>> > > On Fri, Apr 08, 2016 at 04:50:10PM -0600, Jan Beulich wrote:
>> > >> >>> On 09.04.16 at 00:45, <konrad.wilk@oracle.com> wrote:
>> > >> > On Fri, Apr 08, 2016 at 03:18:09PM -0600, Jan Beulich wrote:
>> > >> >> >>> On 08.04.16 at 23:10, <konrad.wilk@oracle.com> wrote:
>> > >> >> >> > +int arch_xsplice_perform_rela(struct xsplice_elf *elf,
>> > >> >> >> > +                              const struct xsplice_elf_sec *base,
>> > >> >> >> > +                              const struct xsplice_elf_sec *rela)
>> > >> >> >> > +{
>> > >> >> >> > +    const Elf_RelA *r;
>> > >> >> >> > +    unsigned int symndx, i;
>> > >> >> >> > +    uint64_t val;
>> > >> >> >> > +    uint8_t *dest;
>> > >> >> >> > +
>> > >> >> >> > +    if ( !rela->sec->sh_entsize || !rela->sec->sh_size ||
>> > >> >> >> > +         rela->sec->sh_entsize != sizeof(Elf_RelA) )
>> > >> >> >> > +    {
>> > >> >> >> > +        dprintk(XENLOG_DEBUG, XSPLICE "%s: Section relative header is 
> corrupted!\n",
>> > >> >> >> > +                elf->name);
>> > >> >> >> 
>> > >> >> >> XENLOG_ERR surely? (and the other examples).
>> > >> >> > 
>> > >> >> > Yes! I modified all of those that return an error. One of them I made
>> > >> >> > an printk (the one about more than 64 sections).
>> > >> >> 
>> > >> >> Why would that be any worse than other check failures? I think
>> > >> >> these log messages should all be issued consistently.
>> > >> > 
>> > >> > OK, so all be printk instead of dprintk?
>> > >> 
>> > >> Rather the other way around I would say.
>> > > 
>> > > Back to dprintk(XENLOG_DEBUG for all of them then.
>> > 
>> > I don't see why dprintk() can't be used with log levels other
>> > than debug, as suggested by (I think) Andrew above.
>> 
>> OK, let me make them dprintk(XENLOG_ERROR
> 
> I've pretty much modified most of them to that, the exceptions are these:
> 
> +                dprintk(XENLOG_DEBUG, XSPLICE "%s: Loaded %s at 0x%p\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: Resolved old address %s => %p\n",
> +                dprintk(XENLOG_DEBUG, XSPLICE "%s: Already loaded as %s!\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: new symbol %s\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: overriding symbol %s\n",
> +    dprintk(XENLOG_DEBUG, XSPLICE "%s: Applying %u functions.\n",
> +    dprintk(XENLOG_DEBUG, XSPLICE "%s: Reverting.\n", data->name);
> +    dprintk(XENLOG_DEBUG, XSPLICE "%s: timeout is %"PRI_stime"ms\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: CPU%u - IPIing the other %u CPUs\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: Undefined symbol resolved: %s => %#"PRIxElfAddr"\n",
> +            dprintk(XENLOG_DEBUG, XSPLICE "%s: Absolute symbol: %s => %#"PRIxElfAddr"\n",
> +                dprintk(XENLOG_DEBUG, XSPLICE "%s: Symbol resolved: %s => %#"PRIxElfAddr"(%s)\n",
> 
> And then these are printk variants:
> 
> 
> +                printk(XENLOG_ERR XSPLICE "%s: Overflow in relocation %u in %s for %s!\n",
> +            printk(XENLOG_ERR XSPLICE "%s: Unhandled relocation %lu\n",
> +        printk(XENLOG_ERR XSPLICE "%s: Could not allocate memory for payload!\n",
> +            printk(XENLOG_ERR XSPLICE "%s: %s is missing!\n",
> +            printk(XENLOG_ERR XSPLICE "%s: %s is empty!\n",
> +            printk(XENLOG_ERR XSPLICE "%s: %s was seen more than once!\n",
> +                    printk(XENLOG_ERR XSPLICE "%s: Could not resolve old address of %s\n",
> +                printk(XENLOG_ERR XSPLICE "%s: duplicate new symbol: %s\n",
> +        printk(XENLOG_ERR XSPLICE "%s: unable to get cpu_maps lock!\n",
> +        printk(XENLOG_ERR XSPLICE "%s: Timed out on %s semaphore %u/%u\n",
> +            printk(XENLOG_ERR XSPLICE "%s: CPU%u - unable to get cpu_maps lock!\n",
> +        printk(XENLOG_INFO XSPLICE "%s finished %s with rc=%d\n",
> +        printk(XENLOG_INFO XSPLICE ": build-id: %*phN\n", len, binary_id);
> +        printk(XENLOG_ERR XSPLICE"%s: Could not allocate memory for section table!\n",
> +        printk(XENLOG_ERR XSPLICE "%s: Could not allocate memory for symbols\n",
> +            printk(XENLOG_ERR XSPLICE "%s: Unexpected common symbol: %s\n",
> +                    printk(XENLOG_ERR XSPLICE "%s: Unknown symbol: %s\n",
> 
> We can change some of those to dprintk if folks want that.

So as mentioned before I'd again like to ask for consistency: I
cannot really see the criteria by which some of these use dprintk()
vs printk(). The main aspect here is: If things go severely wrong,
will the console be spammed? And the second one: Which of these
are really useful in the field?

Jan

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