linux-kernel - Re: [PATCH V5 02/14] perf/x86: Add perf_get_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 8 Feb 2019 19:47:43 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Kan Liang <kan.liang@...ux.intel.com>
cc:     peterz@...radead.org, acme@...nel.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, eranian@...gle.com, jolsa@...hat.com,
        namhyung@...nel.org, ak@...ux.intel.com, luto@...capital.net,
        vbabka@...e.cz, will.deacon@....com, kirill@...temov.name
Subject: Re: [PATCH V5 02/14] perf/x86: Add perf_get_page_size support

On Fri, 8 Feb 2019, kan.liang@...ux.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