* re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
@ 2013-03-27 6:01 Dan Carpenter
2013-03-27 16:55 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2013-03-27 6:01 UTC (permalink / raw)
To: mgorman; +Cc: linux-mm
Hello Mel Gorman,
The patch 290d1a3ce0ec: "mm: page_alloc: avoid marking zones full
prematurely after zone_reclaim()" from Mar 23, 2013, leads to the
following warning:
"mm/page_alloc.c:1957 get_page_from_freelist()
warn: bitwise AND condition is false here"
mm/page_alloc.c
1948 /*
1949 * Failed to reclaim enough to meet watermark.
1950 * Only mark the zone full if checking the min
1951 * watermark or if we failed to reclaim just
1952 * 1<<order pages or else the page allocator
1953 * fastpath will prematurely mark zones full
1954 * when the watermark is between the low and
1955 * min watermarks.
1956 */
1957 if ((alloc_flags & ALLOC_WMARK_MIN) ||
^^^^^^^^^^^^^^^
This is zero.
1958 ret == ZONE_RECLAIM_SOME)
1959 goto this_zone_full;
[snip]
2333 static inline int
2334 gfp_to_alloc_flags(gfp_t gfp_mask)
2335 {
2336 int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
^^^^^^^^^^^^^^^
2337 const gfp_t wait = gfp_mask & __GFP_WAIT;
regards,
dan carpenter
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
2013-03-27 6:01 mm: page_alloc: avoid marking zones full prematurely after zone_reclaim() Dan Carpenter
@ 2013-03-27 16:55 ` Michal Hocko
2013-03-27 17:13 ` Michal Hocko
2013-04-01 11:13 ` Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Michal Hocko @ 2013-03-27 16:55 UTC (permalink / raw)
To: Dan Carpenter; +Cc: mgorman, linux-mm, Andrew Morton
[Adding Andrew into CC]
Hi Dan,
On Wed 27-03-13 09:01:42, Dan Carpenter wrote:
> Hello Mel Gorman,
>
> The patch 290d1a3ce0ec: "mm: page_alloc: avoid marking zones full
> prematurely after zone_reclaim()" from Mar 23, 2013, leads to the
> following warning:
> "mm/page_alloc.c:1957 get_page_from_freelist()
> warn: bitwise AND condition is false here"
Dohh, I have totally missed this during review and I managed to burn
myself on the similar issue in the past (gfp & GFP_NOWAIT).
The follow up fix is bellow
> mm/page_alloc.c
> 1948 /*
> 1949 * Failed to reclaim enough to meet watermark.
> 1950 * Only mark the zone full if checking the min
> 1951 * watermark or if we failed to reclaim just
> 1952 * 1<<order pages or else the page allocator
> 1953 * fastpath will prematurely mark zones full
> 1954 * when the watermark is between the low and
> 1955 * min watermarks.
> 1956 */
> 1957 if ((alloc_flags & ALLOC_WMARK_MIN) ||
> ^^^^^^^^^^^^^^^
> This is zero.
>
> 1958 ret == ZONE_RECLAIM_SOME)
> 1959 goto this_zone_full;
>
> [snip]
>
> 2333 static inline int
> 2334 gfp_to_alloc_flags(gfp_t gfp_mask)
> 2335 {
> 2336 int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
> ^^^^^^^^^^^^^^^
> 2337 const gfp_t wait = gfp_mask & __GFP_WAIT;
---
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
2013-03-27 16:55 ` Michal Hocko
@ 2013-03-27 17:13 ` Michal Hocko
2013-04-01 11:13 ` Dan Carpenter
1 sibling, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2013-03-27 17:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: mgorman, linux-mm, Andrew Morton
And just for record. I wanted to give credit to Dan for reporting this
but I have no idea how when this gets merged into the original patch
which is still sitting in the -mm queue. I will leave that to Andrew ;)
On Wed 27-03-13 17:55:56, Michal Hocko wrote:
> From b60e75c65b855a0df827a28a509e6761b4cf45dd Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Wed, 27 Mar 2013 17:53:09 +0100
> Subject: [PATCH] mm:
> mm-page_alloc-avoid-marking-zones-full-prematurely-after-zone_reclaim-fix
>
> Dan Carpenter has reported that (alloc_flags & ALLOC_WMARK_MIN) test
> doesn't make much sense as the flag is 0 and it is in fact intended for
> wmark indexing rather than being used as a flag.
>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index aa4b5c2..071e66a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1953,7 +1953,7 @@ zonelist_scan:
> * when the watermark is between the low and
> * min watermarks.
> */
> - if ((alloc_flags & ALLOC_WMARK_MIN) ||
> + if (((alloc_flags & ALLOC_WMARK_MASK) == ALLOC_WMARK_MIN) ||
> ret == ZONE_RECLAIM_SOME)
> goto this_zone_full;
>
> --
> 1.7.10.4
>
> --
> Michal Hocko
> SUSE Labs
>
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Michal Hocko
SUSE Labs
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
2013-03-27 16:55 ` Michal Hocko
2013-03-27 17:13 ` Michal Hocko
@ 2013-04-01 11:13 ` Dan Carpenter
2013-04-02 9:55 ` Michal Hocko
2013-04-02 10:37 ` Mel Gorman
1 sibling, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-04-01 11:13 UTC (permalink / raw)
To: Michal Hocko; +Cc: mgorman, linux-mm, Andrew Morton
I still don't understand the code in gfp_to_alloc_flags().
int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
ORing with zero is odd.
regards,
dan carpenter
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
2013-04-01 11:13 ` Dan Carpenter
@ 2013-04-02 9:55 ` Michal Hocko
2013-04-02 10:37 ` Mel Gorman
1 sibling, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2013-04-02 9:55 UTC (permalink / raw)
To: Dan Carpenter; +Cc: mgorman, linux-mm, Andrew Morton
On Mon 01-04-13 14:13:24, Dan Carpenter wrote:
> I still don't understand the code in gfp_to_alloc_flags().
>
> int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
>
> ORing with zero is odd.
It is a named zero ;) Something similar like GFP_NOWAIT AFAIU.
--
Michal Hocko
SUSE Labs
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mm: page_alloc: avoid marking zones full prematurely after zone_reclaim()
2013-04-01 11:13 ` Dan Carpenter
2013-04-02 9:55 ` Michal Hocko
@ 2013-04-02 10:37 ` Mel Gorman
1 sibling, 0 replies; 6+ messages in thread
From: Mel Gorman @ 2013-04-02 10:37 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Michal Hocko, linux-mm, Andrew Morton
On Mon, Apr 01, 2013 at 02:13:24PM +0300, Dan Carpenter wrote:
> I still don't understand the code in gfp_to_alloc_flags().
>
> int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
>
> ORing with zero is odd.
>
Thanks Dan for the report and thanks Michal for fixing it. I was offline
for last week which lead to my tardy response.
The odditiy is that ALLOC_WMARK_MIN is not treated as a flag but as an
offset within the zone->wmark so it starts as 0. It could have been
written as
int alloc_flags = ALLOC_CPUSET;
but then it would be easy to forget that in this path we are using the
MIN watermark.
The "flag" is used as an offset because it eliminated a number of
branches in the page allocator and was a micro-optimisation at the time.
--
Mel Gorman
SUSE Labs
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-02 10:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-27 6:01 mm: page_alloc: avoid marking zones full prematurely after zone_reclaim() Dan Carpenter
2013-03-27 16:55 ` Michal Hocko
2013-03-27 17:13 ` Michal Hocko
2013-04-01 11:13 ` Dan Carpenter
2013-04-02 9:55 ` Michal Hocko
2013-04-02 10:37 ` Mel Gorman
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.