Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1390201 > unrolled thread

[PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT Michal Hocko <mhocko@kernel.org> - 2016-04-28 15:40 +0200
    Re: [PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-04-28 17:00 +0200
      Re: [PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT Michal Hocko <mhocko@kernel.org> - 2016-04-28 17:10 +0200
        Re: [PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT Michal Hocko <mhocko@kernel.org> - 2016-04-29 11:50 +0200

#1390201 — [PATCH 04/20] arm: get rid of superfluous __GFP_REPEAT

From: Michal Hocko <mhocko@suse.com>

__GFP_REPEAT has a rather weak semantic but since it has been introduced
around 2.6.12 it has been ignored for low order allocations.

PGALLOC_GFP uses __GFP_REPEAT but none of the allocation which uses
this flag is for more than order-2. This means that this flag has never
been actually useful here because it has always been used only for
PAGE_ALLOC_COSTLY requests.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arch@vger.kernel.org
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/arm/include/asm/pgalloc.h | 2 +-
 arch/arm/mm/pgd.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index 20febb368844..b2902a5cd780 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -57,7 +57,7 @@ static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
 extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
 
-#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO)
+#define PGALLOC_GFP	(GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO)
 
 static inline void clean_pte_table(pte_t *pte)
 {
diff --git a/arch/arm/mm/pgd.c b/arch/arm/mm/pgd.c
index b8d477321730..c1c1a5c67da1 100644
--- a/arch/arm/mm/pgd.c
+++ b/arch/arm/mm/pgd.c
@@ -23,7 +23,7 @@
 #define __pgd_alloc()	kmalloc(PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL)
 #define __pgd_free(pgd)	kfree(pgd)
 #else
-#define __pgd_alloc()	(pgd_t *)__get_free_pages(GFP_KERNEL | __GFP_REPEAT, 2)
+#define __pgd_alloc()	(pgd_t *)__get_free_pages(GFP_KERNEL, 2)
 #define __pgd_free(pgd)	free_pages((unsigned long)pgd, 2)
 #endif
 
-- 
2.8.0.rc3

[toc] | [next] | [standalone]

#1390279

On Thu, Apr 28, 2016 at 03:23:50PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> __GFP_REPEAT has a rather weak semantic but since it has been introduced
> around 2.6.12 it has been ignored for low order allocations.
> 
> PGALLOC_GFP uses __GFP_REPEAT but none of the allocation which uses
> this flag is for more than order-2. This means that this flag has never
> been actually useful here because it has always been used only for
> PAGE_ALLOC_COSTLY requests.

I'm unconvinced.  Back in 2013, I was seeing a lot of failures, so:

commit 8c65da6dc89ccb605d73773b1dd617e72982d971
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date:   Sat Nov 30 12:52:31 2013 +0000

    ARM: pgd allocation: retry on failure

    Make pgd allocation retry on failure; we really need this to succeed
    otherwise fork() can trigger OOMs.

    Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Maybe something has changed again in the MM layer which makes this flag
unnecessary again, and it was a temporary blip around that time, I don't
know.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1390289

On Thu 28-04-16 15:55:45, Russell King - ARM Linux wrote:
> On Thu, Apr 28, 2016 at 03:23:50PM +0200, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > __GFP_REPEAT has a rather weak semantic but since it has been introduced
> > around 2.6.12 it has been ignored for low order allocations.
> > 
> > PGALLOC_GFP uses __GFP_REPEAT but none of the allocation which uses
> > this flag is for more than order-2. This means that this flag has never
> > been actually useful here because it has always been used only for
> > PAGE_ALLOC_COSTLY requests.
> 
> I'm unconvinced.  Back in 2013, I was seeing a lot of failures, so:
> 
> commit 8c65da6dc89ccb605d73773b1dd617e72982d971
> Author: Russell King <rmk+kernel@arm.linux.org.uk>
> Date:   Sat Nov 30 12:52:31 2013 +0000
> 
>     ARM: pgd allocation: retry on failure
> 
>     Make pgd allocation retry on failure; we really need this to succeed
>     otherwise fork() can trigger OOMs.
> 
>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Maybe something has changed again in the MM layer which makes this flag
> unnecessary again, and it was a temporary blip around that time, I don't
> know.

PAGE_ALLOC_COSTLY_ORDER is defined to order 3 since 2007 and even before
the code was doing
-               if ((order <= 3) || (gfp_mask & __GFP_REPEAT))
+               if ((order <= PAGE_ALLOC_COSTLY_ORDER) ||
+                                               (gfp_mask & __GFP_REPEAT))
                        do_retry = 1;

So an order-2 allocation which is the case for this particular code now
will trigger the OOM killer and fail only when the current task is
killed by the OOM killer. Other than that order-2 is basically
GFP_NOFAIL. Have a look at __alloc_pages_slowpath() for more details.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1390949

On Thu 28-04-16 17:08:31, Michal Hocko wrote:
> On Thu 28-04-16 15:55:45, Russell King - ARM Linux wrote:
> > On Thu, Apr 28, 2016 at 03:23:50PM +0200, Michal Hocko wrote:
> > > From: Michal Hocko <mhocko@suse.com>
> > > 
> > > __GFP_REPEAT has a rather weak semantic but since it has been introduced
> > > around 2.6.12 it has been ignored for low order allocations.
> > > 
> > > PGALLOC_GFP uses __GFP_REPEAT but none of the allocation which uses
> > > this flag is for more than order-2. This means that this flag has never
> > > been actually useful here because it has always been used only for
> > > PAGE_ALLOC_COSTLY requests.
> > 
> > I'm unconvinced.  Back in 2013, I was seeing a lot of failures, so:
> > 
> > commit 8c65da6dc89ccb605d73773b1dd617e72982d971
> > Author: Russell King <rmk+kernel@arm.linux.org.uk>
> > Date:   Sat Nov 30 12:52:31 2013 +0000
> > 
> >     ARM: pgd allocation: retry on failure
> > 
> >     Make pgd allocation retry on failure; we really need this to succeed
> >     otherwise fork() can trigger OOMs.
> > 
> >     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > Maybe something has changed again in the MM layer which makes this flag
> > unnecessary again, and it was a temporary blip around that time, I don't
> > know.
> 
> PAGE_ALLOC_COSTLY_ORDER is defined to order 3 since 2007 and even before
> the code was doing
> -               if ((order <= 3) || (gfp_mask & __GFP_REPEAT))
> +               if ((order <= PAGE_ALLOC_COSTLY_ORDER) ||
> +                                               (gfp_mask & __GFP_REPEAT))
>                         do_retry = 1;
> 
> So an order-2 allocation which is the case for this particular code now
> will trigger the OOM killer and fail only when the current task is
> killed by the OOM killer. Other than that order-2 is basically
> GFP_NOFAIL. Have a look at __alloc_pages_slowpath() for more details.

Does this explanation help?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web