Re: [Xen-devel] [PATCH] x86/EPT: adjustments for redundant function arguments

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Friday, December 20, 2019 11:02 PM
> 
> On 20.12.2019 15:58, George Dunlap wrote:
> > On 12/20/19 2:41 PM, Jan Beulich wrote:
> >> On 20.12.2019 15:26, George Dunlap wrote:
> >>> On 12/20/19 2:21 PM, Jan Beulich wrote:
> >>>> In ept_p2m_type_to_flags() passing in type and access as separate
> >>>> parameters can be considered an optimization, as all callers set the
> >>>> respective fields in the entry being updated before the call. Retain
> >>>> this behavior but add assertions.
> >>>>
> >>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>>
> >>> In what way is it an optimization?
> >>
> >> There's no pointer de-ref needed; the values will already come in
> >> via registers. And "can be considered" because possibly some
> >> compilers are smart enough to eliminate the pointer de-ref again
> >> (but then it'll still be a bitfield extract, which callers may
> >> be able to avoid).
> >
> > Right; on the whole I'd rather let compilers do this sort of
> > micro-optimization, and only do this "manual" sort of optimization with
> > some sort of benchmarks showing that is has some kind of effect.
> >
> >>
> >>> I don't necessarily oppose this, but given that 3 of the 4 callers
> >>> literally do something like:
> >>>
> >>>     ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt, e.access);
> >>>
> >>> It seems like just getting rid of the extraneous arguments might a be
> >>> better option.
> >>
> >> That was my original intention as well, but iirc Andrew didn't like
> >> it when we discussed it back then (the context here being XSA-304).
> >
> > I did a quick skim through those threads and couldn't find any comment
> > on this issue.  Could you point me to the mail with it?  (Or Andy, would
> > you care to repeat your argument?)
> 
> I guess it may have been an irc discussion, quite possibly even
> a private one between him and me.
> 
> > Ultimately the patch as it stands is only making the existing code
> > safer, so I'm OK with giving it my Ack if you don't want to pursue the
> > other option; but I'd prefer trying to understand and potentially
> > improve things while we're at it.  (And if there *is* a good reason for
> > passing in parallel parameters, it would be good to record it in a
> > comment so we don't have this conversation again in 3 years' time.)
> 
> I'd be happy to go the other route - as said, that's what I had
> initially.
> 

Can Andrew chime in for his concern on this approach?

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