IrDA driver fails on PXA255

IrDA driver fails on PXA255

[Dmitry Eremin-Solenikov]

Hello,

Since a197b59ae6e8bee56fcef37ea2482dc08414e2ac (mm: fail GFP_DMA
allocations when ZONE_DMA is not configured), pxaficp_ir.c driver fails
to probe: pxa_irda_init_iobuf asks for a buffer with GFP_KERNEL |
GFP_DMA flags, which fail nicely with the following trace:

------------[ cut here ]------------
WARNING: at mm/page_alloc.c:2251
__alloc_pages_nodemask+0xa0/0x5ac()
Modules linked in:
[<c00385b0>] (unwind_backtrace+0x0/0xf0) from [<c0050b1c>] (warn_slowpath_common+0x4c/0x64)
[<c0050b1c>] (warn_slowpath_common+0x4c/0x64) from [<c0050b4c>] (warn_slowpath_null+0x18/0x1c)
[<c0050b4c>] (warn_slowpath_null+0x18/0x1c) from [<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac)
[<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac) from [<c0090e74>] (__get_free_pages+0x10/0x3c)
[<c0090e74>] (__get_free_pages+0x10/0x3c) from [<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48)
[<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48) from [<c01d61d8>] (pxa_irda_probe+0x11c/0x32c)
[<c01d61d8>] (pxa_irda_probe+0x11c/0x32c) from [<c019474c>] (platform_drv_probe+0x14/0x18)
[<c019474c>] (platform_drv_probe+0x14/0x18) from [<c0193508>] (really_probe+0xa0/0x158)
[<c0193508>] (really_probe+0xa0/0x158) from [<c019360c>] (driver_probe_device+0x4c/0x64)
[<c019360c>] (driver_probe_device+0x4c/0x64) from [<c0193684>] (__driver_attach+0x60/0x84)
[<c0193684>] (__driver_attach+0x60/0x84) from [<c0192d78>] (bus_for_each_dev+0x48/0x84)
[<c0192d78>] (bus_for_each_dev+0x48/0x84) from [<c01926b8>] (bus_add_driver+0xa8/0x220)
[<c01926b8>] (bus_add_driver+0xa8/0x220) from [<c0193c7c>] (driver_register+0xac/0x13c)
[<c0193c7c>] (driver_register+0xac/0x13c) from [<c0033440>] (do_one_initcall+0x94/0x16c)
[<c0033440>] (do_one_initcall+0x94/0x16c) from [<c00083f4>] (kernel_init+0x94/0x140)
[<c00083f4>] (kernel_init+0x94/0x140) from [<c00348d0>] (kernel_thread_exit+0x0/0x8)
---[ end trace 0b8bf08f70147098 ]---

And then I get:

pxa2xx-ir: probe of pxa2xx-ir failed with error -12

Of course one can ask for a buffer w/o GFP_DMA (see attachment), but I
ain't sure that it's correct.

-- 
With best wishes
Dmitry

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Dmitry Eremin-Solenikov wrote:

