All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Christoph Lameter <clameter@sgi.com>
Cc: Andi Kleen <ak@suse.de>, Mel Gorman <mel@skynet.ie>,
	Lee.Schermerhorn@hp.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH 3/4] Embed zone_id information within the zonelist->zones pointer
Date: Tue, 14 Aug 2007 01:42:17 +0200	[thread overview]
Message-ID: <20070813234217.GI3406@bingen.suse.de> (raw)
In-Reply-To: <Pine.LNX.4.64.0708131518320.28626@schroedinger.engr.sgi.com>

On Mon, Aug 13, 2007 at 03:22:25PM -0700, Christoph Lameter wrote:
> On Tue, 14 Aug 2007, Andi Kleen wrote:
> 
> > > x86_64 is the only platforms that uses ZONE_DMA32. Ia64 and other 64 bit 
> > > platforms use ZONE_DMA for <4GB allocs.
> > 
> > Yes, but ZONE_DMA32 == ZONE_DMA.
> 
> I am not sure what you mean by that. Ia64 ZONE_DMA == x86_84 ZONE_DMA32?

Hmm, when I wrote GFP_DMA32 it was a #define GFP_DMA32 GFP_DMA 
on ia64 so that drivers not need to ifdef.  Someone nasty
seems to have removed that too. I guess it would be best
to readd.

> 
> > Also when the slab users of GFP_DMA are all gone ia64 won't need
> > the slab support anymore. So either you change your ifdef in slub or 
> > switch to ZONE_DMA32 for IA64.
> 
> If you have gotten rid of all slab users of GFP_DMA (and also all arch 
> uses of it) then we can drop the code in SLAB.

No, e.g. s390 and some other architectures still use it.
You'll need to bug their respective maintainers.


> 1. Drop sl?b support for GFP_DMA.

Not yet.

> 
> 2. Drop GFP_DMA32 support.
> 
> Then we only allow page allocator allocs using GFP_DMA? That may be the 

Kind of yes.

> least invasive for arch code.

I would prefer for GFP_DMA to go away on x86 (but GFP_DMA32 stay). This way
we get clean compile errors instead of subtle breakage. Silently
changing the semantics would be bad.

But then it wouldn't make sense to have GFP_DMA on ia64 and GFP_DMA32
on x86. Since driver writers are more likely to test on x86
I would recommend ia64 having compatible semantics. It'll
save everybody trouble long term. This means it wouldn't 
help on IA64 machines that don't have a DMA zone -- they
would still need to validate drivers especially -- but at least
the others.

Also from my driver review driver authors often seem to have
trouble understanding what GFP_DMA really does. With GFP_DMA32 it 
is clearer that it applies to a address range and is not
some synonym for pci_map_*

-Andi


WARNING: multiple messages have this Message-ID (diff)
From: Andi Kleen <ak@suse.de>
To: Christoph Lameter <clameter@sgi.com>
Cc: Andi Kleen <ak@suse.de>, Mel Gorman <mel@skynet.ie>,
	Lee.Schermerhorn@hp.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH 3/4] Embed zone_id information within the zonelist->zones pointer
Date: Tue, 14 Aug 2007 01:42:17 +0200	[thread overview]
Message-ID: <20070813234217.GI3406@bingen.suse.de> (raw)
In-Reply-To: <Pine.LNX.4.64.0708131518320.28626@schroedinger.engr.sgi.com>

On Mon, Aug 13, 2007 at 03:22:25PM -0700, Christoph Lameter wrote:
> On Tue, 14 Aug 2007, Andi Kleen wrote:
> 
> > > x86_64 is the only platforms that uses ZONE_DMA32. Ia64 and other 64 bit 
> > > platforms use ZONE_DMA for <4GB allocs.
> > 
> > Yes, but ZONE_DMA32 == ZONE_DMA.
> 
> I am not sure what you mean by that. Ia64 ZONE_DMA == x86_84 ZONE_DMA32?

