All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Minchan Kim <minchan@kernel.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@gmail.com>,
	Aaditya Kumar <aaditya.kumar.30@gmail.com>,
	Mel Gorman <mel@csn.ul.ie>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: Accounting problem of MIGRATE_ISOLATED freed page
Date: Sat, 23 Jun 2012 13:38:04 +0900	[thread overview]
Message-ID: <4FE5482C.3010501@jp.fujitsu.com> (raw)
In-Reply-To: <4FE414A2.3000700@kernel.org>

(2012/06/22 15:45), Minchan Kim wrote:
> On 06/22/2012 10:05 AM, Minchan Kim wrote:
>
>> Second approach which is suggested by KOSAKI is what you mentioned.
>> But the concern about second approach is how to make sure matched count increase/decrease of nr_isolated_areas.
>> I mean how to make sure nr_isolated_areas would be zero when isolation is done.
>> Of course, we can investigate all of current caller and make sure they don't make mistake
>> now. But it's very error-prone if we consider future's user.
>> So we might need test_set_pageblock_migratetype(page, MIGRATE_ISOLATE);
>
>
> It's an implementation about above approach.
>

I like this approach.


> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index bf3404e..3e9a9e1 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -474,6 +474,11 @@ struct zone {
>           * rarely used fields:
>           */
>          const char              *name;
> +       /*
> +        * the number of MIGRATE_ISOLATE pageblock
> +        * We need this for accurate free page counting.
> +        */
> +       atomic_t                nr_migrate_isolate;
>   } ____cacheline_internodealigned_in_smp;

Isn't this counter modified only under zone->lock ?