> Hello,
> 
> Since a197b59ae6e8bee56fcef37ea2482dc08414e2ac (mm: fail GFP_DMA
> allocations when ZONE_DMA is not configured), pxaficp_ir.c driver fails
> to probe: pxa_irda_init_iobuf asks for a buffer with GFP_KERNEL |
> GFP_DMA flags, which fail nicely with the following trace:
> 
> ------------[ cut here ]------------
> WARNING: at mm/page_alloc.c:2251
> __alloc_pages_nodemask+0xa0/0x5ac()
> Modules linked in:
> [<c00385b0>] (unwind_backtrace+0x0/0xf0) from [<c0050b1c>] (warn_slowpath_common+0x4c/0x64)
> [<c0050b1c>] (warn_slowpath_common+0x4c/0x64) from [<c0050b4c>] (warn_slowpath_null+0x18/0x1c)
> [<c0050b4c>] (warn_slowpath_null+0x18/0x1c) from [<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac)
> [<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac) from [<c0090e74>] (__get_free_pages+0x10/0x3c)
> [<c0090e74>] (__get_free_pages+0x10/0x3c) from [<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48)
> [<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48) from [<c01d61d8>] (pxa_irda_probe+0x11c/0x32c)
> [<c01d61d8>] (pxa_irda_probe+0x11c/0x32c) from [<c019474c>] (platform_drv_probe+0x14/0x18)
> [<c019474c>] (platform_drv_probe+0x14/0x18) from [<c0193508>] (really_probe+0xa0/0x158)
> [<c0193508>] (really_probe+0xa0/0x158) from [<c019360c>] (driver_probe_device+0x4c/0x64)
> [<c019360c>] (driver_probe_device+0x4c/0x64) from [<c0193684>] (__driver_attach+0x60/0x84)
> [<c0193684>] (__driver_attach+0x60/0x84) from [<c0192d78>] (bus_for_each_dev+0x48/0x84)
> [<c0192d78>] (bus_for_each_dev+0x48/0x84) from [<c01926b8>] (bus_add_driver+0xa8/0x220)
> [<c01926b8>] (bus_add_driver+0xa8/0x220) from [<c0193c7c>] (driver_register+0xac/0x13c)
> [<c0193c7c>] (driver_register+0xac/0x13c) from [<c0033440>] (do_one_initcall+0x94/0x16c)
> [<c0033440>] (do_one_initcall+0x94/0x16c) from [<c00083f4>] (kernel_init+0x94/0x140)
> [<c00083f4>] (kernel_init+0x94/0x140) from [<c00348d0>] (kernel_thread_exit+0x0/0x8)
> ---[ end trace 0b8bf08f70147098 ]---
> 

The driver is attempting to allocate DMA memory and you have 
CONFIG_ZONE_DMA disabled, which is the only reason you would get this 
warning.  If the allocation did not fail as a result of a197b59ae6e8, the 
page allocator may return any memory in a higher zone that the driver may 
not be expecting.  If you had never noticed a problem before, it may be 
possible that the driver doesn't actually have any zone restrictions and 
GFP_DMA can be removed, but this code is pretty old.  Otherwise, it'll 
need to depend on ZONE_DMA in the Kconfig.

Let's cc Nicolas and Russell as well.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Sat, May 28, 2011 at 04:34:07PM -0700, David Rientjes wrote:
> The driver is attempting to allocate DMA memory and you have 
> CONFIG_ZONE_DMA disabled, which is the only reason you would get this 
> warning.  If the allocation did not fail as a result of a197b59ae6e8, the 
> page allocator may return any memory in a higher zone that the driver may 
> not be expecting.  If you had never noticed a problem before, it may be 
> possible that the driver doesn't actually have any zone restrictions and 
> GFP_DMA can be removed, but this code is pretty old.  Otherwise, it'll 
> need to depend on ZONE_DMA in the Kconfig.
> 
> Let's cc Nicolas and Russell as well.

Ouch.  We're probably going to have a pile of work to do to check that
the DMA masks on all our devices are correct for the unrestricted case
then.  That's probably going to be a very _big_ patch.

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Russell King - ARM Linux wrote:

> > The driver is attempting to allocate DMA memory and you have 
> > CONFIG_ZONE_DMA disabled, which is the only reason you would get this 
> > warning.  If the allocation did not fail as a result of a197b59ae6e8, the 
> > page allocator may return any memory in a higher zone that the driver may 
> > not be expecting.  If you had never noticed a problem before, it may be 
> > possible that the driver doesn't actually have any zone restrictions and 
> > GFP_DMA can be removed, but this code is pretty old.  Otherwise, it'll 
> > need to depend on ZONE_DMA in the Kconfig.
> > 
> > Let's cc Nicolas and Russell as well.
> 
> Ouch.  We're probably going to have a pile of work to do to check that
> the DMA masks on all our devices are correct for the unrestricted case
> then.  That's probably going to be a very _big_ patch.
> 