Hmm, when I wrote GFP_DMA32 it was a #define GFP_DMA32 GFP_DMA 
on ia64 so that drivers not need to ifdef.  Someone nasty
seems to have removed that too. I guess it would be best
to readd.

> 
> > Also when the slab users of GFP_DMA are all gone ia64 won't need
> > the slab support anymore. So either you change your ifdef in slub or 
> > switch to ZONE_DMA32 for IA64.
> 
> If you have gotten rid of all slab users of GFP_DMA (and also all arch 
> uses of it) then we can drop the code in SLAB.

No, e.g. s390 and some other architectures still use it.
You'll need to bug their respective maintainers.


> 1. Drop sl?b support for GFP_DMA.

Not yet.

> 
> 2. Drop GFP_DMA32 support.
> 
> Then we only allow page allocator allocs using GFP_DMA? That may be the 

Kind of yes.

> least invasive for arch code.

I would prefer for GFP_DMA to go away on x86 (but GFP_DMA32 stay). This way
we get clean compile errors instead of subtle breakage. Silently
changing the semantics would be bad.

But then it wouldn't make sense to have GFP_DMA on ia64 and GFP_DMA32
on x86. Since driver writers are more likely to test on x86
I would recommend ia64 having compatible semantics. It'll
save everybody trouble long term. This means it wouldn't 
help on IA64 machines that don't have a DMA zone -- they
would still need to validate drivers especially -- but at least
the others.

Also from my driver review driver authors often seem to have
trouble understanding what GFP_DMA really does. With GFP_DMA32 it 
is clearer that it applies to a address range and is not
some synonym for pci_map_*

-Andi

