On Tue, May 18, 2021 at 09:28:52PM +0800, Xu, Like wrote: > > How would pebs && !intr be possible? > > I don't think it's possible. And yet you keep that 'intr||pebs' weirdness :/ > > Also; wouldn't this be more legible > > when written like: > > > > perf_overflow_handler_t ovf = kvm_perf_overflow; > > > > ... > > > > if (intr) > > ovf = kvm_perf_overflow_intr; > > > > ... > > > > event = perf_event_create_kernel_counter(&attr, -1, current, ovf, pmc); > > > > Please yell if you don't like this: > > diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c > index 711294babb97..a607f5a1b9cd 100644 > --- a/arch/x86/kvm/pmu.c > +++ b/arch/x86/kvm/pmu.c > @@ -122,6 +122,8 @@ static void pmc_reprogram_counter(struct kvm_pmc *pmc, > u32 type, > .config = config, > }; > bool pebs = test_bit(pmc->idx, (unsigned long *)&pmu->pebs_enable); > + perf_overflow_handler_t ovf = (intr || pebs) ? > + kvm_perf_overflow_intr : kvm_perf_overflow; This, that's exactly the kind of code I wanted to get rid of. ?: has it's place I suppose, but you're creating dense ugly code for no reason. perf_overflow_handle_t ovf = kvm_perf_overflow; if (intr) ovf = kvm_perf_overflow_intr; Is so much easier to read. And if you really worry about that pebs thing; you can add: WARN_ON_ONCE(pebs && !intr);
