Re: [Xen-devel] [PATCH v2 4/5] xen/gnttab: Refactor gnttab_clear_flag() to be gnttab_clear_flags()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 7/8/19 9:38 PM, Andrew Cooper wrote:
> On 08/07/2019 21:28, Julien Grall wrote:
> > > > > https://lore.kernel.org/xen-devel/1561109798-8744-5-git-send-email-andrew.cooper3@xxxxxxxxxx/T/#t
> > > > To be honest, I think it is wrong to try to micro-optimize a common
> > > > prototype for the benefit of one architecture and one compiler version
> > > > (or even none per the e-mail).
> > > The prototype wasn't common.  Observe that before this patch, ARM used
> > > unsigned long while x86 used uint16_t.  It should become common,
> > > however.
> > I am not sure to follow this. AFAICT, we use uint16_t properly on Arm.
> Look at the modifications to gnttab_clear_flags(), and in particular,
> the removals.  Before this patch, the API really is different between
> ARM and x86.  (Although it differed between unsigned long and unsigned
> int.  The uint16_t was a mistake on my behalf.)
Oh, yes. I am not sure why we use unsigned long for describing the 
shift. Anyway...
> > > In practice, we're talking about bits 3 and 4, and this isn't liable to
> > > change in a hurry.
> > >
> > > > One could also argue that this may be not beneficial for the non-x86
> > > > architecture depending on how the compiler decide to do the cast from
> > > > 32-bit to 16-bit...
> > > All architecture necessarily suffer the downcast somewhere, even x86.
> > > ARM's is in the prototype for guest_clear_mask16(), but in terms of the
> > > common logic for calculating conditionally which bits to clear, keeping
> > > everything as unsigned int for as long as possible offers the most
> > > flexibility to the compiler, as it can see all the constants involved.
> > This is the argument I was looking for :). Thank you for writing it!
> Can I take this as an ack, or a request for clarification in the commit
> message, or something else?
No need for clarification in the commit message.

Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

--
Julien Grall

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