From: Dave Hansen <haveblue@us.ibm.com>
To: Christoph Lameter <clameter@engr.sgi.com>
Cc: Andrew Morton <akpm@osdl.org>, linux-mm <linux-mm@kvack.org>,
ia64 list <linux-ia64@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Periodically drain non local pagesets
Date: Wed, 01 Jun 2005 18:46:58 +0000 [thread overview]
Message-ID: <1117651618.13600.16.camel@localhost> (raw)
In-Reply-To: <Pine.LNX.4.62.0506011047060.9277@schroedinger.engr.sgi.com>
On Wed, 2005-06-01 at 10:48 -0700, Christoph Lameter wrote:
> + struct per_cpu_pageset *pset;
> +
> + /* Do not drain local pagesets */
> + if (zone = zone_table[numa_node_id()])
> + continue;
> +
It's best to avoid using NUMA-specific data structures, even in #ifdef
NUMA code. This particular use is incorrect, as the zone_table[] is not
indexed by numa_node_id(), but rather by a combination of the node
number and the zone number (see NODEZONE()).
I'd suggest using something like this:
if (zone->zone_pgdat->node_id = numa_node_id())
It might be nice to have a zone_node_id() macro that hides this as well.
With a macro like that that #defines to 0 when !CONFIG_NUMA, the #ifdef
around that function could probably go away.
Also, are you sure that you need the local_irq_en/disable()?
-- Dave
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <haveblue@us.ibm.com>
To: Christoph Lameter <clameter@engr.sgi.com>
Cc: Andrew Morton <akpm@osdl.org>, linux-mm <linux-mm@kvack.org>,
ia64 list <linux-ia64@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Periodically drain non local pagesets
Date: Wed, 01 Jun 2005 11:46:58 -0700 [thread overview]
Message-ID: <1117651618.13600.16.camel@localhost> (raw)
In-Reply-To: <Pine.LNX.4.62.0506011047060.9277@schroedinger.engr.sgi.com>
On Wed, 2005-06-01 at 10:48 -0700, Christoph Lameter wrote:
> + struct per_cpu_pageset *pset;
> +
> + /* Do not drain local pagesets */
> + if (zone == zone_table[numa_node_id()])
> + continue;
> +
It's best to avoid using NUMA-specific data structures, even in #ifdef
NUMA code. This particular use is incorrect, as the zone_table[] is not
indexed by numa_node_id(), but rather by a combination of the node
number and the zone number (see NODEZONE()).
I'd suggest using something like this:
if (zone->zone_pgdat->node_id == numa_node_id())
It might be nice to have a zone_node_id() macro that hides this as well.
With a macro like that that #defines to 0 when !CONFIG_NUMA, the #ifdef
around that function could probably go away.
Also, are you sure that you need the local_irq_en/disable()?
-- Dave
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <haveblue@us.ibm.com>
To: Christoph Lameter <clameter@engr.sgi.com>
Cc: Andrew Morton <akpm@osdl.org>, linux-mm <linux-mm@kvack.org>,
ia64 list <linux-ia64@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Periodically drain non local pagesets
Date: Wed, 01 Jun 2005 11:46:58 -0700 [thread overview]
Message-ID: <1117651618.13600.16.camel@localhost> (raw)
In-Reply-To: <Pine.LNX.4.62.0506011047060.9277@schroedinger.engr.sgi.com>
On Wed, 2005-06-01 at 10:48 -0700, Christoph Lameter wrote:
> + struct per_cpu_pageset *pset;
> +
> + /* Do not drain local pagesets */
> + if (zone == zone_table[numa_node_id()])
> + continue;
> +
It's best to avoid using NUMA-specific data structures, even in #ifdef
NUMA code. This particular use is incorrect, as the zone_table[] is not
indexed by numa_node_id(), but rather by a combination of the node
number and the zone number (see NODEZONE()).
I'd suggest using something like this:
if (zone->zone_pgdat->node_id == numa_node_id())
It might be nice to have a zone_node_id() macro that hides this as well.
With a macro like that that #defines to 0 when !CONFIG_NUMA, the #ifdef
around that function could probably go away.
Also, are you sure that you need the local_irq_en/disable()?
-- Dave
--
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:"aart@kvack.org"> aart@kvack.org </a>
next prev parent reply other threads:[~2005-06-01 18:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-06-01 17:48 [PATCH] Periodically drain non local pagesets Christoph Lameter
2005-06-01 17:48 ` Christoph Lameter
2005-06-01 17:48 ` Christoph Lameter
2005-06-01 18:46 ` Dave Hansen [this message]
2005-06-01 18:46 ` Dave Hansen
2005-06-01 18:46 ` Dave Hansen
2005-06-01 18:59 ` Christoph Lameter
2005-06-01 18:59 ` Christoph Lameter
2005-06-01 18:59 ` Christoph Lameter
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=1117651618.13600.16.camel@localhost \
--to=haveblue@us.ibm.com \
--cc=akpm@osdl.org \
--cc=clameter@engr.sgi.com \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.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.