>
>   typedef enum {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2c29b1c..6cb1f9f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -219,6 +219,11 @@ EXPORT_SYMBOL(nr_online_nodes);
>
>   int page_group_by_mobility_disabled __read_mostly;
>
> +/*
> + * NOTE:
> + * Don't use set_pageblock_migratetype(page, MIGRATE_ISOLATE) direclty.
> + * Instead, use {un}set_pageblock_isolate.
> + */
>   void set_pageblock_migratetype(struct page *page, int migratetype)
>   {
>          if (unlikely(page_group_by_mobility_disabled))
> @@ -1622,6 +1627,28 @@ bool zone_watermark_ok(struct zone *z, int order, unsigned long mark,
>                                          zone_page_state(z, NR_FREE_PAGES));
>   }

I'm glad if this function can be static...Hm. With easy grep, I think it can be...

Thanks,
-Kame

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

WARNING: multiple messages have this Message-ID (diff)
From: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Minchan Kim <minchan@kernel.org>
Cc: undisclosed-recipients:"; KOSAKI Motohiro"
	<kosaki.motohiro@gmail.com>,
	Aaditya Kumar <aaditya.kumar.30@gmail.com>,
	Mel Gorman <mel@csn.ul.ie>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: Accounting problem of MIGRATE_ISOLATED freed page
Date: Sat, 23 Jun 2012 13:38:04 +0900	[thread overview]
Message-ID: <4FE5482C.3010501@jp.fujitsu.com> (raw)
In-Reply-To: <4FE414A2.3000700@kernel.org>

(2012/06/22 15:45), Minchan Kim wrote:
> On 06/22/2012 10:05 AM, Minchan Kim wrote:
>
>> Second approach which is suggested by KOSAKI is what you mentioned.
>> But the concern about second approach is how to make sure matched count increase/decrease of nr_isolated_areas.
>> I mean how to make sure nr_isolated_areas would be zero when isolation is done.
>> Of course, we can investigate all of current caller and make sure they don't make mistake
>> now. But it's very error-prone if we consider future's user.
>> So we might need test_set_pageblock_migratetype(page, MIGRATE_ISOLATE);
>
>
> It's an implementation about above approach.
>

I like this approach.


> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index bf3404e..3e9a9e1 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -474,6 +474,11 @@ struct zone {
>           * rarely used fields:
>           */
>          const char              *name;
> +       /*
> +        * the number of MIGRATE_ISOLATE pageblock
> +        * We need this for accurate free page counting.
> +        */
> +       atomic_t                nr_migrate_isolate;
>   } ____cacheline_internodealigned_in_smp;

Isn't this counter modified only under zone->lock ?


>
>   typedef enum {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2c29b1c..6cb1f9f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -219,6 +219,11 @@ EXPORT_SYMBOL(nr_online_nodes);
>
>   int page_group_by_mobility_disabled __read_mostly;
>
> +/*
> + * NOTE:
> + * Don't use set_pageblock_migratetype(page, MIGRATE_ISOLATE) direclty.
> + * Instead, use {un}set_pageblock_isolate.
> + */
>   void set_pageblock_migratetype(struct page *page, int migratetype)
>   {
>          if (unlikely(page_group_by_mobility_disabled))
> @@ -1622,6 +1627,28 @@ bool zone_watermark_ok(struct zone *z, int order, unsigned long mark,
>                                          zone_page_state(z, NR_FREE_PAGES));
>   }

I'm glad if this function can be static...Hm. With easy grep, I think it can be...

Thanks,
-Kame


  parent reply	other threads:[~2012-06-23  4:40 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-20  6:12 Accounting problem of MIGRATE_ISOLATED freed page Minchan Kim
2012-06-20  6:32 ` KOSAKI Motohiro
2012-06-20  6:32   ` KOSAKI Motohiro
2012-06-20  7:53   ` Minchan Kim
2012-06-20  7:53     ` Minchan Kim
2012-06-20 12:44     ` Hillf Danton
2012-06-20 12:44       ` Hillf Danton
2012-06-20 23:58       ` Minchan Kim
2012-06-20 23:58         ` Minchan Kim
2012-06-20 20:19     ` KOSAKI Motohiro
2012-06-20 20:19       ` KOSAKI Motohiro
2012-06-21  0:01       ` Minchan Kim
2012-06-21  0:01         ` Minchan Kim
2012-06-21  1:39         ` KOSAKI Motohiro
2012-06-21  1:39           ` KOSAKI Motohiro
2012-06-21  1:55           ` Minchan Kim
2012-06-21  1:55             ` Minchan Kim
2012-06-21  2:45             ` KOSAKI Motohiro
2012-06-21  2:45               ` KOSAKI Motohiro
2012-06-21  4:55               ` Minchan Kim
2012-06-21  4:55                 ` Minchan Kim
2012-06-21 10:52                 ` Kamezawa Hiroyuki
2012-06-21 10:52                   ` Kamezawa Hiroyuki
2012-06-21 17:22                   ` KOSAKI Motohiro
2012-06-21 17:22                     ` KOSAKI Motohiro
2012-06-22  1:05                   ` Minchan Kim
2012-06-22  1:05                     ` Minchan Kim
2012-06-22  6:45                     ` Minchan Kim
2012-06-22  6:45                       ` Minchan Kim
2012-06-23  2:56                       ` KOSAKI Motohiro
2012-06-23  2:56                         ` KOSAKI Motohiro
2012-06-25  1:10                         ` Minchan Kim
2012-06-25  1:10                           ` Minchan Kim
2012-06-23  2:59                       ` KOSAKI Motohiro
2012-06-23  2:59                         ` KOSAKI Motohiro
2012-06-25  1:19                         ` Minchan Kim
2012-06-25  1:19                           ` Minchan Kim
2012-06-23  4:38                       ` Kamezawa Hiroyuki [this message]
2012-06-23  4:38                         ` Kamezawa Hiroyuki
2012-06-25  1:01                         ` Minchan Kim
2012-06-25  1:01                           ` Minchan Kim
2012-06-25  4:18                           ` Minchan Kim
2012-06-25  4:18                             ` Minchan Kim
2012-06-22  7:22                     ` KOSAKI Motohiro
2012-06-22  7:22                       ` KOSAKI Motohiro
2012-06-22  7:56                       ` Aaditya Kumar
2012-06-22  7:56                         ` Aaditya Kumar
2012-06-22  8:13                         ` KOSAKI Motohiro
2012-06-22  8:13                           ` KOSAKI Motohiro
2012-06-21 11:02                 ` Aaditya Kumar
2012-06-21 11:02                   ` Aaditya Kumar
2012-06-22  1:20                   ` Minchan Kim
2012-06-22  1:20                     ` Minchan Kim
2012-06-22  2:08                     ` Aaditya Kumar
2012-06-22  2:08                       ` Aaditya Kumar

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=4FE5482C.3010501@jp.fujitsu.com \
    --to=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=aaditya.kumar.30@gmail.com \
    --cc=kosaki.motohiro@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=minchan@kernel.org \
    /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.