There are probably a lot of drivers that are requesting DMA but don't 
explicitly require its support.  ARM has always been one of the exceptions 
when it comes to enabling CONFIG_ZONE_DMA, most archs do by default (x86 
_just_ made it configurable during this merge window), so this probably 
isn't the last report you'll get now that it fails the allocation and 
emits a warning.

	$ grep -r GFP_DMA drivers/* | wc -l
	299



arm, pxa2xx: enable DMA support for pxa2xx IRDA interface

The pxa2xx-ir driver allocates with GFP_DMA, so it must always have 
ZONE_DMA.

Reported-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 drivers/net/irda/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/irda/Kconfig b/drivers/net/irda/Kconfig
--- a/drivers/net/irda/Kconfig
+++ b/drivers/net/irda/Kconfig
@@ -374,6 +374,7 @@ config VIA_FIR
 config PXA_FICP
 	tristate "Intel PXA2xx Internal FICP"
 	depends on ARCH_PXA && IRDA
+	select ZONE_DMA
 	help
 	  Say Y or M here if you want to build support for the PXA2xx
 	  built-in IRDA interface which can support both SIR and FIR.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Sat, May 28, 2011 at 07:22:44PM -0700, David Rientjes wrote:
> arm, pxa2xx: enable DMA support for pxa2xx IRDA interface
> 
> The pxa2xx-ir driver allocates with GFP_DMA, so it must always have 
> ZONE_DMA.

Wrong way.  If there's no restrictions, drivers shouldn't be using
GFP_DMA.  For the majority of SoCs, that's the case.

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Russell King - ARM Linux wrote:

> > arm, pxa2xx: enable DMA support for pxa2xx IRDA interface
> > 
> > The pxa2xx-ir driver allocates with GFP_DMA, so it must always have 
> > ZONE_DMA.
> 
> Wrong way.  If there's no restrictions, drivers shouldn't be using
> GFP_DMA.  For the majority of SoCs, that's the case.
> 

That's great, but before you can actually determine what requires DMA for 
this driver and what doesn't, we need something for this merge window (and 
backported to -stable) so that users aren't forced to go through and 
enable CONFIG_ZONE_DMA on their own .config.

Is there a downside to enabling CONFIG_ZONE_DMA for all configs that 
compile this driver until a better solution can be found?

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Sun, May 29, 2011 at 02:19:40PM -0700, David Rientjes wrote:
> On Sun, 29 May 2011, Russell King - ARM Linux wrote:
> 
> > > arm, pxa2xx: enable DMA support for pxa2xx IRDA interface
> > > 
> > > The pxa2xx-ir driver allocates with GFP_DMA, so it must always have 
> > > ZONE_DMA.
> > 
> > Wrong way.  If there's no restrictions, drivers shouldn't be using
> > GFP_DMA.  For the majority of SoCs, that's the case.
> > 
> 
> That's great, but before you can actually determine what requires DMA for 
> this driver and what doesn't, we need something for this merge window (and 
> backported to -stable) so that users aren't forced to go through and 
> enable CONFIG_ZONE_DMA on their own .config.
> 
> Is there a downside to enabling CONFIG_ZONE_DMA for all configs that 
> compile this driver until a better solution can be found?

We might as well enable CONFIG_ZONE_DMA for everything if that's what
you're proposing, because it's not just this driver which will be affected.
And as soon as we do that, we completely lose the warnings that stuff
needs fixing.

This is not the way to sort this problem out.

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Russell King - ARM Linux wrote:

> We might as well enable CONFIG_ZONE_DMA for everything if that's what
> you're proposing, because it's not just this driver which will be affected.

I'd certainly suggest at least defaulting it to on for arm if you're going 
to be using GFP_DMA in drivers.

> And as soon as we do that, we completely lose the warnings that stuff
> needs fixing.
> 

That's because nothing needs fixing at that point, the page allocator is 
guaranteed to return lowmem if GFP_DMA is passed and CONFIG_ZONE_DMA is 
enabled, or NULL.  Whether GFP_DMA is correct in the memory allocator is a 
different subject, but those types of audits can easily be done on the 
source code.  In my opinion, we should be doing "select ZONE_DMA" on any 
Kconfig option that builds a driver that unconditionally uses GFP_DMA.

The current behavior exists so that the admin reports the error here so it 
can get fixed up (either by finding that GFP_DMA is unnecessary, selective 
depending on the particular hardware, or modifying the Kconfig) and can 
workaround the problem by forcing CONFIG_ZONE_DMA.

IrDA driver fails on PXA255

[Dmitry Eremin-Solenikov]

Hello,

On Sun, May 29, 2011 at 3:34 AM, David Rientjes <rientjes@google.com> wrote:
> On Sun, 29 May 2011, Dmitry Eremin-Solenikov wrote:
>
>> Hello,
>>
>> Since a197b59ae6e8bee56fcef37ea2482dc08414e2ac (mm: fail GFP_DMA
>> allocations when ZONE_DMA is not configured), pxaficp_ir.c driver fails
>> to probe: pxa_irda_init_iobuf asks for a buffer with GFP_KERNEL |
>> GFP_DMA flags, which fail nicely with the following trace:
>>
>> ------------[ cut here ]------------
>> WARNING: at mm/page_alloc.c:2251
>> __alloc_pages_nodemask+0xa0/0x5ac()
>> Modules linked in:
>> [<c00385b0>] (unwind_backtrace+0x0/0xf0) from [<c0050b1c>] (warn_slowpath_common+0x4c/0x64)
>> [<c0050b1c>] (warn_slowpath_common+0x4c/0x64) from [<c0050b4c>] (warn_slowpath_null+0x18/0x1c)
>> [<c0050b4c>] (warn_slowpath_null+0x18/0x1c) from [<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac)
>> [<c00908ec>] (__alloc_pages_nodemask+0xa0/0x5ac) from [<c0090e74>] (__get_free_pages+0x10/0x3c)
>> [<c0090e74>] (__get_free_pages+0x10/0x3c) from [<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48)
>> [<c01d608c>] (pxa_irda_init_iobuf+0x18/0x48) from [<c01d61d8>] (pxa_irda_probe+0x11c/0x32c)
>> [<c01d61d8>] (pxa_irda_probe+0x11c/0x32c) from [<c019474c>] (platform_drv_probe+0x14/0x18)
>> [<c019474c>] (platform_drv_probe+0x14/0x18) from [<c0193508>] (really_probe+0xa0/0x158)
>> [<c0193508>] (really_probe+0xa0/0x158) from [<c019360c>] (driver_probe_device+0x4c/0x64)
>> [<c019360c>] (driver_probe_device+0x4c/0x64) from [<c0193684>] (__driver_attach+0x60/0x84)
>> [<c0193684>] (__driver_attach+0x60/0x84) from [<c0192d78>] (bus_for_each_dev+0x48/0x84)
>> [<c0192d78>] (bus_for_each_dev+0x48/0x84) from [<c01926b8>] (bus_add_driver+0xa8/0x220)
>> [<c01926b8>] (bus_add_driver+0xa8/0x220) from [<c0193c7c>] (driver_register+0xac/0x13c)
>> [<c0193c7c>] (driver_register+0xac/0x13c) from [<c0033440>] (do_one_initcall+0x94/0x16c)
>> [<c0033440>] (do_one_initcall+0x94/0x16c) from [<c00083f4>] (kernel_init+0x94/0x140)
>> [<c00083f4>] (kernel_init+0x94/0x140) from [<c00348d0>] (kernel_thread_exit+0x0/0x8)
>> ---[ end trace 0b8bf08f70147098 ]---
>>
>
> The driver is attempting to allocate DMA memory and you have
> CONFIG_ZONE_DMA disabled, which is the only reason you would get this
> warning. ?If the allocation did not fail as a result of a197b59ae6e8, the
> page allocator may return any memory in a higher zone that the driver may
> not be expecting. ?If you had never noticed a problem before, it may be
> possible that the driver doesn't actually have any zone restrictions and
> GFP_DMA can be removed, but this code is pretty old. ?Otherwise, it'll
> need to depend on ZONE_DMA in the Kconfig.

What about changing your patch for less intrusive one (to emit a
WARN_ON) for at least one
or two major releases and only then changing it back to the current state?

-- 
With best wishes
Dmitry

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Dmitry Eremin-Solenikov wrote:

> What about changing your patch for less intrusive one (to emit a
> WARN_ON) for at least one
> or two major releases and only then changing it back to the current state?
> 

That would return memory that is not guaranteed to be within the first 
16MB of address space, so a GFP_DMA allocation would succeed with memory 
not from ZONE_DMA.  That's an invalid configuration, so users, including 
you, should at least edit their .config by hand to enable CONFIG_ZONE_DMA 
as a workaround.  Then, we should try to fix up the Kconfig entries for 
drivers requiring DMA allocations to select CONFIG_ZONE_DMA or fix 
defconfigs when DMA is known to be needed for a device.

IrDA driver fails on PXA255

[Dmitry Eremin-Solenikov]

On Mon, May 30, 2011 at 1:17 AM, David Rientjes <rientjes@google.com> wrote:
> On Sun, 29 May 2011, Dmitry Eremin-Solenikov wrote:
>
>> What about changing your patch for less intrusive one (to emit a
>> WARN_ON) for at least one
>> or two major releases and only then changing it back to the current state?
>>
>
> That would return memory that is not guaranteed to be within the first
> 16MB of address space, so a GFP_DMA allocation would succeed with memory
> not from ZONE_DMA. ?That's an invalid configuration, so users, including
> you, should at least edit their .config by hand to enable CONFIG_ZONE_DMA
> as a workaround. ?Then, we should try to fix up the Kconfig entries for
> drivers requiring DMA allocations to select CONFIG_ZONE_DMA or fix
> defconfigs when DMA is known to be needed for a device.

Am I right that this was the previous behaviour for GFP_DMA allocations
w/o CONFIG_ZONE_DMA? If so, we can have it (probably) for one more
major release to get all warnings.

-- 
With best wishes
Dmitry

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Sun, May 29, 2011 at 02:17:02PM -0700, David Rientjes wrote:
> On Sun, 29 May 2011, Dmitry Eremin-Solenikov wrote:
> 
> > What about changing your patch for less intrusive one (to emit a
> > WARN_ON) for at least one
> > or two major releases and only then changing it back to the current state?
> > 
> 
> That would return memory that is not guaranteed to be within the first 
> 16MB of address space, so a GFP_DMA allocation would succeed with memory 
> not from ZONE_DMA.

Err, no. GFP_DMA returns memory in a zone which the platform has setup.
There's nothing specific about it being "16MB" or any other size; the
arch can chose what size that is.

> That's an invalid configuration, so users, including 
> you, should at least edit their .config by hand to enable CONFIG_ZONE_DMA 
> as a workaround.

Again, no.  This change has caused a load of previously working drivers
to suddenly start failing without _any_ explanation why or even warning
about the change.  It needs to start off as a WARN_ON() so that stuff
can be fixed, and then changed to a hard error.

IrDA driver fails on PXA255

[David Rientjes]

On Sun, 29 May 2011, Russell King - ARM Linux wrote:

> > That would return memory that is not guaranteed to be within the first 
> > 16MB of address space, so a GFP_DMA allocation would succeed with memory 
> > not from ZONE_DMA.
> 
> Err, no. GFP_DMA returns memory in a zone which the platform has setup.
> There's nothing specific about it being "16MB" or any other size; the
> arch can chose what size that is.
> 

Sorry, was talking from the x86 perspective, which probably doesn't make 
much sense since we're talking about an arm driver :)

> > That's an invalid configuration, so users, including 
> > you, should at least edit their .config by hand to enable CONFIG_ZONE_DMA 
> > as a workaround.
> 
> Again, no.  This change has caused a load of previously working drivers
> to suddenly start failing without _any_ explanation why or even warning
> about the change.  It needs to start off as a WARN_ON() so that stuff
> can be fixed, and then changed to a hard error.
> 

I haven't seen a "load" of error reports where this is causing an issue, 
maybe it is much more popular on arm?

This also isn't a hard error, admins should be able to enable 
CONFIG_ZONE_DMA and rebuild so that the driver being loaded can get the 
type of memory it is requesting.  Just putting a WARN_ON() doesn't provide 
any incentive to ever get this stuff fixed.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Mon, May 30, 2011 at 10:05:38PM -0700, David Rientjes wrote:
> On Sun, 29 May 2011, Russell King - ARM Linux wrote:
> > Again, no.  This change has caused a load of previously working drivers
> > to suddenly start failing without _any_ explanation why or even warning
> > about the change.  It needs to start off as a WARN_ON() so that stuff
> > can be fixed, and then changed to a hard error.
> > 
> 
> I haven't seen a "load" of error reports where this is causing an issue, 
> maybe it is much more popular on arm?
> 
> This also isn't a hard error, admins should be able to enable 
> CONFIG_ZONE_DMA and rebuild so that the driver being loaded can get the 
> type of memory it is requesting.  Just putting a WARN_ON() doesn't provide 
> any incentive to ever get this stuff fixed.

I completely and violently disagree with your approach on this, and I'll
continue to state that I believe you are wrong until you change your
position.

Enabling CONFIG_ZONE_DMA is not a fix, it's making the problem vanish off
the radar.  It will mean that the drivers using GFP_DMA will never get
fixed up.

I don't care whether you say "it's easy enough to audit the source code" -
who's going to do that work?  If you make the problem go away the answer
to that will be NO ONE.

Change it to be a soft WARN_ON for one release.  Anything else will just
result in the problem being IGNORED.

IrDA driver fails on PXA255

[David Rientjes]

On Tue, 31 May 2011, Russell King - ARM Linux wrote:

> Enabling CONFIG_ZONE_DMA is not a fix, it's making the problem vanish off
> the radar.  It will mean that the drivers using GFP_DMA will never get
> fixed up.
> 

Disabling CONFIG_ZONE_DMA is an optimization, do you agree?  I asked 
on Sunday what the downsides of enabling the option on arm are, and you 
didn't mention any.  So what's the problem with my patch to enable it for 
this driver since it is using GFP_DMA?  You claim that it'll never get 
removed again to return to the _optimized_ case, yet my patch is 
guaranteed to work for that driver in all configurations at the moment.  I 
don't think we should be fighting for the optimized case where the 
alternative has no downside.

 [ Patching the page allocator to also emit a line to the dmesg to direct 
   users directly to enabling CONFIG_ZONE_DMA wouldn't be a problem, 
   either. ]

> Change it to be a soft WARN_ON for one release.  Anything else will just
> result in the problem being IGNORED.
> 

The problem certainly isn't being ignored in this thread, or in the patch 
that I sent to fix Dmitry's issue by default, so that doesn't seem to be 
the case.  What would be ignored, though, is if it just emitted a 
WARN_ON() without failing the allocation so everything works perfectly.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Tue, May 31, 2011 at 01:03:03PM -0700, David Rientjes wrote:
> The problem certainly isn't being ignored in this thread, or in the patch 
> that I sent to fix Dmitry's issue by default, so that doesn't seem to be 
> the case.  What would be ignored, though, is if it just emitted a 
> WARN_ON() without failing the allocation so everything works perfectly.

Sorry, you did not send a patch to fix it.  You sent a *bodge* to enable
the DMA zone.  As long as you insist that's a valid fix, you're going
to carry zero credibility with me.

The fact is that this driver should not be using GFP_DMA to allocate
things which aren't even DMA buffers, and its use of GFP_DMA should be
removed.  But rather than look at that and work it out, and then produce
a patch to sort that out, the only thing you can do is come up with
bodge to enable the DMA zone, and continue to insist that's the right
solution.

I repeat, enabling the DMA zone for this driver is a pure and utter
bodge, and the change to make these allocations fail _will_ and _has_
caused regressions.

Your whinge that we should re-enable the DMA zone which has been
disabled for quite a long time now to work around this new restriction
is extremely idiotic.

Do the right thing.  Make allocations to GFP_DMA zones _warn_ first for
a cycle so that affected drivers can be fixed.  Then for the next cycle,
make it a hard failure.

IrDA driver fails on PXA255

[David Rientjes]

On Tue, 31 May 2011, Russell King - ARM Linux wrote:

> > The problem certainly isn't being ignored in this thread, or in the patch 
> > that I sent to fix Dmitry's issue by default, so that doesn't seem to be 
> > the case.  What would be ignored, though, is if it just emitted a 
> > WARN_ON() without failing the allocation so everything works perfectly.
> 
> Sorry, you did not send a patch to fix it.  You sent a *bodge* to enable
> the DMA zone.  As long as you insist that's a valid fix, you're going
> to carry zero credibility with me.
> 
> The fact is that this driver should not be using GFP_DMA to allocate
> things which aren't even DMA buffers, and its use of GFP_DMA should be
> removed.  But rather than look at that and work it out, and then produce
> a patch to sort that out, the only thing you can do is come up with
> bodge to enable the DMA zone, and continue to insist that's the right
> solution.
> 

I'm not going to hack on an arm driver when I have no idea what its DMA 
requirements are and have no ability to test the change.  I'll rely on the 
authors or maintainers of that driver to figure it out.  In the meantime, 
I suggested enabling CONFIG_ZONE_DMA for that driver because, in its 
current state, it is using GFP_DMA and you haven't provided a single 
reason why enabling CONFIG_ZONE_DMA is going to cause an issue.

In other words, I cannot do your work for you: if GFP_DMA can be removed 
from that driver, great, in the meantime it should enable CONFIG_ZONE_DMA 
to fix the invalid configurations that are currently allowed.  It's 
simple: if you're going to pass GFP_DMA to the page allocator, you need to 
require CONFIG_ZONE_DMA for it to be useful.  Anything else is an invalid 
configuration and is error prone.

> Your whinge that we should re-enable the DMA zone which has been
> disabled for quite a long time now to work around this new restriction
> is extremely idiotic.
> 

The restriction isn't new: GFP_DMA only makes sense with CONFIG_ZONE_DMA.  
The fact that the page allocator completely ignored GFP_DMA in the past 
for CONFIG_ZONE_DMA=n doesn't change that.  That's obviously error prone 
since it will return memory from anywhere simply because of the fact that 
it is an invalid configuration.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Tue, May 31, 2011 at 03:11:40PM -0700, David Rientjes wrote:
> The restriction isn't new: GFP_DMA only makes sense with CONFIG_ZONE_DMA.  
> The fact that the page allocator completely ignored GFP_DMA in the past 
> for CONFIG_ZONE_DMA=n doesn't change that.  That's obviously error prone 
> since it will return memory from anywhere simply because of the fact that 
> it is an invalid configuration.

Your approach to this is wrong.  Make it warn for one release.  Give
people a chance to fix things before they become a regression.  Then
make it a hard failure.

IrDA driver fails on PXA255

[Russell King - ARM Linux]

On Wed, Jun 01, 2011 at 08:54:20AM +0100, Russell King - ARM Linux wrote:
> On Tue, May 31, 2011 at 03:11:40PM -0700, David Rientjes wrote:
> > The restriction isn't new: GFP_DMA only makes sense with CONFIG_ZONE_DMA.  
> > The fact that the page allocator completely ignored GFP_DMA in the past 
> > for CONFIG_ZONE_DMA=n doesn't change that.  That's obviously error prone 
> > since it will return memory from anywhere simply because of the fact that 
> > it is an invalid configuration.
> 
> Your approach to this is wrong.  Make it warn for one release.  Give
> people a chance to fix things before they become a regression.  Then
> make it a hard failure.

And to prove that its not just this driver, I've now received a report
that it fails with the CF PATA driver on Zaurus.  Should we make the
PATA subsystem select CONFIG_ZONE_DMA too, or should we give people
some grace period to fix the drivers as _everyone_ except you is
suggesting.

Please do the sensible thing.  Make it warn for a release like everyone
is telling you to.