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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <57c48ebb-029c-1192-e30e-c546742ae311@linux.intel.com>
Date:   Wed, 6 Feb 2019 15:23:47 -0500
From:   "Liang, Kan" <kan.liang@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     acme@...nel.org, tglx@...utronix.de, mingo@...hat.com,
        linux-kernel@...r.kernel.org, eranian@...gle.com, jolsa@...hat.com,
        namhyung@...nel.org, ak@...ux.intel.com, luto@...capital.net,
        Vlastimil Babka <vbabka@...e.cz>,
        Will Deacon <will.deacon@....com>
Subject: Re: [PATCH V4 01/13] perf/core, x86: Add PERF_SAMPLE_DATA_PAGE_SIZE



On 2/4/2019 5:54 AM, Peter Zijlstra wrote:
> On Fri, Feb 01, 2019 at 11:16:51AM -0500, Liang, Kan wrote:
> 
>>> 	if (p4d_large(*p4d)) {
>>
>> This one looks like x86 specific?
> 
>>> 	if (pud_large(*pud)) {
> 
>>> 	if (pmd_large(*pmd)) {
> 
> Kirill did indeed note that p*_large() isn't universally availale (but
> there's definitely !x86 archs that have them). He also said it would
> probably make sense to have them universally available and might help
> clean up mm/gup.c a little.
> 
> A quick grep shows that: ARM, PowerPC, S390, Sparc and x86 have
> 'pmd_large'.
> 
> Anyway; it probably makes sense (and shouldn't be too hard) to fix up
> all architectures to provide this.
> 
Hi Peter and Kirill,

It looks like it's not easy to support get_page_size() universally.
Even the 'pmd_large' you mentioned is not universal. I got error message 
when building with ARCH=riscv.
There is even less support for pud_large and p4d_large.
We have to check and add something like "#define p*d_large(a) 0" in the 
pg headers for each ARCH. I think it's ugly.


> +               if (PageHuge(page)) {
> +                       page = compound_head(page);
> +                       shift = PAGE_SHIFT + compound_order(page);
> +               }

PageHuge() only returns true for hugetlbfs. I think the transparent huge 
pages should also use compound pages, right? Besides hugetlbfs and THP, 
are there any other cases which also use compound pages?
Can the codes handle all these cases?

> +static u64 __perf_get_page_size(struct mm_struct *mm, unsigned long
> +addr) {
> +	pgd_t *pgd;
> +	p4d_t *p4d;

An universal get_page_size() function should not be implemented in perf. 
It will be a problem for future maintenance.

All in all, I think we are far away from an universal get_page_size(). A 
__weak function + x86 implementation solution proposed in this patch 
series should be a better choice.
- Other ARCH can have their own implementation later if they want this 
feature.
- Standard pg table helper functions are used for x86. Maintenance will 
not be a problem.

What do you think?

Thanks,
Kan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