[PATCH v7 4/5] x86: perf: Refactor misc flag assignments

[Colton Lewis]

Colton Lewis <coltonlewis at google.com> writes:

> Peter Zijlstra <peterz at infradead.org> writes:

>> On Fri, Nov 08, 2024 at 08:20:44PM +0100, Peter Zijlstra wrote:

>>> Isn't the below more or less what you want?

>>> static unsigned long misc_flags(struct pt_regs *regs)
>>> {
>>> 	unsigned long flags = 0;

>>> 	if (regs->flags & PERF_EFLAGS_EXACT)
>>> 		flags |= PERF_RECORD_MISC_EXACT_IP;

>>> 	return flags;
>>> }

>>> static unsigned long native_flags(struct pt_regs *regs)
>>> {
>>> 	unsigned long flags = 0;

>>> 	if (user_mode(regs))
>>> 		flags |= PERF_RECORD_MISC_USER;
>>> 	else
>>> 		flags |= PERF_RECORD_MISC_KERNEL;

>>> 	return flags;
>>> }

>>> static unsigned long guest_flags(struct pt_regs *regs)
>>> {
>>> 	unsigned long guest_state = perf_guest_state();
>>> 	unsigned long flags = 0;

>>> 	if (guest_state & PERF_GUEST_ACTIVE) {
>>> 		if (guest_state & PERF_GUEST_USER)
>>> 			flags |= PERF_RECORD_MISC_GUEST_USER;
>>> 		else
>>> 			flags |= PERF_RECORD_MISC_GUEST_KERNEL;
>>> 	}

>>> 	return flags;
>>> }

>>> unsigned long perf_arch_guest_misc_flags(struct pt_regs *regs)
>>> {
>>> 	unsigned long flags;

>>> 	flags = misc_flags(regs);
>>> 	flags |= guest_flags(regs);

>>> 	return flags;
>>> }

>>> unsigned long perf_arch_misc_flags(struct pt_regs *regs)
>>> {
>>> 	unsigned long flags;
>>> 	unsigned long guest;

>>> 	flags = misc_flags(regs);
>>> 	guest = guest_flags(regs);
>>> 	if (guest)
>>> 		flags |= guest;
>>> 	else
>>> 		flags |= native_flags(regs);

>>> 	return flags;
>>> }

>> This last can be written more concise:

>> unsigned long perf_arch_misc_flags(struct pt_regs *regs)
>> {
>> 	unsigned long flags;

>> 	flags = guest_flags(regs);
>> 	if (!flags)
>> 		flags |= native_flags(regs);

>> 	flgs |= misc_flags(regs);

>> 	return flags;
>> }

> This isn't right because it is choosing to return guest or native
> flags depending on the presence of guest flags, but that's not what we
> want.

> See perf_misc_flags in kernel/events/core.c which chooses to return
> perf_arch_guest_misc_flags or perf_arch_misc_flags depending on
> should_sample_guest which depends on more than current guest state.

This is in the next patch. Excuse me for not clarifying.

> But I will take some of your suggestions to split the functions out
> more.