Message-id: <alpine.LFD.2.00.1010091125190.20826@xanadu.home>
Date:	Sat, 09 Oct 2010 13:38:33 -0400 (EDT)
From:	Nicolas Pitre <nico@...xnic.net>
To:	Felipe Contreras <felipe.contreras@...il.com>
Cc:	Greg KH <greg@...ah.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	linux-main <linux-kernel@...r.kernel.org>,
	linux-arm <linux-arm-kernel@...ts.infradead.org>,
	Arnd Hannemann <arnd@...dnet.de>,
	Han Jonghun <jonghun79.han@...il.com>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>, Hemant Pedanekar <hemantp@...com>
Subject: Re: [PATCH] ARM: allow, but warn, when issuing ioremap() on RAM

On Sat, 9 Oct 2010, Felipe Contreras wrote:

> On Sat, Oct 9, 2010 at 6:36 AM, Nicolas Pitre <nico@...xnic.net> wrote:
> > On Fri, 8 Oct 2010, Greg KH wrote:
> >> Wait, let me get this straight:
> >>   - drivers used to work on 2.6.35
> >
> > Possibly on pre ARMv6 only.  On ARMv6 and above, the discussed behavior
> > _never_ worked.  It is fundamental to the CPU design.
> 
> That's not true, drivers on ARMv6 and above do work.

So you want to fool yourself?  Fine.  This is your problem.

> I still wonder how exactly to trigger the issue, and how often does 
> this happen, because I've never seen it.

It's not because you don't see it right away that this is not happening.

> I trust the experts that it is an issue, but I don't agree that
> drivers that are currently working on ARMv6+ should be broken on
> purpose right away.

They are not working *reliably*.  Get that notion out of your mind.

> >>   - some ARM core code changed in .36-rc to fix this iomem problem that
> >>     you found
> >
> > Nothing was fixed.  There is nothing to fix.  Simply that a wrong driver
> > assumption about the hardware capability was prohibited by the API.
> 
> Not exactly. There is supposedly a problem on ARMv6+, there must be a
> mechanism to avoid this problem, all drivers should migrate to that,
> or a different on, so that we have a sane API.

Exact.  So please put the necessary effort to 1) understand the issue 
which was clearly explained in details by RMK already, or simply read 
the ARM Architecture Reference Manual for yourself, so that you 
understand the issue and stop asking for unreasonable things, and 2) do 
invest some time to fix your code that never worked properly in the 
first place.

> > There is no API change.  Simply an API misuse.  Granted, on pre ARMv6
> > this usage used to work, but on ARMv6 and above this never was supported
> > at all by the hardware.  The fact that the software has let it through
> > was a gross mistake, period.
> 
> No. It worked just fine, now it's doesn't, that's a change in API
> behavior, which is also part of the API. It might not be ideal, or
> proper, but it does work.

Look, if you can't be brought to reason and common sense this will be my 
last reply to you.

What would you say if someone suggested to turn the spinlocks into 
no-ops in a SMP kernel with full preemption configured in?  Let's say 
that person has done that because the lock validator was complaining and 
removing the locks made the warnings as well as the deadlock go away?  
And that person is claiming that the driver now works just fine (or 
appears to work) and no issue what so ever has been observed?  Because 
that's exactly what you're asking for right now.  Except that in this 
case you are willing to ignore a hardware race instead of a software 
one.

> >>   - drivers break when run as the api stops returning valid addresses
> >
> > Such drivers on ARMv6 should *never* have worked in the first place.
> > When they did "work", they corrupted memory.
> 
> They corrupted their own memory, right? And on _very_ rare occasions I
> presume, since I've never seen it.

Not their own memory, but *ANY* random memory.  You could corrupt your 
filesystem, have bugs in totally unrelated drivers, and so in ways that 
are totally unpredictable and unreproducible.  Is that the kind of 
system you want to debug?  If you have customers, is that the kind of 
products you want to sell them?

> If so, there's no damage in letting
> them work at least one more release (2.6.36).

On what basis can you dare make such a foolish claim?  Only on your 
limited testing and own observations?  Do you have some insider 
knowledge of the ARM architecture that would give us confidence in 
ignoring written documentation about this very issue?  What about those 
people besides you who did experience problems before?

If you want that, then do it in your own tree, oh and let me remember to 
ignore any future request for help from you.  But the mainline tree has 
to be proper regardless of the inconvenience that may cause you, 
_especially_ when data integrity is at stake.

> >>   - no known way is around to fix the broken drivers
> >
> > That's B/s.  As Russell said, there are known solutions that were
> > proposed at least six months ago.
> 
> You seriously think most people out there know how to "set aside some
> RAM at boot time"?

Indeed I do.  That is not rocket science.  You can easily figure it 
yourself too.

> I don't, otherwise the drivers would be fixed by now.

And that's why you want me to accept your argument for restoring a 
broken behavior?

Go do your homework and fix your code.


Nicolas

Powered by blists - more mailing lists