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

[Thomas Gleixner <tglx@linutronix.de>]

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