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.dun...@citrix.com> >>> 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.leng...@intel.com> >>>>> Reviewed-by: Jan Beulich <jbeul...@suse.com> >>>> >>>> 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? I'm happy to go back and Ack all the mm/ ones you've given an R-b to up until this point in the series; I just didn't think it was necessary. I'll probably Ack patch 2 once I become convinced the change in that patch is necessary (which I'm guessing might be when I get to 15/18). -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
