On Thu, Feb 06, 2025 at 12:28:42PM +0000, Marc Zyngier wrote:
> On Thu, 06 Feb 2025 10:55:21 +0000,
> Mark Rutland <mark.rutland@xxxxxxx> wrote:
> >
> > On Thu, Feb 06, 2025 at 10:42:29AM +0000, Marc Zyngier wrote:
> > > On Thu, 06 Feb 2025 10:03:46 +0000,
> > > Mark Rutland <mark.rutland@xxxxxxx> wrote:
> > > > That said, I'm going to go with the below, adding 'inline' to
> > > > kvm_hyp_handle_memory_fault() and using CPP defines to alias the
> > > > function names:
> > > >
> > > > | static inline bool kvm_hyp_handle_memory_fault(struct kvm_vcpu *vcpu,
> > > > | u64 *exit_code)
> > > > | {
> > > > | if (!__populate_fault_info(vcpu))
> > > > | return true;
> > > > |
> > > > | return false;
> > > > | }
> > > > | #define kvm_hyp_handle_iabt_low kvm_hyp_handle_memory_fault
> > > > | #define kvm_hyp_handle_watchpt_low kvm_hyp_handle_memory_fault
> > > >
> > > > I think that's clearer, and it's more alisnged with how we usually alias
> > > > function names in headers. Other than these two cases, __alias() is only
> > > > used in C files to create a sesparate exprted symbol, and it's odd to
> > > > use it in a header anyhow.
> > > >
> > > > Marc, please should if you'd prefer otherwise.
> > >
> > > Nah, that's fine by me.
> > >
> > > My only issue was with marking functions as inline, and yet storing
> > > pointers to these functions. But it looks like the compiler (GCC 12.2
> > > in my case) is doing a good job noticing the weird pattern, and
> > > generating only one function, even if we store multiple pointers.
> >
> > That's fair -- I'm fairly certain that we do this elsewhere too, but I
> > can switch to __maybe_unused if we're worried that might bite us in
> > future?
>
> Sure, that'd be equally fine.
Looking around, we don't seem to do that elsewhere for functions in
headers, so I'll stick with inline for now unless anyone has a strong
opinion.
I'll send out v2 shortly.
Mark.
