Re: [Xen-devel] [PATCH v3 4/5] xen: modify lock profiling interface

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2019 10:30, Juergen Gross wrote:
> On 03.09.19 16:46, Jan Beulich wrote:
>> On 29.08.2019 12:18, Juergen Gross wrote:
>>> Today adding locks located in a struct to lock profiling requires a
>>> unique type index for each structure. This makes it hard to add any
>>> new structure as the related sysctl interface needs to be changed, too.
>>>
>>> Instead of using an index the already existing struct name specified
>>> in lock_profile_register_struct() can be used as an identifier instead.
>>>
>>> Modify the sysctl interface to use the struct name instead of the type
>>> index and adapt the related coding accordingly. Instead of an array of
>>> struct anchors for lock profiling we now use a linked list for that
>>> purpose. Use the special idx value -1 for cases where the idx isn't
>>> relevant (global locks) and shouldn't be printed.
>>
>> Just to make explicit what was implied by my v1 reply: I can see why
>> you want to do this, but I'm not really a friend of string literals
>> in the hypercall interface, when binary values can also do. So to
>> me this looks to be a move in the wrong direction. Therefore, while
>> I'm fine reviewing such a change, I'm not very likely to eventually
>> ack it.
> 
> I'll write two example patches for adding support of lock profiling in a
> new structure, one with patch 4 of this series applied and one for the
> interface without that patch. This should make clear why I'm really in
> favor of this patch.

Well, I can easily see how one will be quite a bit smaller than the
other, but patch size / intrusiveness is not the only aspect to
consider. IOW I'm afraid the difference won't change my opinion on
string literals in hypercall interfaces. But recall, you don't
depend on me acking this patch of yours, there are enough other
people who can if they are happy with such a model.

Jan

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