* [PATCH][mm] adjust the logic of checking THP [not found] <pebolle@tiscali.nl;linux-kernel@vger.kernel.org;linux-mm@kvack.org;gjhe@suse.com> @ 2011-11-02 6:34 ` Guanjun He 2011-11-02 12:17 ` Hillf Danton 2011-11-23 0:04 ` Andrea Arcangeli 0 siblings, 2 replies; 11+ messages in thread From: Guanjun He @ 2011-11-02 6:34 UTC (permalink / raw) To: linux-kernel; +Cc: Guanjun He Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, it's a pmd entry bits, only mark a size, not a flag;As one can easily create the same pmd entry bits for some special use,then the check will get confused.And this patch is to adjust the logic to use the flag, it can perfectly avoid this potential issuse,and basically no impact to the current code. Signed-off-by: Guanjun He <heguanbo@gmail.com> --- mm/memory.c | 28 +++++++++++++++------------- 1 files changed, 15 insertions(+), 13 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index a56e3ba..a76b17f 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, pmd = pmd_alloc(mm, pud, address); if (!pmd) return VM_FAULT_OOM; - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { - if (!vma->vm_ops) - return do_huge_pmd_anonymous_page(mm, vma, address, - pmd, flags); - } else { - pmd_t orig_pmd = *pmd; - barrier(); - if (pmd_trans_huge(orig_pmd)) { - if (flags & FAULT_FLAG_WRITE && - !pmd_write(orig_pmd) && - !pmd_trans_splitting(orig_pmd)) - return do_huge_pmd_wp_page(mm, vma, address, - pmd, orig_pmd); + if (transparent_hugepage_enabled(vma)) { + if (pmd_none(*pmd)) { + if (!vma->vm_ops) + return do_huge_pmd_anonymous_page(mm, vma, address, + pmd, flags); + } else { + pmd_t orig_pmd = *pmd; + barrier(); + if (pmd_trans_huge(orig_pmd)) { + if (flags & FAULT_FLAG_WRITE && + !pmd_write(orig_pmd) && + !pmd_trans_splitting(orig_pmd)) + return do_huge_pmd_wp_page(mm, vma, address, + pmd, orig_pmd); return 0; + } } } -- 1.7.7 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP 2011-11-02 6:34 ` [PATCH][mm] adjust the logic of checking THP Guanjun He @ 2011-11-02 12:17 ` Hillf Danton 2011-11-03 2:19 ` GuanJun He 2011-11-23 0:04 ` Andrea Arcangeli 1 sibling, 1 reply; 11+ messages in thread From: Hillf Danton @ 2011-11-02 12:17 UTC (permalink / raw) To: Guanjun He; +Cc: linux-kernel [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3514 bytes --] On Wed, Nov 2, 2011 at 2:34 PM, Guanjun He <heguanbo@gmail.com> wrote: > > Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, > it's a pmd entry bits, only mark a size, not a flag;As one can easily > create the same pmd entry bits for some special use,then the check > will get confused.And this patch is to adjust the logic to use the flag, > it can perfectly avoid this potential issuse,and basically no impact > to the current code. > > > Signed-off-by: Guanjun He <heguanbo@gmail.com> > --- >  mm/memory.c |  28 +++++++++++++++------------- >  1 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index a56e3ba..a76b17f 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >     pmd = pmd_alloc(mm, pud, address); >     if (!pmd) >         return VM_FAULT_OOM; > -    if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { > -        if (!vma->vm_ops) > -            return do_huge_pmd_anonymous_page(mm, vma, address, > -                             pmd, flags); > -    } else { > -        pmd_t orig_pmd = *pmd; > -        barrier(); > -        if (pmd_trans_huge(orig_pmd)) { > -            if (flags & FAULT_FLAG_WRITE && > -              !pmd_write(orig_pmd) && > -              !pmd_trans_splitting(orig_pmd)) > -                return do_huge_pmd_wp_page(mm, vma, address, > -                              pmd, orig_pmd); > +    if (transparent_hugepage_enabled(vma)) { Well, how about THP not configured? Thanks, Hillf > +        if (pmd_none(*pmd)) { > +            if (!vma->vm_ops) > +                return do_huge_pmd_anonymous_page(mm, vma, address, > +                                 pmd, flags); > +        } else { > +            pmd_t orig_pmd = *pmd; > +            barrier(); > +            if (pmd_trans_huge(orig_pmd)) { > +                if (flags & FAULT_FLAG_WRITE && > +                  !pmd_write(orig_pmd) && > +                  !pmd_trans_splitting(orig_pmd)) > +                    return do_huge_pmd_wp_page(mm, vma, address, > +                                  pmd, orig_pmd); >             return 0; > +            } >         } >     } > > -- > 1.7.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html > Please read the FAQ at  http://www.tux.org/lkml/ > > > ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP 2011-11-02 12:17 ` Hillf Danton @ 2011-11-03 2:19 ` GuanJun He 2011-11-04 2:41 ` GuanJun He 0 siblings, 1 reply; 11+ messages in thread From: GuanJun He @ 2011-11-03 2:19 UTC (permalink / raw) To: Hillf Danton; +Cc: linux-kernel On Wed, Nov 2, 2011 at 8:17 PM, Hillf Danton <dhillf@gmail.com> wrote: > On Wed, Nov 2, 2011 at 2:34 PM, Guanjun He <heguanbo@gmail.com> wrote: >> >> Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, >> it's a pmd entry bits, only mark a size, not a flag;As one can easily >> create the same pmd entry bits for some special use,then the check >> will get confused.And this patch is to adjust the logic to use the flag, >> it can perfectly avoid this potential issuse,and basically no impact >> to the current code. >> >> >> Signed-off-by: Guanjun He <heguanbo@gmail.com> >> --- >> mm/memory.c | 28 +++++++++++++++------------- >> 1 files changed, 15 insertions(+), 13 deletions(-) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index a56e3ba..a76b17f 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >> pmd = pmd_alloc(mm, pud, address); >> if (!pmd) >> return VM_FAULT_OOM; >> - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { >> - if (!vma->vm_ops) >> - return do_huge_pmd_anonymous_page(mm, vma, address, >> - pmd, flags); >> - } else { >> - pmd_t orig_pmd = *pmd; >> - barrier(); >> - if (pmd_trans_huge(orig_pmd)) { >> - if (flags & FAULT_FLAG_WRITE && >> - !pmd_write(orig_pmd) && >> - !pmd_trans_splitting(orig_pmd)) >> - return do_huge_pmd_wp_page(mm, vma, address, >> - pmd, orig_pmd); >> + if (transparent_hugepage_enabled(vma)) { > > Well, how about THP not configured? What do you mean 'not configured'? assume not enabled. If THP not enabled, of course the transparent_hugepage_enabled(vma) will be false. > > Thanks, > Hillf > >> + if (pmd_none(*pmd)) { >> + if (!vma->vm_ops) >> + return do_huge_pmd_anonymous_page(mm, vma, address, >> + pmd, flags); >> + } else { >> + pmd_t orig_pmd = *pmd; >> + barrier(); >> + if (pmd_trans_huge(orig_pmd)) { >> + if (flags & FAULT_FLAG_WRITE && >> + !pmd_write(orig_pmd) && >> + !pmd_trans_splitting(orig_pmd)) >> + return do_huge_pmd_wp_page(mm, vma, address, >> + pmd, orig_pmd); >> return 0; >> + } >> } >> } >> >> -- >> 1.7.7 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP 2011-11-03 2:19 ` GuanJun He @ 2011-11-04 2:41 ` GuanJun He 0 siblings, 0 replies; 11+ messages in thread From: GuanJun He @ 2011-11-04 2:41 UTC (permalink / raw) To: Hillf Danton, linux-kernel, linux-mm; +Cc: gjhe On Thu, Nov 3, 2011 at 10:19 AM, GuanJun He <heguanbo@gmail.com> wrote: > On Wed, Nov 2, 2011 at 8:17 PM, Hillf Danton <dhillf@gmail.com> wrote: >> On Wed, Nov 2, 2011 at 2:34 PM, Guanjun He <heguanbo@gmail.com> wrote: >>> >>> Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, >>> it's a pmd entry bits, only mark a size, not a flag;As one can easily >>> create the same pmd entry bits for some special use,then the check >>> will get confused.And this patch is to adjust the logic to use the flag, >>> it can perfectly avoid this potential issuse,and basically no impact >>> to the current code. >>> >>> >>> Signed-off-by: Guanjun He <heguanbo@gmail.com> >>> --- >>> mm/memory.c | 28 +++++++++++++++------------- >>> 1 files changed, 15 insertions(+), 13 deletions(-) >>> >>> diff --git a/mm/memory.c b/mm/memory.c >>> index a56e3ba..a76b17f 100644 >>> --- a/mm/memory.c >>> +++ b/mm/memory.c >>> @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >>> pmd = pmd_alloc(mm, pud, address); >>> if (!pmd) >>> return VM_FAULT_OOM; >>> - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { >>> - if (!vma->vm_ops) >>> - return do_huge_pmd_anonymous_page(mm, vma, address, >>> - pmd, flags); >>> - } else { >>> - pmd_t orig_pmd = *pmd; >>> - barrier(); >>> - if (pmd_trans_huge(orig_pmd)) { >>> - if (flags & FAULT_FLAG_WRITE && >>> - !pmd_write(orig_pmd) && >>> - !pmd_trans_splitting(orig_pmd)) >>> - return do_huge_pmd_wp_page(mm, vma, address, >>> - pmd, orig_pmd); >>> + if (transparent_hugepage_enabled(vma)) { >> >> Well, how about THP not configured? > > What do you mean 'not configured'? assume not enabled. > If THP not enabled, of course the transparent_hugepage_enabled(vma) > will be false. In the original logic, it's possible that the code go into the logic of THP while the THP is not enabled. Just think, If build another module that make pmd with the same bits, even THP is not enabled, the code can go into the logic of the THP.And this adjust can avoid this. best, Guanjun > >> >> Thanks, >> Hillf >> >>> + if (pmd_none(*pmd)) { >>> + if (!vma->vm_ops) >>> + return do_huge_pmd_anonymous_page(mm, vma, address, >>> + pmd, flags); >>> + } else { >>> + pmd_t orig_pmd = *pmd; >>> + barrier(); >>> + if (pmd_trans_huge(orig_pmd)) { >>> + if (flags & FAULT_FLAG_WRITE && >>> + !pmd_write(orig_pmd) && >>> + !pmd_trans_splitting(orig_pmd)) >>> + return do_huge_pmd_wp_page(mm, vma, address, >>> + pmd, orig_pmd); >>> return 0; >>> + } >>> } >>> } >>> >>> -- >>> 1.7.7 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> >>> >>> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP @ 2011-11-04 2:41 ` GuanJun He 0 siblings, 0 replies; 11+ messages in thread From: GuanJun He @ 2011-11-04 2:41 UTC (permalink / raw) To: Hillf Danton, linux-kernel, linux-mm; +Cc: gjhe On Thu, Nov 3, 2011 at 10:19 AM, GuanJun He <heguanbo@gmail.com> wrote: > On Wed, Nov 2, 2011 at 8:17 PM, Hillf Danton <dhillf@gmail.com> wrote: >> On Wed, Nov 2, 2011 at 2:34 PM, Guanjun He <heguanbo@gmail.com> wrote: >>> >>> Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, >>> it's a pmd entry bits, only mark a size, not a flag;As one can easily >>> create the same pmd entry bits for some special use,then the check >>> will get confused.And this patch is to adjust the logic to use the flag, >>> it can perfectly avoid this potential issuse,and basically no impact >>> to the current code. >>> >>> >>> Signed-off-by: Guanjun He <heguanbo@gmail.com> >>> --- >>> mm/memory.c | 28 +++++++++++++++------------- >>> 1 files changed, 15 insertions(+), 13 deletions(-) >>> >>> diff --git a/mm/memory.c b/mm/memory.c >>> index a56e3ba..a76b17f 100644 >>> --- a/mm/memory.c >>> +++ b/mm/memory.c >>> @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >>> pmd = pmd_alloc(mm, pud, address); >>> if (!pmd) >>> return VM_FAULT_OOM; >>> - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { >>> - if (!vma->vm_ops) >>> - return do_huge_pmd_anonymous_page(mm, vma, address, >>> - pmd, flags); >>> - } else { >>> - pmd_t orig_pmd = *pmd; >>> - barrier(); >>> - if (pmd_trans_huge(orig_pmd)) { >>> - if (flags & FAULT_FLAG_WRITE && >>> - !pmd_write(orig_pmd) && >>> - !pmd_trans_splitting(orig_pmd)) >>> - return do_huge_pmd_wp_page(mm, vma, address, >>> - pmd, orig_pmd); >>> + if (transparent_hugepage_enabled(vma)) { >> >> Well, how about THP not configured? > > What do you mean 'not configured'? assume not enabled. > If THP not enabled, of course the transparent_hugepage_enabled(vma) > will be false. In the original logic, it's possible that the code go into the logic of THP while the THP is not enabled. Just think, If build another module that make pmd with the same bits, even THP is not enabled, the code can go into the logic of the THP.And this adjust can avoid this. best, Guanjun > >> >> Thanks, >> Hillf >> >>> + if (pmd_none(*pmd)) { >>> + if (!vma->vm_ops) >>> + return do_huge_pmd_anonymous_page(mm, vma, address, >>> + pmd, flags); >>> + } else { >>> + pmd_t orig_pmd = *pmd; >>> + barrier(); >>> + if (pmd_trans_huge(orig_pmd)) { >>> + if (flags & FAULT_FLAG_WRITE && >>> + !pmd_write(orig_pmd) && >>> + !pmd_trans_splitting(orig_pmd)) >>> + return do_huge_pmd_wp_page(mm, vma, address, >>> + pmd, orig_pmd); >>> return 0; >>> + } >>> } >>> } >>> >>> -- >>> 1.7.7 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> >>> >>> >> > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP 2011-11-02 6:34 ` [PATCH][mm] adjust the logic of checking THP Guanjun He 2011-11-02 12:17 ` Hillf Danton @ 2011-11-23 0:04 ` Andrea Arcangeli 2011-12-06 4:18 ` GuanJun He 1 sibling, 1 reply; 11+ messages in thread From: Andrea Arcangeli @ 2011-11-23 0:04 UTC (permalink / raw) To: Guanjun He; +Cc: linux-kernel On Wed, Nov 02, 2011 at 02:34:30PM +0800, Guanjun He wrote: > > Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, > it's a pmd entry bits, only mark a size, not a flag;As one can easily > create the same pmd entry bits for some special use,then the check > will get confused.And this patch is to adjust the logic to use the flag, > it can perfectly avoid this potential issuse,and basically no impact > to the current code. You can't use _PAGE_PSE for special use for the pmd. Besides this is common code, archs without such bit can define pmd_trans_huge to return 0 like it happens with TRANSPARENT_HUGEPAGE=n. > diff --git a/mm/memory.c b/mm/memory.c > index a56e3ba..a76b17f 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, > pmd = pmd_alloc(mm, pud, address); > if (!pmd) > return VM_FAULT_OOM; > - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { > - if (!vma->vm_ops) > - return do_huge_pmd_anonymous_page(mm, vma, address, > - pmd, flags); > - } else { > - pmd_t orig_pmd = *pmd; > - barrier(); > - if (pmd_trans_huge(orig_pmd)) { > - if (flags & FAULT_FLAG_WRITE && > - !pmd_write(orig_pmd) && > - !pmd_trans_splitting(orig_pmd)) > - return do_huge_pmd_wp_page(mm, vma, address, > - pmd, orig_pmd); > + if (transparent_hugepage_enabled(vma)) { > + if (pmd_none(*pmd)) { > + if (!vma->vm_ops) > + return do_huge_pmd_anonymous_page(mm, vma, address, > + pmd, flags); > + } else { > + pmd_t orig_pmd = *pmd; > + barrier(); > + if (pmd_trans_huge(orig_pmd)) { > + if (flags & FAULT_FLAG_WRITE && > + !pmd_write(orig_pmd) && > + !pmd_trans_splitting(orig_pmd)) > + return do_huge_pmd_wp_page(mm, vma, address, > + pmd, orig_pmd); > return 0; > + } This will infinite loop if you disable THP at runtime while some mapping needing cow is established. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm] adjust the logic of checking THP 2011-11-23 0:04 ` Andrea Arcangeli @ 2011-12-06 4:18 ` GuanJun He 0 siblings, 0 replies; 11+ messages in thread From: GuanJun He @ 2011-12-06 4:18 UTC (permalink / raw) To: Andrea Arcangeli; +Cc: linux-kernel On Wed, Nov 23, 2011 at 8:04 AM, Andrea Arcangeli <aarcange@redhat.com> wrote: > On Wed, Nov 02, 2011 at 02:34:30PM +0800, Guanjun He wrote: >> >> Acturally, pmd_trans_huge(orig_pmd) only checks the _PAGE_PSE bits, >> it's a pmd entry bits, only mark a size, not a flag;As one can easily >> create the same pmd entry bits for some special use,then the check >> will get confused.And this patch is to adjust the logic to use the flag, >> it can perfectly avoid this potential issuse,and basically no impact >> to the current code. > > You can't use _PAGE_PSE for special use for the pmd. Besides this is > common code, archs without such bit can define pmd_trans_huge to > return 0 like it happens with TRANSPARENT_HUGEPAGE=n. ok, even _PAGE_PSE can not be used for special use for the pmd, This check is more efficient. the original code always check 2 items " pmd_none(*pmd) && transparent_hugepage_enabled(vma) ", even TRANSPARENT_HUGEPAGE not enabled. > >> diff --git a/mm/memory.c b/mm/memory.c >> index a56e3ba..a76b17f 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -3465,20 +3465,22 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >> pmd = pmd_alloc(mm, pud, address); >> if (!pmd) >> return VM_FAULT_OOM; >> - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { >> - if (!vma->vm_ops) >> - return do_huge_pmd_anonymous_page(mm, vma, address, >> - pmd, flags); >> - } else { >> - pmd_t orig_pmd = *pmd; >> - barrier(); >> - if (pmd_trans_huge(orig_pmd)) { >> - if (flags & FAULT_FLAG_WRITE && >> - !pmd_write(orig_pmd) && >> - !pmd_trans_splitting(orig_pmd)) >> - return do_huge_pmd_wp_page(mm, vma, address, >> - pmd, orig_pmd); >> + if (transparent_hugepage_enabled(vma)) { >> + if (pmd_none(*pmd)) { >> + if (!vma->vm_ops) >> + return do_huge_pmd_anonymous_page(mm, vma, address, >> + pmd, flags); >> + } else { >> + pmd_t orig_pmd = *pmd; >> + barrier(); >> + if (pmd_trans_huge(orig_pmd)) { >> + if (flags & FAULT_FLAG_WRITE && >> + !pmd_write(orig_pmd) && >> + !pmd_trans_splitting(orig_pmd)) >> + return do_huge_pmd_wp_page(mm, vma, address, >> + pmd, orig_pmd); >> return 0; >> + } > > This will infinite loop if you disable THP at runtime while some > mapping needing cow is established. ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <shaohua.li@intel.com;linux-mm@kvack.org;linux-kernel@vger.kernel.org>]
* [PATCH][mm:] adjust the logic of checking THP [not found] <shaohua.li@intel.com;linux-mm@kvack.org;linux-kernel@vger.kernel.org> @ 2011-11-01 11:41 ` Guanjun He 2011-11-01 12:26 ` Paul Bolle 0 siblings, 1 reply; 11+ messages in thread From: Guanjun He @ 2011-11-01 11:41 UTC (permalink / raw) To: linux-kernel; +Cc: Guanjun He Signed-off-by: Guanjun He <heguanbo@gmail.com> --- mm/memory.c | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index a56e3ba..d6dd6b3 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3465,20 +3465,24 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, pmd = pmd_alloc(mm, pud, address); if (!pmd) return VM_FAULT_OOM; - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { - if (!vma->vm_ops) - return do_huge_pmd_anonymous_page(mm, vma, address, - pmd, flags); - } else { - pmd_t orig_pmd = *pmd; - barrier(); - if (pmd_trans_huge(orig_pmd)) { - if (flags & FAULT_FLAG_WRITE && - !pmd_write(orig_pmd) && - !pmd_trans_splitting(orig_pmd)) - return do_huge_pmd_wp_page(mm, vma, address, - pmd, orig_pmd); - return 0; + if (transparent_hugepage_enabled(vma)) { + if(pmd_none(*pmd)){ + if (!vma->vm_ops) + return do_huge_pmd_anonymous_page(mm, vma, address, + pmd, flags); + } + else + { + pmd_t orig_pmd = *pmd; + barrier(); + if (pmd_trans_huge(orig_pmd)) { + if (flags & FAULT_FLAG_WRITE && + !pmd_write(orig_pmd) && + !pmd_trans_splitting(orig_pmd)) + return do_huge_pmd_wp_page(mm, vma, address, + pmd, orig_pmd); + return 0; + } } } -- 1.7.7 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH][mm:] adjust the logic of checking THP 2011-11-01 11:41 ` [PATCH][mm:] " Guanjun He @ 2011-11-01 12:26 ` Paul Bolle 2011-11-01 12:29 ` Paul Bolle 2011-11-03 8:26 ` GuanJun He 0 siblings, 2 replies; 11+ messages in thread From: Paul Bolle @ 2011-11-01 12:26 UTC (permalink / raw) To: Guanjun He; +Cc: linux-kernel Perhaps a summary would help the people actually knowledgeable about mm understand why this adjustment is needed. I'll just mention a few obvious things. On Tue, 2011-11-01 at 19:41 +0800, Guanjun He wrote: > Signed-off-by: Guanjun He <heguanbo@gmail.com> > --- > mm/memory.c | 32 ++++++++++++++++++-------------- > 1 files changed, 18 insertions(+), 14 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index a56e3ba..d6dd6b3 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3465,20 +3465,24 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, > pmd = pmd_alloc(mm, pud, address); > if (!pmd) > return VM_FAULT_OOM; > - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { > - if (!vma->vm_ops) > - return do_huge_pmd_anonymous_page(mm, vma, address, > - pmd, flags); > - } else { > - pmd_t orig_pmd = *pmd; > - barrier(); > - if (pmd_trans_huge(orig_pmd)) { > - if (flags & FAULT_FLAG_WRITE && > - !pmd_write(orig_pmd) && > - !pmd_trans_splitting(orig_pmd)) > - return do_huge_pmd_wp_page(mm, vma, address, > - pmd, orig_pmd); > - return 0; > + if (transparent_hugepage_enabled(vma)) { > + if(pmd_none(*pmd)){ codingstyle (scripts/checkpatch.pl would have caught that one). > + if (!vma->vm_ops) > + return do_huge_pmd_anonymous_page(mm, vma, address, > + pmd, flags); > + } > + else > + { codingstyle (ditto). There's probably more. > + pmd_t orig_pmd = *pmd; > + barrier(); > + if (pmd_trans_huge(orig_pmd)) { > + if (flags & FAULT_FLAG_WRITE && > + !pmd_write(orig_pmd) && > + !pmd_trans_splitting(orig_pmd)) > + return do_huge_pmd_wp_page(mm, vma, address, > + pmd, orig_pmd); > + return 0; > + } > } > } > Paul Bolle ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm:] adjust the logic of checking THP 2011-11-01 12:26 ` Paul Bolle @ 2011-11-01 12:29 ` Paul Bolle 2011-11-03 8:26 ` GuanJun He 1 sibling, 0 replies; 11+ messages in thread From: Paul Bolle @ 2011-11-01 12:29 UTC (permalink / raw) To: Guanjun He; +Cc: linux-kernel On Tue, 2011-11-01 at 13:26 +0100, Paul Bolle wrote: > Perhaps a summary would help the people actually knowledgeable about mm > understand why this adjustment is needed s/a summary/an explanation/ Paul Bolle ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][mm:] adjust the logic of checking THP 2011-11-01 12:26 ` Paul Bolle 2011-11-01 12:29 ` Paul Bolle @ 2011-11-03 8:26 ` GuanJun He 1 sibling, 0 replies; 11+ messages in thread From: GuanJun He @ 2011-11-03 8:26 UTC (permalink / raw) To: Paul Bolle; +Cc: linux-kernel Thanks!I have updated and resent it. best, Guanjun On Tue, Nov 1, 2011 at 8:26 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > Perhaps a summary would help the people actually knowledgeable about mm > understand why this adjustment is needed. > > I'll just mention a few obvious things. > > On Tue, 2011-11-01 at 19:41 +0800, Guanjun He wrote: >> Signed-off-by: Guanjun He <heguanbo@gmail.com> >> --- >> mm/memory.c | 32 ++++++++++++++++++-------------- >> 1 files changed, 18 insertions(+), 14 deletions(-) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index a56e3ba..d6dd6b3 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -3465,20 +3465,24 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma, >> pmd = pmd_alloc(mm, pud, address); >> if (!pmd) >> return VM_FAULT_OOM; >> - if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) { >> - if (!vma->vm_ops) >> - return do_huge_pmd_anonymous_page(mm, vma, address, >> - pmd, flags); >> - } else { >> - pmd_t orig_pmd = *pmd; >> - barrier(); >> - if (pmd_trans_huge(orig_pmd)) { >> - if (flags & FAULT_FLAG_WRITE && >> - !pmd_write(orig_pmd) && >> - !pmd_trans_splitting(orig_pmd)) >> - return do_huge_pmd_wp_page(mm, vma, address, >> - pmd, orig_pmd); >> - return 0; >> + if (transparent_hugepage_enabled(vma)) { >> + if(pmd_none(*pmd)){ > > codingstyle (scripts/checkpatch.pl would have caught that one). > >> + if (!vma->vm_ops) >> + return do_huge_pmd_anonymous_page(mm, vma, address, >> + pmd, flags); >> + } >> + else >> + { > > codingstyle (ditto). There's probably more. > >> + pmd_t orig_pmd = *pmd; >> + barrier(); >> + if (pmd_trans_huge(orig_pmd)) { >> + if (flags & FAULT_FLAG_WRITE && >> + !pmd_write(orig_pmd) && >> + !pmd_trans_splitting(orig_pmd)) >> + return do_huge_pmd_wp_page(mm, vma, address, >> + pmd, orig_pmd); >> + return 0; >> + } >> } >> } >> > > > Paul Bolle > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-12-06 4:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <pebolle@tiscali.nl;linux-kernel@vger.kernel.org;linux-mm@kvack.org;gjhe@suse.com>
2011-11-02 6:34 ` [PATCH][mm] adjust the logic of checking THP Guanjun He
2011-11-02 12:17 ` Hillf Danton
2011-11-03 2:19 ` GuanJun He
2011-11-04 2:41 ` GuanJun He
2011-11-04 2:41 ` GuanJun He
2011-11-23 0:04 ` Andrea Arcangeli
2011-12-06 4:18 ` GuanJun He
[not found] <shaohua.li@intel.com;linux-mm@kvack.org;linux-kernel@vger.kernel.org>
2011-11-01 11:41 ` [PATCH][mm:] " Guanjun He
2011-11-01 12:26 ` Paul Bolle
2011-11-01 12:29 ` Paul Bolle
2011-11-03 8:26 ` GuanJun He
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.