From: Peter Zijlstra <[email protected]>
Expose the gfp to alloc_flags mapping, so we can use it in other parts
of the vm.
Signed-off-by: Peter Zijlstra <[email protected]>
Signed-off-by: Suresh Jayaraman <[email protected]>
---
mm/internal.h | 15 +++++++++++++++
mm/page_alloc.c | 16 +---------------
2 files changed, 16 insertions(+), 15 deletions(-)
Index: mmotm/mm/internal.h
===================================================================
--- mmotm.orig/mm/internal.h
+++ mmotm/mm/internal.h
@@ -194,6 +194,21 @@ static inline struct page *mem_map_next(
#define __paginginit __init
#endif
+/* The ALLOC_WMARK bits are used as an index to zone->watermark */
+#define ALLOC_WMARK_MIN WMARK_MIN
+#define ALLOC_WMARK_LOW WMARK_LOW
+#define ALLOC_WMARK_HIGH WMARK_HIGH
+#define ALLOC_NO_WATERMARKS 0x04 /* don't check watermarks at all */
+
+/* Mask to get the watermark bits */
+#define ALLOC_WMARK_MASK (ALLOC_NO_WATERMARKS-1)
+
+#define ALLOC_HARDER 0x10 /* try to alloc harder */
+#define ALLOC_HIGH 0x20 /* __GFP_HIGH set */
+#define ALLOC_CPUSET 0x40 /* check for correct cpuset */
+
+int gfp_to_alloc_flags(gfp_t gfp_mask);
+
/* Memory initialisation debug and verification */
enum mminit_level {
MMINIT_WARNING,
Index: mmotm/mm/page_alloc.c
===================================================================
--- mmotm.orig/mm/page_alloc.c
+++ mmotm/mm/page_alloc.c
@@ -1190,19 +1190,6 @@ failed:
return NULL;
}
-/* The ALLOC_WMARK bits are used as an index to zone->watermark */
-#define ALLOC_WMARK_MIN WMARK_MIN
-#define ALLOC_WMARK_LOW WMARK_LOW
-#define ALLOC_WMARK_HIGH WMARK_HIGH
-#define ALLOC_NO_WATERMARKS 0x04 /* don't check watermarks at all */
-
-/* Mask to get the watermark bits */
-#define ALLOC_WMARK_MASK (ALLOC_NO_WATERMARKS-1)
-
-#define ALLOC_HARDER 0x10 /* try to alloc harder */
-#define ALLOC_HIGH 0x20 /* __GFP_HIGH set */
-#define ALLOC_CPUSET 0x40 /* check for correct cpuset */
-
#ifdef CONFIG_FAIL_PAGE_ALLOC
static struct fail_page_alloc_attr {
@@ -1691,8 +1678,7 @@ void wake_all_kswapd(unsigned int order,
wakeup_kswapd(zone, order);
}
-static inline int
-gfp_to_alloc_flags(gfp_t gfp_mask)
+int gfp_to_alloc_flags(gfp_t gfp_mask)
{
struct task_struct *p = current;
int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
On Thu, 1 Oct 2009, Suresh Jayaraman wrote:
> From: Peter Zijlstra <[email protected]>
>
> Expose the gfp to alloc_flags mapping, so we can use it in other parts
> of the vm.
>
> Signed-off-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Suresh Jayaraman <[email protected]>
Nack, these flags are internal to the page allocator and exporting them to
generic VM code is unnecessary.
The only bit you actually use in your patchset is ALLOC_NO_WATERMARKS to
determine whether a particular allocation can use memory reserves. I'd
suggest adding a bool function that returns whether the current context is
given access to reserves including your new __GFP_MEMALLOC flag and
exporting that instead.
On Thursday October 1, [email protected] wrote:
> On Thu, 1 Oct 2009, Suresh Jayaraman wrote:
>
> > From: Peter Zijlstra <[email protected]>
> >
> > Expose the gfp to alloc_flags mapping, so we can use it in other parts
> > of the vm.
> >
> > Signed-off-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Suresh Jayaraman <[email protected]>
>
> Nack, these flags are internal to the page allocator and exporting them to
> generic VM code is unnecessary.
>
> The only bit you actually use in your patchset is ALLOC_NO_WATERMARKS to
> determine whether a particular allocation can use memory reserves. I'd
> suggest adding a bool function that returns whether the current context is
> given access to reserves including your new __GFP_MEMALLOC flag and
> exporting that instead.
That sounds like a very appropriate suggestion, thanks.
So something like this?
Then change every occurrence of
+ if (!(gfp_to_alloc_flags(gfpflags) & ALLOC_NO_WATERMARKS))
to
+ if (!(gfp_has_no_watermarks(gfpflags)))
??
Thanks,
NeilBrown
diff --git a/mm/internal.h b/mm/internal.h
index 22ec8d2..7ff78d6 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -195,6 +195,8 @@ static inline struct page *mem_map_next(struct page *iter,
#define __paginginit __init
#endif
+int gfp_has_no_watermarks(gfp_t gfp_mask);
+
/* Memory initialisation debug and verification */
enum mminit_level {
MMINIT_WARNING,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bf72055..4b4292a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1782,6 +1782,11 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
return alloc_flags;
}
+int gfp_has_no_watermarks(gfp_t gfp_mask)
+{
+ return (gfp_to_alloc_flags(gfp_mask) & ALLOC_NO_WATERMARKS);
+}
+
static inline struct page *
__alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
struct zonelist *zonelist, enum zone_type high_zoneidx,
David Rientjes wrote:
> On Thu, 1 Oct 2009, Suresh Jayaraman wrote:
>
>> From: Peter Zijlstra <[email protected]>
>>
>> Expose the gfp to alloc_flags mapping, so we can use it in other parts
>> of the vm.
>>
>> Signed-off-by: Peter Zijlstra <[email protected]>
>> Signed-off-by: Suresh Jayaraman <[email protected]>
>
> Nack, these flags are internal to the page allocator and exporting them to
> generic VM code is unnecessary.
Yes, you're right.
> The only bit you actually use in your patchset is ALLOC_NO_WATERMARKS to
> determine whether a particular allocation can use memory reserves. I'd
> suggest adding a bool function that returns whether the current context is
> given access to reserves including your new __GFP_MEMALLOC flag and
> exporting that instead.
Makes sense and Neil already posted a patch citing the suggested
changes, will incorporate the change.
Thanks,
--
Suresh Jayaraman
On Fri, 2 Oct 2009, Neil Brown wrote:
> So something like this?
> Then change every occurrence of
> + if (!(gfp_to_alloc_flags(gfpflags) & ALLOC_NO_WATERMARKS))
> to
> + if (!(gfp_has_no_watermarks(gfpflags)))
>
> ??
>
No, it's not even necessary to call gfp_to_alloc_flags() at all, just
create a globally exported function such as can_alloc_use_reserves() and
use it in gfp_to_alloc_flags().
[ Using 'p' in gfp_to_alloc_flags() is actually wrong since
test_thread_flag() only works on current anyway, so it would be
inconsistent if p were set to anything other than current; we can
get rid of that auto variable. ]
Something like the following, which you can fold into this patch proposal
and modify later for GFP_MEMALLOC.
Signed-off-by: David Rientjes <[email protected]>
---
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 557bdad..7dd62a0 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -265,6 +265,8 @@ static inline void arch_free_page(struct page *page, int order) { }
static inline void arch_alloc_page(struct page *page, int order) { }
#endif
+int can_alloc_use_reserves(void);
+
struct page *
__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
struct zonelist *zonelist, nodemask_t *nodemask);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bf72055..cf1d765 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1744,10 +1744,19 @@ void wake_all_kswapd(unsigned int order, struct zonelist *zonelist,
wakeup_kswapd(zone, order);
}
+/*
+ * Does the current context allow the allocation to utilize memory reserves
+ * by ignoring watermarks for all zones?
+ */
+int can_alloc_use_reserves(void)
+{
+ return !in_interrupt() && ((current->flags & PF_MEMALLOC) ||
+ unlikely(test_thread_flag(TIF_MEMDIE)));
+}
+
static inline int
gfp_to_alloc_flags(gfp_t gfp_mask)
{
- struct task_struct *p = current;
int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
const gfp_t wait = gfp_mask & __GFP_WAIT;
@@ -1769,15 +1778,12 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
* See also cpuset_zone_allowed() comment in kernel/cpuset.c.
*/
alloc_flags &= ~ALLOC_CPUSET;
- } else if (unlikely(rt_task(p)))
+ } else if (unlikely(rt_task(current)))
alloc_flags |= ALLOC_HARDER;
- if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
- if (!in_interrupt() &&
- ((p->flags & PF_MEMALLOC) ||
- unlikely(test_thread_flag(TIF_MEMDIE))))
+ if (likely(!(gfp_mask & __GFP_NOMEMALLOC)))
+ if (can_alloc_use_reserves())
alloc_flags |= ALLOC_NO_WATERMARKS;
- }
return alloc_flags;
}