--
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:[~2007-08-13 22:48 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-09 21:06 [PATCH 0/4] Use one zonelist per node instead of multiple zonelists v3 Mel Gorman
2007-08-09 21:06 ` Mel Gorman
2007-08-09 21:06 ` [PATCH 1/4] Use zonelists instead of zones when direct reclaiming pages Mel Gorman
2007-08-09 21:06   ` Mel Gorman
2007-08-09 21:19   ` Christoph Lameter
2007-08-09 21:19     ` Christoph Lameter
2007-08-09 21:06 ` [PATCH 2/4] Use one zonelist that is filtered instead of multiple zonelists Mel Gorman
2007-08-09 21:06   ` Mel Gorman
2007-08-09 21:27   ` Christoph Lameter
2007-08-09 21:27     ` Christoph Lameter
2007-08-09 21:07 ` [PATCH 3/4] Embed zone_id information within the zonelist->zones pointer Mel Gorman
2007-08-09 21:07   ` Mel Gorman
2007-08-09 21:37   ` Christoph Lameter
2007-08-09 21:37     ` Christoph Lameter
2007-08-09 23:33     ` Mel Gorman
2007-08-09 23:33       ` Mel Gorman
2007-08-10  1:44       ` Christoph Lameter
2007-08-10  1:44         ` Christoph Lameter
2007-08-10 10:47         ` Mel Gorman
2007-08-10 10:47           ` Mel Gorman
2007-08-10 17:37           ` Christoph Lameter
2007-08-10 17:37             ` Christoph Lameter
2007-08-10 18:13             ` Andi Kleen
2007-08-10 18:13               ` Andi Kleen
2007-08-10 19:02               ` Christoph Lameter
2007-08-10 19:02                 ` Christoph Lameter
2007-08-11  1:04                 ` Andi Kleen
2007-08-11  1:04                   ` Andi Kleen
2007-08-13 21:25                   ` Christoph Lameter
2007-08-13 21:25                     ` Christoph Lameter
2007-08-13 22:50                     ` Andi Kleen
2007-08-13 22:50                       ` Andi Kleen
2007-08-13 22:00                       ` Christoph Lameter
2007-08-13 22:00                         ` Christoph Lameter
2007-08-13 22:58                         ` Andi Kleen
2007-08-13 22:58                           ` Andi Kleen
2007-08-13 22:09                           ` Christoph Lameter
2007-08-13 22:09                             ` Christoph Lameter
2007-08-13 23:08                             ` Andi Kleen
2007-08-13 23:08                               ` Andi Kleen
2007-08-13 22:22                               ` Christoph Lameter
2007-08-13 22:22                                 ` Christoph Lameter
2007-08-13 23:42                                 ` Andi Kleen [this message]
2007-08-13 23:42                                   ` Andi Kleen
2007-08-13 22:52                                   ` Christoph Lameter
2007-08-13 22:52                                     ` Christoph Lameter
2007-08-13 23:55                                     ` Andi Kleen
2007-08-13 23:55                                       ` Andi Kleen
2007-08-13 23:12                                       ` Christoph Lameter
2007-08-13 23:12                                         ` Christoph Lameter
2007-08-14  0:16                                         ` Andi Kleen
2007-08-14  0:16                                           ` Andi Kleen
2007-08-13 23:25                                           ` Christoph Lameter
2007-08-13 23:25                                             ` Christoph Lameter
2007-08-14  0:25                                             ` Andi Kleen
2007-08-14  0:25                                               ` Andi Kleen
2007-08-13 22:38                               ` Christoph Lameter
2007-08-13 22:38                                 ` Christoph Lameter
2007-08-13 23:43                                 ` Andi Kleen
2007-08-13 23:43                                   ` Andi Kleen
2007-08-13 22:54                                   ` Christoph Lameter
2007-08-13 22:54                                     ` Christoph Lameter
2007-08-14  0:00                                     ` Andi Kleen
2007-08-14  0:00                                       ` Andi Kleen
2007-08-13 23:16                                       ` Christoph Lameter
2007-08-13 23:16                                         ` Christoph Lameter
2007-08-14  0:16                                         ` Andi Kleen
2007-08-14  0:16                                           ` Andi Kleen
2007-08-13 23:27                                           ` Christoph Lameter
2007-08-13 23:27                                             ` Christoph Lameter
2007-08-14  0:26                                             ` Andi Kleen
2007-08-14  0:26                                               ` Andi Kleen
2007-08-14 19:56                                               ` Christoph Lameter
2007-08-14 19:56                                                 ` Christoph Lameter
2007-08-13 23:22                                       ` Alan Cox
2007-08-13 23:22                                         ` Alan Cox
2007-08-14  0:14                                         ` Andi Kleen
2007-08-14  0:14                                           ` Andi Kleen
2007-08-13 23:44                                           ` Alan Cox
2007-08-13 23:44                                             ` Alan Cox
2007-08-14 19:11                                           ` Andy Isaacson
2007-08-14 19:11                                             ` Andy Isaacson
2007-08-14 20:23                                             ` Andi Kleen
2007-08-14 20:23                                               ` Andi Kleen
2007-08-14 19:43                                               ` Andy Isaacson
2007-08-14 19:43                                                 ` Andy Isaacson
2007-08-14 21:05                                                 ` Andi Kleen
2007-08-14 21:05                                                   ` Andi Kleen
2007-08-15 11:37                                           ` Ralf Baechle
2007-08-15 11:37                                             ` Ralf Baechle
2007-08-15 12:59                                             ` Andi Kleen
2007-08-15 12:59                                               ` Andi Kleen
2007-08-15 12:32                                               ` Ralf Baechle
2007-08-15 12:32                                                 ` Ralf Baechle
2007-08-15 19:59                                               ` Christoph Lameter
2007-08-15 19:59                                                 ` Christoph Lameter
2007-08-09 21:07 ` [PATCH 4/4] Apply MPOL_BIND policy to two highest zones when highest is ZONE_MOVABLE Mel Gorman
2007-08-09 21:07   ` Mel Gorman
2007-08-09 21:37   ` Christoph Lameter
2007-08-09 21:37     ` Christoph Lameter
2007-08-09 21:19 ` [PATCH 0/4] Use one zonelist per node instead of multiple zonelists v3 Christoph Lameter
2007-08-09 21:19   ` 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=20070813234217.GI3406@bingen.suse.de \
    --to=ak@suse.de \
    --cc=Lee.Schermerhorn@hp.com \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@skynet.ie \
    /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.