Very Computer

hummmm magic casts?? why ?

Quote:> +          ptr = snd_malloc_pci_pages(sgbuf->pci, PAGE_SIZE, &addr);

what is wrong with the PCI DMA API that makes ALSA wants a private
interface/implementation ?

Quote:>  EXPORT_SYMBOL(snd_wrapper_kmalloc);
>  EXPORT_SYMBOL(snd_wrapper_kfree);
> +EXPORT_SYMBOL(snd_wrapper_vmalloc);
> +EXPORT_SYMBOL(snd_wrapper_vfree);

why do you need a wrapper for vfree?

signature.asc < 1K Download

We are trying to check if 'void *' pointers in structures are used
correctly. It's our tool for debugging.

Quote:> > +             ptr = snd_malloc_pci_pages(sgbuf->pci, PAGE_SIZE, &addr);

> what is wrong with the PCI DMA API that makes ALSA wants a private
> interface/implementation ?

These lines (i386 arch):

        if (hwdev == NULL || ((u32)hwdev->dma_mask != 0xffffffff))
                gfp |= GFP_DMA;
        ret = (void *)__get_free_pages(gfp, get_order(size));

Note that some of soundcards have various PCI DMA transfer limits
(dma_mask is not set to use full 32-bits). In this case, restricting this
hardware to allocate these buffers in first 16MB is not a very good idea.
Thus, we have own hacks to allocate memory in whole hardware area.

Quote:> >  EXPORT_SYMBOL(snd_wrapper_kmalloc);
> >  EXPORT_SYMBOL(snd_wrapper_kfree);
> > +EXPORT_SYMBOL(snd_wrapper_vmalloc);
> > +EXPORT_SYMBOL(snd_wrapper_vfree);

> why do you need a wrapper for vfree?

Debugging. We enumerate all allocations, so we can check for memory leaks.
I'm happy to say, that our code is very well debugged in this regard.

                                                Jaroslav

-----

Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

It sounds like it would be reasonable to fix the ia32 arch code, would
it not?

Quote:> Debugging. We enumerate all allocations, so we can check for memory leaks.
> I'm happy to say, that our code is very well debugged in this regard.

So are the net drivers, but without wrappers :)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

We have simple method in our hack:

  - try to allocate memory anywhere
  - try to allocate memory with GFP DMA if first step fails

Here is updated pci-dma.c code for comments:

===== pci-dma.c 1.8 vs edited =====
--- 1.8/arch/i386/kernel/pci-dma.c      Thu May  9 19:24:12 2002

        void *ret;
        int gfp = GFP_ATOMIC;

-       if (hwdev == NULL || ((u32)hwdev->dma_mask != 0xffffffff))
+       if (hwdev == NULL || (u32)hwdev->dma_mask <= 0x00ffffff)
                gfp |= GFP_DMA;
        ret = (void *)__get_free_pages(gfp, get_order(size));

-       if (ret != NULL) {
-               memset(ret, 0, size);
-               *dma_handle = virt_to_phys(ret);
+       if (ret == NULL)
+               return NULL;
+
+       /* try to allocate area in first 16MB memory */
+       if ((gfp & GFP_DMA) == 0 && (u32)hwdev->dma_mask != 0xffffffff &&
+           ((virt_to_phys(ret) + size - 1) & ~(u32)hwdev->dma_mask) != 0) {
+               free_pages(ret, get_order(size));
+               ret = (void *)__get_free_pages(gfp | GFP_DMA, get_order(size));
+               if (ret == NULL)
+                       return NULL;
        }
+
+       memset(ret, 0, size);
+       *dma_handle = virt_to_phys(ret);
        return ret;
 }

                                                Jaroslav

-----

Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

This is wrong. You just broke ISA DMA handling, and also blocking of
devices with sub ISA requirements

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

   Date: 29 Sep 2002 21:12:23 +0200

   what is wrong with the PCI DMA API that makes ALSA wants a private
   interface/implementation ?

It makes the layers not have to know what the BUS is, and this can all
be deleted when everything goes through a generic struct device and
assosciated OPS.

I think ALSA is definitely doing the right thing here.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

   Date: Sun, 29 Sep 2002 22:34:51 +0200 (CEST)

   -    if (hwdev == NULL || ((u32)hwdev->dma_mask != 0xffffffff))
   +    if (hwdev == NULL || (u32)hwdev->dma_mask <= 0x00ffffff)
                gfp |= GFP_DMA;

So alan, why is this really broken?

EISA/ISA DMA is defined as using a hwdev of NULL or requiring
<16MB address, he is preserving GFP_DMA in those cases.

The only problem I have with the patch is that it probably isn't
that hard to let the page allocator take some MASK arg (defaults
to all 1's) to implement this in 2.5.x
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Firstly the DMA mask on x86 can't be below 24bits, we don't support
allocation from a smaller zone. Secondly what about PCI for 25-31bits -
there we do need to force gfp_dma to have any chance of getting the
right pages

Giving the page allocator a mask argument does sound a lot nicer

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Not really. The test in original code - if the page is in right area - was
bellow first allocation. Sure, it's dirty hack.

Quote:> Giving the page allocator a mask argument does sound a lot nicer

Right, my aim was to point to a bug. Let's go to create proper DMA
allocation for broken hardware. I still wonder why hardware vendors are
lazy to support full bus addressing. Perhaps, saving few coins per
chip make them happy.

                                                Jaroslav

-----

Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

   Date: 30 Sep 2002 13:54:11 +0100

   > EISA/ISA DMA is defined as using a hwdev of NULL or requiring
   > <16MB address, he is preserving GFP_DMA in those cases.

   Firstly the DMA mask on x86 can't be below 24bits, we don't support
   allocation from a smaller zone.

Understood.

   Secondly what about PCI for 25-31bits -
   there we do need to force gfp_dma to have any chance of getting the
   right pages

Look at what his code does after the GFP_DMA setting, it goes
a non-GFP_DMA setting, and if the 25-31 bits case is not satisfied
it backs down to GFP_DMA.

   Giving the page allocator a mask argument does sound a lot nicer

It's pretty simple to implement too since we do have page_to_phys.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/