Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 11 Jan 2023 13:54:54 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 2/3] perf/core: Set data->sample_flags in perf_prepare_sample() |
| |
On Tue, Jan 10, 2023 at 12:06:00PM -0800, Namhyung Kim wrote:
> Another example, but in this case it's real, is ADDR. We cannot update > the data->addr just because filtered_sample_type has PHYS_ADDR or > DATA_PAGE_SIZE as it'd lose the original value.
Hmm, how about something like so?
/* * if (flags & s) flags |= d; // without branches */ static __always_inline unsigned long __cond_set(unsigned long flags, unsigned long s, unsigned long d) { return flags | (d * !!(flags & s)); }
Then:
fst = sample_type; fst = __cond_set(fst, PERF_SAMPLE_CODE_PAGE_SIZE, PERF_SAMPLE_IP); fst = __cond_set(fst, PERF_SAMPLE_DATA_PAGE_SIZE | PERF_SAMPLE_PHYS_ADDR, PERF_SAMPLE_ADDR); fst = __cond_set(fst, PERF_SAMPLE_STACK_USER, PERF_SAMPLE_REGS_USER); fst &= ~data->sample_flags;
This way we express the implicit conditions by setting the required sample data flags, then we mask those we already have set.
After the above something like:
if (fst & PERF_SAMPLE_ADDR) { data->addr = 0; data->sample_flags |= PERF_SAMPLE_ADDR; }
if (fst & PERF_SAMPLE_PHYS_ADDR) { data->phys_addr = perf_virt_to_phys(data->addr); data->sample_flags |= PERF_SAMPLE_PHYS_ADDR; }
if (fst & PERF_SAMPLE_DATA_PAGE_SIZE) { data->data_page_size = perf_get_page_size(data->addr); data->sample_flags |= PERF_SAMPLE_DATA_PAGE_SIZE; }
And maybe something like:
#define __IF_SAMPLE_DATA(f_) ({ \ bool __f = fst & PERF_SAMPLE_##f_; \ if (__f) data->sample_flags |= PERF_SAMPLE_##f_;\ __f; })
#define IF_SAMPLE_DATA(f_) if (__IF_SAMPLE_DATA(f_))
Then we can write:
IF_SAMPLE_DATA(ADDR) data->addr = 0;
IF_SAMPLE_DATA(PHYS_ADDR) data->phys_addr = perf_virt_to_phys(data->addr);
IF_SAMPLE_DATA(DATA_PAGE_SIZE) data->data_page_size = perf_get_page_size(data->addr);
But I didn't check code-gen for this last suggestion.
| ![\](/images/icornerr.gif) |