On Mon, Feb 18, 2008 at 04:57:36AM -0800, Andrew Morton wrote:
> On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
>
> > ...
> >
> > All my testers have reported back that with these 5 patches applied they can
> > now run with a 2.6.24 kernel the same way they ran before. However there is
> > that reported issue, with the dma_free_coherent WARN_ON (above). The code was
> > like that from day one and it is a very old issue, however it is a regression
> > because 2.6.24 introduced that new WARN_ON.
> > (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> > >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
> > it looks that all a driver can do is work around it with different kernel mechanisms
> > and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
> > why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
> > is needed to replace it. Could you please have a look in gdth_proc.c and also in
> > gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
> > what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
> > a bugfix.
> >
> > Apart from the above issue, please accept patches 3,4,5 above they have now
> > been tested and are reported to bring broken system back to production.
> > (Given that you approve off course). And mark them for inclusion to the
> > 2.6.24 stable releases. (Or is there some thing that I should do)
> >
> > ---
> > Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> > pose any harm. Some people have reported stability with temporarily disabling
> > it. For testers that want to try, here it is below. At your own risk.
> >
> > ---
> > >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
> > From: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> > Date: Sun, 17 Feb 2008 12:49:35 +0200
> > Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
> >
> > gdth uses dma_free_coherent() with interrupts disabled. Which
> > is not portable, but is safe on the HW that supports gdth.
> >
> > NOT Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> > ---
> > arch/x86/kernel/pci-dma_32.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
> > index 5133032..350dcfd 100644
> > --- a/arch/x86/kernel/pci-dma_32.c
> > +++ b/arch/x86/kernel/pci-dma_32.c
> > @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
> > struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
> > int order = get_order(size);
> >
> > - WARN_ON(irqs_disabled()); /* for portability */
> > +/* WARN_ON(irqs_disabled());*/ /* for portability */
> > if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
> > int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
> >
>
> Yes. Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:
>
> : commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> : Author: David Brownell <david-b@xxxxxxxxxxx>
> : Date: Fri Aug 10 13:10:27 2007 -0700
> :
> : dma_free_coherent() needs irqs enabled (sigh)
> :
> : On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
> : call context requirement: unlike its dma_alloc_coherent() sibling, it may
> : not be called with IRQs disabled. (This was new behavior on ARM as of late
> : 2005, caused by ARM SMP updates.) This little surprise can be annoyingly
> : driver-visible.
> :
> : Since it looks like that restriction won't be removed, this patch changes
> : the definition of the API to include that requirement. Also, to help catch
> : nonportable drivers, it updates the x86 and swiotlb versions to include the
> : relevant warnings. (I already observed that it trips on the
> : bus_reset_tasklet of the new firewire_ohci driver.)
> :
>
> In general, all Linux memory-freeing functions can be called from all
> contexts. (vfree is an irritating exception). This is good, and provides
> maximum usefulness to callees, as all utility functions should seek to do.
> It would be best to fix arm and mips.
>
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call. Presumably because
> of certain unusual TLB protocols.
>
> I'm not sure what we should do about this. Presumably the gdth-on-arm
> usage base is, umm, zero, so we could lamely add
> CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
> to disable gdth (and similar) on arm amd mips. But ugh.
>
> Russell, Ralf: is there something we can do here to relax this requirement?
The current MIPS implementation of dma_alloc_coherent / dma_free_coherent
is a glorified wrapper around __get_free_pages rsp. free_pages that is
it doesn't depend on interrupts enabled.
This works because the current dma_alloc_coherent will only return
lowmem which is accessible without TLB mappings through KSEG0/1.
The embedded world being as quirky as it is I expect support of highmem
which will require the use of TLB entries to become eventually relevant
on MIPS but I think the lazy teardown of the mappings can take care of
that and worst case a caller of dma_alloc_coherent() has to be prepared
to handle an error return anyway.
Ralf
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
