2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Jeff Chua]

No response so far ... I'm sure someone know this stuff ...  Thanks, Jeff.


I'm trying to understand how this oops in the diva driver and it's just a
simple dma_alloc_coherent() followed by dma_free_coherent(), but it oops.
Why?

Thanks,
Jeff


static u32 *clock_data_addr;
static dma_addr_t clock_data_bus_addr;

        if((clock_data_addr = dma_alloc_coherent(NULL, PAGE_SIZE,
                        &clock_data_bus_addr, GFP_KERNEL)) != 0) {
                printk(KERN_INFO "dma_alloc_coherent ok\n");
                memset (clock_data_addr, 0x00, PAGE_SIZE);
        } else
                printk(KERN_INFO "dma_alloc_coherent bad!!!\n");
        if(clock_data_addr) {
                printk(KERN_INFO "dma_free_coherent!!!\n");
                dma_free_coherent(NULL, PAGE_SIZE, clock_data_addr,
                        clock_data_bus_addr);
                clock_data_addr = NULL;
        }


dma_alloc_coherent ok
dma_free_coherent!!!
BUG: unable to handle kernel NULL pointer dereference at 0000000000000080
IP: [<ffffffff813ec01c>] iommu_no_mapping+0xc/0xee
PGD 20e4d8067 PUD 20bda6067 PMD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: divadidd(+) i915 drm_kms_helper iwldvm iwlwifi mac80211
cfg80211
CPU 0
Pid: 2823, comm: modprobe Not tainted 3.8.0-rc2 #9 LENOVO 4291JB0/4291JB0
RIP: 0010:[<ffffffff813ec01c>]  [<ffffffff813ec01c>]
iommu_no_mapping+0xc/0xee
RSP: 0018:ffff880211665d68  EFLAGS: 00010246
RAX: 00000000d3801000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000001000 RSI: 00000000d3801000 RDI: 0000000000000000
RBP: 0000000000000001 R08: 0000000000000000 R09: 00000000000045c7
R10: 00000000000045c7 R11: 00000000000045c7 R12: ffffffff81a4d800
R13: ffffffffa013a550 R14: ffff88020be0fa80 R15: 0000000000000001
FS:  00007f9df9786700(0000) GS:ffff88021e200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000080 CR3: 00000002114c1000 CR4: 00000000000407f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400

Re: 2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Jonathan Corbet]

On Tue, 8 Jan 2013 23:51:59 +0800
Jeff Chua <jeff.chua.linux@gmail.com> wrote:

> I'm trying to understand how this oops in the diva driver and it's just a
> simple dma_alloc_coherent() followed by dma_free_coherent(), but it oops.
> Why?

Hmm...from a quick look...

> static u32 *clock_data_addr;
> static dma_addr_t clock_data_bus_addr;
> 
>         if((clock_data_addr = dma_alloc_coherent(NULL, PAGE_SIZE,
>                         &clock_data_bus_addr, GFP_KERNEL)) != 0) {
>                 printk(KERN_INFO "dma_alloc_coherent ok\n");
>                 memset (clock_data_addr, 0x00, PAGE_SIZE);
>         } else
>                 printk(KERN_INFO "dma_alloc_coherent bad!!!\n");
>         if(clock_data_addr) {
>                 printk(KERN_INFO "dma_free_coherent!!!\n");
>                 dma_free_coherent(NULL, PAGE_SIZE, clock_data_addr,
>                         clock_data_bus_addr);
>                 clock_data_addr = NULL;
>         }

Perhaps passing NULL as your device structure pointer might just have
something to do with a crash due to a null pointer dereference?  If you're
doing DMA, you should have a device.  Try passing it and you might just
find that things work better.

(Incidentally, this:

> IP: [<ffffffff813ec01c>] iommu_no_mapping+0xc/0xee

is a fairly good clue as well - it's trying to dereference the device
structure pointer).

jon

Re: 2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Linus Torvalds]

