Re: [PATCH][mm/memory.c]: transparent hugepage check condition missed

[Guan Jun He]

Excuse me! I got the one email sent 6 times!

best,
Guanjun

>>> On 10/31/2011 at 05:02 PM, in message
<1320051775-13992-1-...@suse.com>, Guanjun He <gj...@suse.com>
wrote:
> For the transparent hugepage module still does not support
> tmpfs and cache,the check condition should always be checked
> to make sure that it only affect the anonymous maps, the
> original check condition missed this, this patch is to fix this.
> Otherwise,the hugepage may affect the file-backed maps,
> then the cache for the small-size pages will be unuseful,
> and till now there is still no implementation for hugepage's cache.
>
> Signed-off-by: Guanjun He <gj...@suse.com>
> ---
> mm/memory.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index a56e3ba..79b85fe 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3475,7 +3475,8 @@ int handle_mm_fault(struct mm_struct *mm, struct
> vm_area_struct *vma,
> if (pmd_trans_huge(orig_pmd)) {
> if (flags & FAULT_FLAG_WRITE &&
> !pmd_write(orig_pmd) &&
> - !pmd_trans_splitting(orig_pmd))
> + !pmd_trans_splitting(orig_pmd) &&
> + !vma->vm_ops)
> return do_huge_pmd_wp_page(mm, vma, address,
> pmd, orig_pmd);
> return 0;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[GuanJun He]

[Shaohua Li]

so if vma->vm_ops != NULL, how could the pmd_trans_huge(orig_pmd) be
true? We never enable THP if vma->vm_ops != NULL.

[Guan Jun He]

>>> On 11/1/2011 at 09:18 AM, in message <1320110288.22361.190.camel@sli10-conroe>,

acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits,
it's only a pagesize, not a flag to identity a hugepage.
If I change my default pagesize to PAGE_PSE, then THP will be confused.

There is already a defination:

#define VM_HUGEPAGE 0x01000000 /* MADV_HUGEPAGE marked this vma */

maybe,this can be the flag to identity a hugepage.But the comment marked it only stands for MADV_HUGEPAGE,
and it's still a hugepage.So, I suggest to add the check condition !vma->vm_ops, or turn to
use VM_HUGEPAGE as the flag.
or
adjust the logic to:
(transparent_hugepage_enabled() use the VM_HUGEPAGE flag)

if(transparent_hugepage_enabled(vma)){
if (pmd_none(*pmd){
...
}
else
{
...
}
}


the original logic is:

if (pmd_none(*pmd) && transparent_hugepage_enabled(vma))
{
...
}
else
{
...

[Shaohua Li]

2011/11/1 Guan Jun He <gj...@suse.com>:

Not sure what pagesize means here, assume pmd entry bits.
how could you make the default 'pagesize' to PAGE_PSE?

[Guan Jun He]

>>> On 11/1/2011 at 04:42 PM, in message
<CANejiEVk41X-P+UyMf96jmPr...@mail.gmail.com>, Shaohua

yes, it's pmd entry bits.

> how could you make the default 'pagesize' to PAGE_PSE?

That requires some work and not so easy and need hardware support... So, recently it won't come.
But one can easily create the same pmd entry bits for some special use;
as comment above, it's a pmd entry bits, only mark a size, not a flag;
and adjust the logic to use the flag can perfect avoid this potential issuse,
and basically no impact to the current code.

[Andrea Arcangeli]

Hi,

Not needed, this is the cow case and pmd_trans_huge can't be true if
vm_ops is not null. That's already checked a few lines before the
above code and in collapse_huge_page for khugepaged.

Thanks.