From: Tang Chen <tangchen@cn.fujitsu.com>
To: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
akpm@linux-foundation.org, santosh.shilimkar@ti.com,
grygorii.strashko@ti.com, yinghai@kernel.org,
isimatu.yasuaki@jp.fujitsu.co, fabf@skynet.be, nzimmer@sgi.com,
wangnan0@huawei.com, vdavydov@parallels.com, toshi.kani@hp.com,
phacht@linux.vnet.ibm.com, tj@kernel.org,
kirill.shutemov@linux.intel.com, riel@redhat.com,
luto@amacapital.net, hpa@linux.intel.com, aarcange@redhat.com,
qiuxishi@huawei.com, mgorman@suse.de, rientjes@google.com,
hannes@cmpxchg.org
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Gu Zheng <guz.fnst@cn.fujitsu.com>,
jiang.liu@linux.intel.com
Subject: Re: [PATCH 2/2] mem-hotplug: Fix wrong check for zone->pageset initialization in online_pages().
Date: Wed, 5 Nov 2014 10:17:34 +0800 [thread overview]
Message-ID: <545988BE.3050201@cn.fujitsu.com> (raw)
In-Reply-To: <545976F9.50503@jp.fujitsu.com>
On 11/05/2014 09:01 AM, Kamezawa Hiroyuki wrote:
> ......
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 3ab01b2..bc0de0f 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1013,9 +1013,13 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
> * If this zone is not populated, then it is not in zonelist.
> * This means the page allocator ignores this zone.
> * So, zonelist must be updated after online.
> + *
> + * If this zone is populated, zone->pageset could be initialized
> + * to boot_pageset for the first time a node is added. If so,
> + * zone->pageset should be allocated.
> */
> mutex_lock(&zonelists_mutex);
> - if (!populated_zone(zone)) {
> + if (!populated_zone(zone) || !zone_pcp_initialized(zone)) {
> Please don't add another strange meanings to zone's pcplist.
>
> If you say zone->present_pages doesn't mean zone has pages in buddy list any more,
> please rewrite all parts using zone->present_pages including populated_zone().
Adding Liu Jiang...
I think zone->managed_pages was introduced by Liu Jiang in the following
patch:
WARNING: multiple messages have this Message-ID (diff)
From: Tang Chen <tangchen@cn.fujitsu.com>
To: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
<akpm@linux-foundation.org>, <santosh.shilimkar@ti.com>,
<grygorii.strashko@ti.com>, <yinghai@kernel.org>,
<isimatu.yasuaki@jp.fujitsu.co>, <fabf@skynet.be>,
<nzimmer@sgi.com>, <wangnan0@huawei.com>,
<vdavydov@parallels.com>, <toshi.kani@hp.com>,
<phacht@linux.vnet.ibm.com>, <tj@kernel.org>,
<kirill.shutemov@linux.intel.com>, <riel@redhat.com>,
<luto@amacapital.net>, <hpa@linux.intel.com>,
<aarcange@redhat.com>, <qiuxishi@huawei.com>, <mgorman@suse.de>,
<rientjes@google.com>, <hannes@cmpxchg.org>
Cc: <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
Gu Zheng <guz.fnst@cn.fujitsu.com>, <jiang.liu@linux.intel.com>
Subject: Re: [PATCH 2/2] mem-hotplug: Fix wrong check for zone->pageset initialization in online_pages().
Date: Wed, 5 Nov 2014 10:17:34 +0800 [thread overview]
Message-ID: <545988BE.3050201@cn.fujitsu.com> (raw)
In-Reply-To: <545976F9.50503@jp.fujitsu.com>
On 11/05/2014 09:01 AM, Kamezawa Hiroyuki wrote:
> ......
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 3ab01b2..bc0de0f 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1013,9 +1013,13 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
> * If this zone is not populated, then it is not in zonelist.
> * This means the page allocator ignores this zone.
> * So, zonelist must be updated after online.
> + *
> + * If this zone is populated, zone->pageset could be initialized
> + * to boot_pageset for the first time a node is added. If so,
> + * zone->pageset should be allocated.
> */
> mutex_lock(&zonelists_mutex);
> - if (!populated_zone(zone)) {
> + if (!populated_zone(zone) || !zone_pcp_initialized(zone)) {
> Please don't add another strange meanings to zone's pcplist.
>
> If you say zone->present_pages doesn't mean zone has pages in buddy list any more,
> please rewrite all parts using zone->present_pages including populated_zone().
Adding Liu Jiang...
I think zone->managed_pages was introduced by Liu Jiang in the following
patch:
>From 9feedc9d831e18ae6d0d15aa562e5e46ba53647b Mon Sep 17 00:00:00 2001
From: Jiang Liu <liuj97@gmail.com>
Date: Wed, 12 Dec 2012 13:52:12 -0800
Subject: [PATCH 1/1] mm: introduce new field "managed_pages" to struct zone
Before this patch, zone->present_pages meant "amount of memory
(excluding holes)",
not the zone has pages in buddy system.
So I think zone->present_pages keeps its meaning as before. But it may
be abused
somewhere else, such as here.
> I think there are several parts calculates parameters based on present_pages.
>
> I myself doesn't welcome having both of compliated "managed pages" and "present pages"...
>
> How about adding
>
> static inline int managed_zone(struct zone *zone)
> {
> return (!!zone->managed_pages);
> }
>
> for this bug fix ?
Yes, adding this function could fix this problem.
And by the way, we have the following code in onine_pages():
zone->present_pages += onlined_pages;
pgdat_resize_lock(zone->zone_pgdat, &flags);
zone->zone_pgdat->node_present_pages += onlined_pages;
pgdat_resize_unlock(zone->zone_pgdat, &flags);
We should do this when the zone size is changed, not where it is now.
Will send patches for this soon.
Thanks.
>
> Other parts, using present_pages, should be considered well.
>
>
next prev parent reply other threads:[~2014-11-05 2:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-31 9:46 [PATCH 0/2] Fix node meminfo corruption Tang Chen
2014-10-31 9:46 ` Tang Chen
2014-10-31 9:46 ` [PATCH 1/2] mem-hotplug: Reset node managed pages when hot-adding a new pgdat Tang Chen
2014-10-31 9:46 ` Tang Chen
2014-11-04 1:50 ` Xishi Qiu
2014-11-04 1:50 ` Xishi Qiu
2014-10-31 9:46 ` [PATCH 2/2] mem-hotplug: Fix wrong check for zone->pageset initialization in online_pages() Tang Chen
2014-10-31 9:46 ` Tang Chen
2014-11-04 8:10 ` Yasuaki Ishimatsu
2014-11-04 8:10 ` Yasuaki Ishimatsu
2014-11-04 8:46 ` Tang Chen
2014-11-04 8:46 ` Tang Chen
2014-11-05 1:01 ` Kamezawa Hiroyuki
2014-11-05 1:01 ` Kamezawa Hiroyuki
2014-11-05 2:17 ` Tang Chen [this message]
2014-11-05 2:17 ` Tang Chen
2014-11-04 1:10 ` [PATCH 0/2] Fix node meminfo corruption Tang Chen
2014-11-04 1:10 ` Tang Chen
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=545988BE.3050201@cn.fujitsu.com \
--to=tangchen@cn.fujitsu.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=fabf@skynet.be \
--cc=grygorii.strashko@ti.com \
--cc=guz.fnst@cn.fujitsu.com \
--cc=hannes@cmpxchg.org \
--cc=hpa@linux.intel.com \
--cc=isimatu.yasuaki@jp.fujitsu.co \
--cc=jiang.liu@linux.intel.com \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@amacapital.net \
--cc=mgorman@suse.de \
--cc=nzimmer@sgi.com \
--cc=phacht@linux.vnet.ibm.com \
--cc=qiuxishi@huawei.com \
--cc=riel@redhat.com \
--cc=rientjes@google.com \
--cc=santosh.shilimkar@ti.com \
--cc=tj@kernel.org \
--cc=toshi.kani@hp.com \
--cc=vdavydov@parallels.com \
--cc=wangnan0@huawei.com \
--cc=yinghai@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.