Re: [PATCH V5 02/14] perf/x86: Add perf_get_page_size support

Thomas Gleixner
2019-02-08T18:47:57Z

On Fri, 8 Feb 2019, kan.liang@linux.intel.com wrote:
> +u64 perf_get_page_size(u64 virt)
> +{
> + unsigned long flags;
> + unsigned int level;
> + pte_t *pte;
> +
> + if (!virt)
> + return 0;
> +
> + /*
> + * Interrupts are disabled, so it prevents any tear down
> + * of the page tables.
> + * See the comment near struct mmu_table_batch.
> + */
> + local_irq_save(flags);
> + if (virt >= TASK_SIZE)
> + pte = lookup_address(virt, &level);
> + else {
> + if (current->mm) {
> + pte = lookup_address_in_pgd(pgd_offset(current->mm, virt),
> + virt, &level);
> + } else
> + level = PG_LEVEL_NUM;
> + }

This still lacks quite some curly brackets and aside of that you can write
that so it becomes readable:

if (virt >= TASK_SIZE)
pte = lookup_address(virt, &level);
} else if (current->mm) {
pte = lookup_address_in_pgd(pgd_offset(current->mm, virt),
virt, &level);
} else
level = PG_LEVEL_NUM;
}

> + local_irq_restore(flags);
> + if (level >= PG_LEVEL_NUM)
> + return 0;
> +
> + return (u64)page_level_size(level);
> +}
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index e9acf1d..720dc9e 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -1274,7 +1274,8 @@ static void setup_pebs_sample_data(struct perf_event *event,
> }
>
>
> - if ((sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR)) &&
> + if ((sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR
> + | PERF_SAMPLE_DATA_PAGE_SIZE)) &&
> x86_pmu.intel_cap.pebs_format >= 1)

Can you please define a mask from those constants and use that instead of
adding this ugly line break.

Thanks,

tglx