All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: "minchan kim" <minchan.kim@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Martin Bligh <mbligh@mbligh.org>,
	Nick Piggin <nickpiggin@yahoo.com.au>
Subject: Re: [PATCH] remove duplicating priority setting in try_to_free_p
Date: Sun, 27 Jan 2008 21:33:12 -0800	[thread overview]
Message-ID: <20080127213312.517b8014.akpm@linux-foundation.org> (raw)
In-Reply-To: <28c262360801252329q7232edc2l2d0e4ed17c054832@mail.gmail.com>

On Sat, 26 Jan 2008 02:29:23 -0500 "minchan kim" <minchan.kim@gmail.com> wrote:

> shrink_zones in try_to_free_pages already set zone through
> note_zone_scanning_priority.
> So, setting prev_priority in try_to_free_pages is needless.
> 
> This patch is made by 2.6.24-rc8.
> 
> Signed-off-by: barrios <minchan.kim@gmail.com>
> ---
>  mm/vmscan.c |   17 -----------------
>  1 files changed, 0 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index e5a9597..fc55c23 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1273,23 +1273,6 @@ unsigned long try_to_free_pages(struct z
>     if (!sc.all_unreclaimable)
>         ret = 1;
>  out:
> -   /*
> -    * Now that we've scanned all the zones at this priority level, note
> -    * that level within the zone so that the next thread which performs
> -    * scanning of this zone will immediately start out at this priority
> -    * level.  This affects only the decision whether or not to bring
> -    * mapped pages onto the inactive list.
> -    */
> -   if (priority < 0)
> -       priority = 0;
> -   for (i = 0; zones[i] != NULL; i++) {
> -       struct zone *zone = zones[i];
> -
> -       if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> -           continue;
> -
> -       zone->prev_priority = priority;
> -   }
>     return ret;
>  }

(your mail client is replacing tabs with spaces)

I think this is actually a bugfix.  The code you're removing doesn't do the 

	if (priority < zone->prev_priority)

thing.

otoh with this change, the only thing which will cause prev_priority to
increase (ie: lower priority) is kswapd, which seems odd.

So:

a) this is a functional change and needs more thought and lots of runtime
   testing.  I'll duck it for now.

b) the prev_priority stuff is still screwed up.

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: minchan kim <minchan.kim@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Martin Bligh <mbligh@mbligh.org>,
	Nick Piggin <nickpiggin@yahoo.com.au>
Subject: Re: [PATCH] remove duplicating priority setting in try_to_free_p
Date: Sun, 27 Jan 2008 21:33:12 -0800	[thread overview]
Message-ID: <20080127213312.517b8014.akpm@linux-foundation.org> (raw)
In-Reply-To: <28c262360801252329q7232edc2l2d0e4ed17c054832@mail.gmail.com>

On Sat, 26 Jan 2008 02:29:23 -0500 "minchan kim" <minchan.kim@gmail.com> wrote:

> shrink_zones in try_to_free_pages already set zone through
> note_zone_scanning_priority.
> So, setting prev_priority in try_to_free_pages is needless.
> 
> This patch is made by 2.6.24-rc8.
> 
> Signed-off-by: barrios <minchan.kim@gmail.com>
> ---
>  mm/vmscan.c |   17 -----------------
>  1 files changed, 0 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index e5a9597..fc55c23 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1273,23 +1273,6 @@ unsigned long try_to_free_pages(struct z
>     if (!sc.all_unreclaimable)
>         ret = 1;
>  out:
> -   /*
> -    * Now that we've scanned all the zones at this priority level, note
> -    * that level within the zone so that the next thread which performs
> -    * scanning of this zone will immediately start out at this priority
> -    * level.  This affects only the decision whether or not to bring
> -    * mapped pages onto the inactive list.
> -    */
> -   if (priority < 0)
> -       priority = 0;
> -   for (i = 0; zones[i] != NULL; i++) {
> -       struct zone *zone = zones[i];
> -
> -       if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> -           continue;
> -
> -       zone->prev_priority = priority;
> -   }
>     return ret;
>  }

(your mail client is replacing tabs with spaces)

I think this is actually a bugfix.  The code you're removing doesn't do the 

	if (priority < zone->prev_priority)

thing.

otoh with this change, the only thing which will cause prev_priority to
increase (ie: lower priority) is kswapd, which seems odd.

So:

a) this is a functional change and needs more thought and lots of runtime
   testing.  I'll duck it for now.

b) the prev_priority stuff is still screwed up.

--
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>

  reply	other threads:[~2008-01-28  5:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-26  7:29 [PATCH] remove duplicating priority setting in try_to_free_p minchan kim
2008-01-26  7:29 ` minchan kim
2008-01-28  5:33 ` Andrew Morton [this message]
2008-01-28  5:33   ` Andrew Morton
2008-01-28  6:43   ` minchan kim
2008-01-28  6:43     ` minchan kim
2008-01-28  9:01     ` Andrew Morton
2008-01-28  9:01       ` Andrew Morton
2008-01-28 12:38       ` minchan kim
2008-01-28 12:38         ` minchan kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080127213312.517b8014.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mbligh@mbligh.org \
    --cc=minchan.kim@gmail.com \
    --cc=nickpiggin@yahoo.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.