Re: [Xen-devel] [PATCH v2 2/5] xen/common: Use %*pb[l] instead of {cpu, node}mask_scn{, list}printf()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.11.18 at 19:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/10/18 11:56, Jan Beulich wrote:
>>>>> On 22.10.18 at 14:57, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> This removes all use of keyhandler_scratch as a bounce-buffer for the 
> rendered
>>> string.  In some cases, collapse combine adjacent printk()'s which are 
> writing
>>> parts of the same line.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Acked-by: Juergen Gross <jgross@xxxxxxxx>
>>> ---
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: Julien Grall <julien.grall@xxxxxxx>
>>>
>>> v2:
>>>  * Use ->bits for cpumasks
>> No, I'm sorry - this is not what I gave my ack for. I specifically
>> said that ->bits may only be used by the cpumask implementation.
>> Everything else should use the cpumask_bits() wrapper.
> 
> cpumask_bits() is worse than useless, because it is longer than the
> alternative, and results in harder to read code.
> 
> I will not be adding to its (mis)use.

Whether that's proper use or misuse would need to be decided. If
it's misuse, then the construct should go away. Despite being longer
than the open coding I do think though that it's better to have (and
use consistently).

>>> --- a/xen/common/event_channel.c
>>> +++ b/xen/common/event_channel.c
>>> @@ -1377,11 +1377,9 @@ static void domain_dump_evtchn_info(struct domain *d)
>>>      unsigned int port;
>>>      int irq;
>>>  
>>> -    bitmap_scnlistprintf(keyhandler_scratch, sizeof(keyhandler_scratch),
>>> -                         d->poll_mask, d->max_vcpus);
>>>      printk("Event channel information for domain %d:\n"
>>> -           "Polling vCPUs: {%s}\n"
>>> -           "    port [p/m/s]\n", d->domain_id, keyhandler_scratch);
>>> +           "Polling vCPUs: {%*pbl}\n"
>>> +           "    port [p/m/s]\n", d->domain_id, d->max_vcpus, d->poll_mask);
>> Neither cpumask_bits() nor its open coding here at all?
> 
> No.  Why do you think poll_mask is a cpumask?  Its a straight bitmap.

Oh, sorry.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel