2015-04-20 13:33-0500, Wei Huang:
> <snip>
> >>+/* check if msr_idx is a valid index to access PMU */
> >>+inline int kvm_pmu_check_msr_idx(struct kvm_vcpu *vcpu, unsigned msr_idx)
> >
> >If we really want it inline, it's better done in header.
> >(I think GCC would inline this in-module anyway, but other modules still
> >  have to call it.)
> >
> >>+{
> >>+   return kvm_pmu_ops->check_msr_idx(vcpu, msr_idx);
> >>+}
> >>+
> >| [...]
> >>+bool kvm_pmu_is_msr(struct kvm_vcpu *vcpu, u32 msr)
> >
> >(Might make sense to inline these trivial wrappers.)
> Hi Radim,
> 
> I forgot to mention that I didn't create inline for these functions in V3.
> For an inline to work on across source files, I have to explicitly use
> "extern"; so I decided not to touch it in V3 yet.

(No need for the "extern inline" magic -- you can simply define them as
 "static inline" in pmu.h.)

>                                                   If you insist, I will
> change it.

I don't, it has minimal impact.

> Another solution is to replace the functions with kvm_opmu_ops->blah_blah().
> But this looks less appealing to me.

Agreed, wrappers were a good idea.

Will review today, thanks for the explanation.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to