Re: [Xen-devel] [PATCH v5 10/18] x86/mem_sharing: Convert MEM_SHARING_DESTROY_GFN to a bool

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.01.2020 17:42, George Dunlap wrote:
> On 1/23/20 4:37 PM, Jan Beulich wrote:
>> On 23.01.2020 17:32, George Dunlap wrote:
>>> On 1/23/20 4:23 PM, Tamas K Lengyel wrote:
>>>> On Thu, Jan 23, 2020 at 9:14 AM George Dunlap <george.dunlap@xxxxxxxxxx> 
>>>> wrote:
>>>>>
>>>>> On 1/21/20 5:49 PM, Tamas K Lengyel wrote:
>>>>>> MEM_SHARING_DESTROY_GFN is used on the 'flags' bitfield during unsharing.
>>>>>> However, the bitfield is not used for anything else, so just convert it 
>>>>>> to a
>>>>>> bool instead.
>>>>>>
>>>>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
>>>>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>
>>>>> But is there a particular advantage to getting rid of the bitfield?
>>>>>
>>>>> You're the maintainer, so it's your decision of course.  But if it were
>>>>> me I'd leave it as a bitfield so that all the bitfield code is there if
>>>>> it's needed in the future.  Flipping it to a bool, with the risk of
>>>>> flipping it back to a bitfield later, seems like pointless churn to me.
>>>>>
>>>>> (Although perhaps the reason will become evident by the time I get to
>>>>> the end of the series.)
>>>>
>>>> Provided its been many years since this code has been added with no
>>>> need for any extra bits, and with no expectations that this would
>>>> change any time soon, I wouldn't worry about that. True, there is very
>>>> little difference between keeping the code as-is vs converting it to
>>>> bool, but IMHO it makes the code easier to follow without you
>>>> wondering what might be those non-existent situations that warranted
>>>> it to be a bitmap to begin with.
>>>
>>> It's definitely a judgement call, and I can see where you're coming
>>> from.  Like I said, if it were me I'd leave it, but it's not. :-)   Just
>>> wanted to raise the issue as I was going through.  I'd Ack it but you've
>>> already got an R-b.
>>
>> Until your proposal gets accepted, isn't it that your ack is needed
>> despite the R-b? This is also why e.g. for patch 2 I didn't see a
>> point in sending any R-b, as the patch will need your ack anyway,
>> and it's so simple that "reviewed" would be an overstatement.
> 
> I don't think I'm co-maintainer of this; and you're a less-specific
> maintainer than Tamas, right?  Do you need my Ack?

Well, I was under the impression that the maintainership nesting
was hierarchical, i.e. the next level up would become the relevant
one in cases like this one. But I'm of course happy to commit the
parts of this series which are ready, if just any less specific
maintainer's ack is sufficient. Probably tomorrow morning then.

Jan

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