Re: [Xen-devel] [PATCH RFC] xen/vsprintf: Introduce %pd formatter for domains
On 30/08/18 15:18, Jan Beulich wrote:
On 30.08.18 at 15:06, wrote:
>> This allows all system domids to be printed by name, rather than special
>> casing the idle vcpus alone.
>>
>> Signed-off-by: Andrew Cooper
>> ---
>> CC: George Dunlap
>> CC: Jan Beulich
>> CC: Konrad Rzeszutek Wilk
>> CC: Stefano Stabellini
>> CC: Tim Deegan
>> CC: Wei Liu
>> CC: Julien Grall
>>
>> RFC, because this was proposed before but rejected at the time. I'm looking
>> to try and turn errors like this:
>>
>> (XEN) mm.c:1023:d0v2 pg_owner d32754 l1e_owner d0, but real_pg_owner d0
>> (XEN) mm.c:1099:d0v2 Error getting mfn 810020 (pfn 59db1) from L1 entry
>> 800810020227 for l1e_owner d0, pg_owner d32754
>>
>> into the slightly more helpful:
>>
>> (XEN) mm.c:1022:d0v2 pg_owner dXEN l1e_owner d0, but real_pg_owner d0
>> (XEN) mm.c:1098:d0v2 Error getting mfn 810020 (pfn 59db1) from L1 entry
>> 800810020227 for l1e_owner d0, pg_owner dXEN
>>
>> although even in this case, the former printk has an awkward corner case of a
>> possibly NULL domain pointer, which can possibly only reasonably be fixed
>> inside pointer() itself.
> Or in print_domain(), producing "NULL".
Sounds like a good plan.
>
>> --- a/docs/misc/printk-formats.txt
>> +++ b/docs/misc/printk-formats.txt
>> @@ -28,5 +28,8 @@ Symbol/Function pointers:
>>
>> Domain and vCPU information:
>>
>> + %pd Domain from a 'struct domain *d' (printed as d, but
>> with
>> + system domains represented by name, e.g. 'dIDLE')
> This looks a little awkward - how about d etc?
A sample looks like:
dIDLEv0 dIO dXEN dCOW
dv0 d d d
Another alternative would be:
d[IDLE]v0 d[IO] d[XEN] d[COW]
Which I think I prefer to angle brackets.
>
>> --- a/xen/common/vsprintf.c
>> +++ b/xen/common/vsprintf.c
>> @@ -264,6 +264,41 @@ static char *string(char *str, char *end, const char *s,
>> return str;
>> }
>>
>> +/* Print a domain as d or d for system domains. */
>> +static char *print_domain(char *str, char *end, const struct domain *d)
>> +{
>> +const char *name = NULL;
>> +
>> +if ( str < end )
>> +*str++ = 'd';
> I would guess you've copied this idiom from somewhere, and if so
> it would be good to know where we still have got such broken
> construct(s) left:
No - serves me write from trying to code from memory. Fixed.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] xen/vsprintf: Introduce %pd formatter for domains
>>> On 30.08.18 at 15:06, wrote:
> This allows all system domids to be printed by name, rather than special
> casing the idle vcpus alone.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: George Dunlap
> CC: Jan Beulich
> CC: Konrad Rzeszutek Wilk
> CC: Stefano Stabellini
> CC: Tim Deegan
> CC: Wei Liu
> CC: Julien Grall
>
> RFC, because this was proposed before but rejected at the time. I'm looking
> to try and turn errors like this:
>
> (XEN) mm.c:1023:d0v2 pg_owner d32754 l1e_owner d0, but real_pg_owner d0
> (XEN) mm.c:1099:d0v2 Error getting mfn 810020 (pfn 59db1) from L1 entry
> 800810020227 for l1e_owner d0, pg_owner d32754
>
> into the slightly more helpful:
>
> (XEN) mm.c:1022:d0v2 pg_owner dXEN l1e_owner d0, but real_pg_owner d0
> (XEN) mm.c:1098:d0v2 Error getting mfn 810020 (pfn 59db1) from L1 entry
> 800810020227 for l1e_owner d0, pg_owner dXEN
>
> although even in this case, the former printk has an awkward corner case of a
> possibly NULL domain pointer, which can possibly only reasonably be fixed
> inside pointer() itself.
Or in print_domain(), producing "NULL".
> --- a/docs/misc/printk-formats.txt
> +++ b/docs/misc/printk-formats.txt
> @@ -28,5 +28,8 @@ Symbol/Function pointers:
>
> Domain and vCPU information:
>
> + %pd Domain from a 'struct domain *d' (printed as d, but
> with
> + system domains represented by name, e.g. 'dIDLE')
This looks a little awkward - how about d etc?
> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -264,6 +264,41 @@ static char *string(char *str, char *end, const char *s,
> return str;
> }
>
> +/* Print a domain as d or d for system domains. */
> +static char *print_domain(char *str, char *end, const struct domain *d)
> +{
> +const char *name = NULL;
> +
> +if ( str < end )
> +*str++ = 'd';
I would guess you've copied this idiom from somewhere, and if so
it would be good to know where we still have got such broken
construct(s) left: The increment needs to happen outside of the
if(), for snprintf() et al to return a large enough number. See e.g.
string(). Perhaps best to do this like you do ...
> +static char *print_vcpu(char *str, char *end, const struct vcpu *v)
> +{
> +str = print_domain(str, end, v->domain);
> +
> +if ( str < end )
> +*str = 'v';
> +
> +return number(str + 1, end, v->vcpu_id, 10, -1, -1, 0);
... here.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