On Tue, Jan 8, 2013 at 7:51 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
> No response so far ... I'm sure someone know this stuff ...  Thanks, Jeff.
>
> I'm trying to understand how this oops in the diva driver and it's just a
> simple dma_alloc_coherent() followed by dma_free_coherent(), but it oops.
> Why?

Umm. You're using a NULL 'dev' pointer?

iommu_no_mapping() will look if the device is a PCI device, and that's
where it oopses.

Now, it looks like some users "know" that the device doesn't matter
(which is true on some platforms), but that's bogus. The device is how
you even find out what the rules for DMA access are (see for example
dma_set_mask() etc), so giving a NULL device is insane.

              Linus

Re: 2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Jeff Chua]

On Wed, Jan 9, 2013 at 12:39 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jan 8, 2013 at 7:51 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
>> No response so far ... I'm sure someone know this stuff ...  Thanks, Jeff.
>>
>> I'm trying to understand how this oops in the diva driver and it's just a
>> simple dma_alloc_coherent() followed by dma_free_coherent(), but it oops.
>> Why?
>
> Umm. You're using a NULL 'dev' pointer?
>
> iommu_no_mapping() will look if the device is a PCI device, and that's
> where it oopses.
>
> Now, it looks like some users "know" that the device doesn't matter
> (which is true on some platforms), but that's bogus. The device is how
> you even find out what the rules for DMA access are (see for example
> dma_set_mask() etc), so giving a NULL device is insane.
>
>               Linus


Interesting, but there are 54 lines under the kernel directories that
use "dma_alloc_coherent(NULL," followed by "dma_free_coherent(NULL,"

So, shouldn't they be fixed as well? ... unless they are so old that
nobody cares anymore ...


# find . -exec grep -H "dma_alloc_coherent(NULL" {} \; | wc -l
54

#find . -exec grep -H "dma_free_coherent(NULL" {} \; | wc -l
72



Thanks,
Jeff

Re: 2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Linus Torvalds]

On Tue, Jan 8, 2013 at 9:17 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
>
> Interesting, but there are 54 lines under the kernel directories that
> use "dma_alloc_coherent(NULL," followed by "dma_free_coherent(NULL,"

As mentioned, it works on some platforms. That doesn't make it right.

> So, shouldn't they be fixed as well? ... unless they are so old that
> nobody cares anymore ...

Some of the ones I saw are in MIPS or blackfin. Others probably *are*
so old that nobody cares, and happen to work because there's iommu's
or other things that simply don't care about the device.

> # find . -exec grep -H "dma_alloc_coherent(NULL" {} \; | wc -l
> 54
>
> #find . -exec grep -H "dma_free_coherent(NULL" {} \; | wc -l
> 72

Let me tell you about "git grep", so that you never need to do that
disgusting "find -exec grep" ever again.

It does threading, it's fast, and it just works.

                Linus

Re: 2nd attempt: help with dma_alloc_coherent() + dma_free_coherent()

[Jeff Chua]

On Wed, Jan 9, 2013 at 1:24 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jan 8, 2013 at 9:17 AM, Jeff Chua <jeff.chua.linux@gmail.com> wrote:
>>
>> Interesting, but there are 54 lines under the kernel directories that
>> use "dma_alloc_coherent(NULL," followed by "dma_free_coherent(NULL,"
>
> As mentioned, it works on some platforms. That doesn't make it right.
>
>> So, shouldn't they be fixed as well? ... unless they are so old that
>> nobody cares anymore ...
>
> Some of the ones I saw are in MIPS or blackfin. Others probably *are*
> so old that nobody cares, and happen to work because there's iommu's
> or other things that simply don't care about the device.
>
>> # find . -exec grep -H "dma_alloc_coherent(NULL" {} \; | wc -l
>> 54
>>
>> #find . -exec grep -H "dma_free_coherent(NULL" {} \; | wc -l
>> 72
>
> Let me tell you about "git grep", so that you never need to do that
> disgusting "find -exec grep" ever again.
>
> It does threading, it's fast, and it just works.
>
>                 Linus

Yes, "git grep" is really fast. Cool! ... didn't know that before.

Thanks again for the help.

Jeff.