dma_free_coherent in arcmsr when calling areca tools

Bug 9880 [email protected] 4 participants 13 messages Feb 4, 2008 – Feb 9, 2008 1 message not in the archive SCSI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]/

[email protected]

author

2008-02-04 03:00 UTC #1 lore raw

http://bugzilla.kernel.org/show_bug.cgi?id=9880

[email protected] changed:

What |Removed |Added
----------------------------------------------------------------------------

         AssignedTo|[email protected]              |scsi_drivers-other@kernel-
                   |                            |bugs.osdl.org
          Component|PCI                         |Other
            Product|Drivers                     |SCSI Drivers

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

James Bottomley

maintainer

2008-02-04 03:55 UTC #2 lore raw

Quoted 44 lines · On Sun, 2008-02-03 at 19:00 -0800, [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=9880

Latest working kernel version: 2.6.23.12
Earliest failing kernel version: 2.6.24
Distribution: Gentoo
Hardware Environment:
Intel Core2Quad, 2GB Memory, Areca 1220ML

Software Environment:
Areca CLI, Version: 1.72.250, Date: Apr 11 2007( Linux ) Areca HTTP proxy server V1.81.250 for Areca RAID controllers.

Problem Description:

Feb  2 02:04:01 mrb WARNING: at arch/x86/kernel/pci-dma_64.c:169
dma_free_coherent()
Feb  2 02:04:01 mrb Pid: 16080, comm: archttp64 Not tainted
2.6.24 #1
Feb  2 02:04:01 mrb
Feb  2 02:04:01 mrb Call Trace:
Feb  2 02:04:01 mrb [<ffffffff80213568>] dma_free_coherent+0x43/0x82
Feb  2 02:04:01 mrb [<ffffffff88178b85>] :arcmsr:arcmsr_queue_command
+0x379/0x973
Feb  2 02:04:01 mrb [<ffffffff8053d8c2>] _spin_unlock_irqrestore
+0x16/0x31
Feb  2 02:04:01 mrb [<ffffffff8043cce3>] scsi_dispatch_cmd+0x194/0x1e9
Feb  2 02:04:01 mrb [<ffffffff80441bab>] scsi_request_fn+0x28b/0x360
Feb  2 02:04:01 mrb [<ffffffff80387726>] blk_execute_rq_nowait+0x7a/0x8b
Feb  2 02:04:01 mrb [<ffffffff80441698>] scsi_execute_async+0x33d/0x36e
Feb  2 02:04:01 mrb [<ffffffff88159379>] :sg:sg_common_write+0x66f/0x6a3
Feb  2 02:04:01 mrb [<ffffffff881595c0>] :sg:sg_cmd_done+0x0/0x1fe
Feb  2 02:04:01 mrb [<ffffffff8053d97f>] _write_unlock_irqrestore
+0x17/0x34
Feb  2 02:04:01 mrb [<ffffffff8815959d>] :sg:sg_new_write+0x1f0/0x213
Feb  2 02:04:01 mrb [<ffffffff8815ab47>] :sg:sg_ioctl+0x1f3/0x9f5
Feb  2 02:04:01 mrb [<ffffffff8024d1c0>] hrtimer_try_to_cancel+0x5f/0x68
Feb  2 02:04:01 mrb [<ffffffff8024d1d5>] hrtimer_cancel+0xc/0x16
Feb  2 02:04:01 mrb [<ffffffff8053c62a>] do_nanosleep+0x55/0x7e
Feb  2 02:04:01 mrb [<ffffffff8029ae2d>] do_ioctl+0x55/0x6b
Feb  2 02:04:01 mrb [<ffffffff8029b076>] vfs_ioctl+0x233/0x24c
Feb  2 02:04:01 mrb [<ffffffff8029b0e0>] sys_ioctl+0x51/0x71
Feb  2 02:04:01 mrb [<ffffffff8020bfae>] system_call+0x7e/0x83

Quoted 7 lines

Steps to reproduce:
If the httpd deamon runs the message occurs endless. The areca cli command produces the message for each call.
The problem occurs after upgrading to kernel 2.6.24 kernel 2.6.23.12 and before works well.

This is here, isn't it:

void dma_free_coherent(struct device *dev, size_t size,
void *vaddr, dma_addr_t bus)

{
	WARN_ON(irqs_disabled());	/* for portability */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	if (dma_ops->unmap_single)
		dma_ops->unmap_single(dev, bus, size, 0);
	free_pages((unsigned long)vaddr, get_order(size));
}

That's caused by this commit:

commit aa24886e379d2b641c5117e178b15ce1d5d366ba

Author: David Brownell <[email protected]>
Date:   Fri Aug 10 13:10:27 2007 -0700

dma_free_coherent() needs irqs enabled (sigh)

I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

James

David Brownell

2008-02-04 04:36 UTC #3 lore raw

Quoted 6 lines · James Bottomley

That's caused by this commit:
commit aa24886e379d2b641c5117e178b15ce1d5d366ba
Author: David Brownell <[email protected]>
Date: Fri Aug 10 13:10:27 2007 -0700

Quoted 7 lines

dma_free_coherent() needs irqs enabled (sigh)

I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?

That was my reaction ... but I was told this would not be fixed.

See also 8a3c1f573c771e60f67ef172d2392d1a28385b4a ... several other controller drivers needed similar logic at one point. (I eventually ripped that programming interface out, or *EVERY* driver would have needed crap like that. The interface was mostly misused anyway, so it was no big loss.)

Quoted 2 lines

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

Lacking a tasklet in dma_free_coherent() analagous to that omap_udc commit I referenced ...

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Russell King

2008-02-04 08:20 UTC #4 lore raw

In reply to James Bottomley

Quoted 5 lines · James Bottomley

I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?

For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.

Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:

x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/

arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.

Which is more idiotic?

Quoted 2 lines

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

Yes.

Signature · 3 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

James Bottomley

maintainer

2008-02-09 16:51 UTC #5 lore raw

Quoted 11 lines · Russell King

Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.

Actually it's the first time I'm seeing this.

Quoted 28 lines

Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

Quoted 6 lines

So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.
Which is more idiotic?

Why are you actually going to the trouble of freeing the mappings?

The MO of most consumers is either to allocate once on attach and free on detach (effectively tying up the memory forever) or to alloc and free descriptors as they come in and go out. In the former case, there's no need for free, in the latter the cycle of setup followed by teardown will add a pretty big latency overhead. Why not just use a pool implementation, so you always add but never free ... it should reach a steady state for the system and be a lot faster than the current implementation. Most architectures actually operate like this ... and they tend to alloc and free in page size chunks to make it easy.

If you're worried about memory, you can always reap the pool in the background.

James

Russell King

2008-02-09 17:24 UTC #6 lore raw

Quoted 43 lines · James Bottomley

Quoted 11 lines · Russell King
Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.
Actually it's the first time I'm seeing this.
Quoted 28 lines
Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

Quoted 8 lines

Quoted 6 lines
So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.
Which is more idiotic?
Why are you actually going to the trouble of freeing the mappings?

If we don't free the mappings, we'll eventually end up running out of VM space for the coherent DMA. We only have about 2MB of virtual space for these mappings on most machines.

Quoted 10 lines

The MO of most consumers is either to allocate once on attach and free on detach (effectively tying up the memory forever) or to alloc and free descriptors as they come in and go out. In the former case, there's no need for free, in the latter the cycle of setup followed by teardown will add a pretty big latency overhead. Why not just use a pool implementation, so you always add but never free ... it should reach a steady state for the system and be a lot faster than the current implementation. Most architectures actually operate like this ... and they tend to alloc and free in page size chunks to make it easy.

Quoted 2 lines

If you're worried about memory, you can always reap the pool in the background.

The existing implementation comes from the original PCI DMA code, which had the restriction that free was not called from interrupt contexts.

We now seem to be saying that the newer generic DMA API has different requirements, and these have not been reflected in the ARM implementation. If some of these folk who have the problem want to provide a patch to implement what you've outlined above, I'll certainly review it.

Signature · 3 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

James Bottomley

maintainer

2008-02-09 17:33 UTC #7 lore raw

Quoted 49 lines · Russell King

Quoted 48 lines · James Bottomley
Quoted 11 lines · Russell King
Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.
Actually it's the first time I'm seeing this.
Quoted 33 lines
Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */
So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Which is more idiotic?
Why are you actually going to the trouble of freeing the mappings?
If we don't free the mappings, we'll eventually end up running out of VM space for the coherent DMA. We only have about 2MB of virtual space for these mappings on most machines.

No, free the memory back to the coherent DMA pool but don't free the mappings; the next use takes a page with existing mappings, thus saving the mapping setup/teardown cycle. My contention is that the system reaches steady state even for alloc/free drivers.

Quoted 13 lines

Quoted 10 lines
The MO of most consumers is either to allocate once on attach and free on detach (effectively tying up the memory forever) or to alloc and free descriptors as they come in and go out. In the former case, there's no need for free, in the latter the cycle of setup followed by teardown will add a pretty big latency overhead. Why not just use a pool implementation, so you always add but never free ... it should reach a steady state for the system and be a lot faster than the current implementation. Most architectures actually operate like this ... and they tend to alloc and free in page size chunks to make it easy.
Quoted 2 lines
If you're worried about memory, you can always reap the pool in the background.

Quoted 7 lines

The existing implementation comes from the original PCI DMA code, which had the restriction that free was not called from interrupt contexts.
We now seem to be saying that the newer generic DMA API has different requirements, and these have not been reflected in the ARM implementation. If some of these folk who have the problem want to provide a patch to implement what you've outlined above, I'll certainly review it.

Being able to allocate from interrupt but not free from interrupt does violate the principle of least surprise.

James

[email protected]

author

2008-02-04 03:56 UTC #8 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #1 from [email protected] 2008-02-03 19:55 -------
Reply-To: [email protected]

Quoted 44 lines · On Sun, 2008-02-03 at 19:00 -0800, [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=9880

Latest working kernel version: 2.6.23.12
Earliest failing kernel version: 2.6.24
Distribution: Gentoo
Hardware Environment:
Intel Core2Quad, 2GB Memory, Areca 1220ML

Software Environment:
Areca CLI, Version: 1.72.250, Date: Apr 11 2007( Linux ) Areca HTTP proxy server V1.81.250 for Areca RAID controllers.

Problem Description:

Feb  2 02:04:01 mrb WARNING: at arch/x86/kernel/pci-dma_64.c:169
dma_free_coherent()
Feb  2 02:04:01 mrb Pid: 16080, comm: archttp64 Not tainted
2.6.24 #1
Feb  2 02:04:01 mrb
Feb  2 02:04:01 mrb Call Trace:
Feb  2 02:04:01 mrb [<ffffffff80213568>] dma_free_coherent+0x43/0x82
Feb  2 02:04:01 mrb [<ffffffff88178b85>] :arcmsr:arcmsr_queue_command
+0x379/0x973
Feb  2 02:04:01 mrb [<ffffffff8053d8c2>] _spin_unlock_irqrestore
+0x16/0x31
Feb  2 02:04:01 mrb [<ffffffff8043cce3>] scsi_dispatch_cmd+0x194/0x1e9
Feb  2 02:04:01 mrb [<ffffffff80441bab>] scsi_request_fn+0x28b/0x360
Feb  2 02:04:01 mrb [<ffffffff80387726>] blk_execute_rq_nowait+0x7a/0x8b
Feb  2 02:04:01 mrb [<ffffffff80441698>] scsi_execute_async+0x33d/0x36e
Feb  2 02:04:01 mrb [<ffffffff88159379>] :sg:sg_common_write+0x66f/0x6a3
Feb  2 02:04:01 mrb [<ffffffff881595c0>] :sg:sg_cmd_done+0x0/0x1fe
Feb  2 02:04:01 mrb [<ffffffff8053d97f>] _write_unlock_irqrestore
+0x17/0x34
Feb  2 02:04:01 mrb [<ffffffff8815959d>] :sg:sg_new_write+0x1f0/0x213
Feb  2 02:04:01 mrb [<ffffffff8815ab47>] :sg:sg_ioctl+0x1f3/0x9f5
Feb  2 02:04:01 mrb [<ffffffff8024d1c0>] hrtimer_try_to_cancel+0x5f/0x68
Feb  2 02:04:01 mrb [<ffffffff8024d1d5>] hrtimer_cancel+0xc/0x16
Feb  2 02:04:01 mrb [<ffffffff8053c62a>] do_nanosleep+0x55/0x7e
Feb  2 02:04:01 mrb [<ffffffff8029ae2d>] do_ioctl+0x55/0x6b
Feb  2 02:04:01 mrb [<ffffffff8029b076>] vfs_ioctl+0x233/0x24c
Feb  2 02:04:01 mrb [<ffffffff8029b0e0>] sys_ioctl+0x51/0x71
Feb  2 02:04:01 mrb [<ffffffff8020bfae>] system_call+0x7e/0x83

Quoted 7 lines

Steps to reproduce:
If the httpd deamon runs the message occurs endless. The areca cli command produces the message for each call.
The problem occurs after upgrading to kernel 2.6.24 kernel 2.6.23.12 and before works well.

This is here, isn't it:

void dma_free_coherent(struct device *dev, size_t size,
void *vaddr, dma_addr_t bus)
{
WARN_ON(irqs_disabled()); /* for portability */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        if (dma_ops->unmap_single)
                dma_ops->unmap_single(dev, bus, size, 0);
        free_pages((unsigned long)vaddr, get_order(size));
}

That's caused by this commit:

commit aa24886e379d2b641c5117e178b15ce1d5d366ba

Author: David Brownell <[email protected]>
Date:   Fri Aug 10 13:10:27 2007 -0700

dma_free_coherent() needs irqs enabled (sigh)

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

James

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[email protected]

author

2008-02-04 04:37 UTC #9 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #2 from [email protected] 2008-02-03 20:36 -------
Reply-To: [email protected]

Quoted 6 lines · James Bottomley

That's caused by this commit:
commit aa24886e379d2b641c5117e178b15ce1d5d366ba
Author: David Brownell <[email protected]>
Date: � Fri Aug 10 13:10:27 2007 -0700

Quoted 7 lines

� � dma_free_coherent() needs irqs enabled (sigh)
� �
I've cc'd the people responsible for this apparent bit of idiocy. �Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?

That was my reaction ... but I was told this would not be fixed.

Quoted 2 lines

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

Lacking a tasklet in dma_free_coherent() analagous to that omap_udc commit I referenced ...

Signature · 7 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[email protected]

author

2008-02-04 08:20 UTC #10 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #3 from [email protected] 2008-02-04 00:20 -------

Quoted 5 lines · James Bottomley

I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?

x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other
CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler. */

arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

Which is more idiotic?

Quoted 2 lines

Is it seriously true that you can call dma_alloc_coherent() from atomic context on arm, but not dma_free_coherent()?

Yes.

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[email protected]

author

2008-02-09 16:54 UTC #11 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #4 from [email protected] 2008-02-09 08:52 -------
Reply-To: [email protected]

Quoted 11 lines · Russell King

Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.

Actually it's the first time I'm seeing this.

Quoted 28 lines

Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

Quoted 6 lines

So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.
Which is more idiotic?

Why are you actually going to the trouble of freeing the mappings?

If you're worried about memory, you can always reap the pool in the background.

James

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[email protected]

author

2008-02-09 17:25 UTC #12 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #5 from [email protected] 2008-02-09 09:24 -------

Quoted 43 lines · James Bottomley

Quoted 11 lines · Russell King
Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.
Actually it's the first time I'm seeing this.
Quoted 28 lines
Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */

Quoted 8 lines

Quoted 6 lines
So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.
Which is more idiotic?
Why are you actually going to the trouble of freeing the mappings?

If we don't free the mappings, we'll eventually end up running out of VM space for the coherent DMA. We only have about 2MB of virtual space for these mappings on most machines.

Quoted 10 lines

The MO of most consumers is either to allocate once on attach and free on detach (effectively tying up the memory forever) or to alloc and free descriptors as they come in and go out. In the former case, there's no need for free, in the latter the cycle of setup followed by teardown will add a pretty big latency overhead. Why not just use a pool implementation, so you always add but never free ... it should reach a steady state for the system and be a lot faster than the current implementation. Most architectures actually operate like this ... and they tend to alloc and free in page size chunks to make it easy.

Quoted 2 lines

If you're worried about memory, you can always reap the pool in the background.

The existing implementation comes from the original PCI DMA code, which had the restriction that free was not called from interrupt contexts.

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[email protected]

author

2008-02-09 17:46 UTC #13 lore raw

In reply to a message not in the archive

http://bugzilla.kernel.org/show_bug.cgi?id=9880

------- Comment #6 from [email protected] 2008-02-09 09:34 -------
Reply-To: [email protected]

Quoted 49 lines · Russell King

Quoted 48 lines · James Bottomley
Quoted 11 lines · Russell King
Quoted 5 lines · James Bottomley
I've cc'd the people responsible for this apparent bit of idiocy. Since the API addition to dma_alloc_coherent() was the GFP flags so you could call it from interrupt context with GFP_ATOMIC if so desired, (pci_dma_alloc_consistent always has GFP_ATOMIC semantics), why on earth would the corresponding free routine require non-atomic semantics?
For the N-th time, when tearing down the MMU mappings which we need to setup for coherent mappings, we need to invalidate the TLB. On SMP systems, that means doing an IPI to the other CPUs to tell them to invalidate their TLBs as well.
Actually it's the first time I'm seeing this.
Quoted 33 lines
Now, look at the restrictions on calling smp_call_function_on_cpu() or equivalent function which is used to do the IPIs. I don't care if you look that up for x86 or ARM, because the restrictions are the same
- see the last two lines:
x86:
/**
* smp_call_function_mask(): Run a function on a set of other CPUs.
* @mask: The set of cpus to run on. Must not include the current cpu.
* @func: The function to run. This must be fast and non-blocking.
* @info: An arbitrary pointer to pass to the function.
* @wait: If true, wait (atomically) until function has completed on other CPUs. *
* Returns 0 on success, else a negative status code.
*
* If @wait is true, then returns once @func has returned; otherwise
* it returns just before the target cpu calls @func.
*
* You must not call this function with disabled interrupts or from a
* hardware interrupt handler or from a bottom half handler.
*/
arm:
/*
* You must not call this function with disabled interrupts, from a
* hardware interrupt handler, nor from a bottom half handler. */
So, if the architecture requires you to IPI the TLB flush to other CPUs this restriction has to propagate to dma_free_coherent. Or we just don't provide *any* DMA coherent memory and dictate that such platforms can never use DMA.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Which is more idiotic?
Why are you actually going to the trouble of freeing the mappings?
If we don't free the mappings, we'll eventually end up running out of VM space for the coherent DMA. We only have about 2MB of virtual space for these mappings on most machines.

Quoted 13 lines

Quoted 10 lines
The MO of most consumers is either to allocate once on attach and free on detach (effectively tying up the memory forever) or to alloc and free descriptors as they come in and go out. In the former case, there's no need for free, in the latter the cycle of setup followed by teardown will add a pretty big latency overhead. Why not just use a pool implementation, so you always add but never free ... it should reach a steady state for the system and be a lot faster than the current implementation. Most architectures actually operate like this ... and they tend to alloc and free in page size chunks to make it easy.
Quoted 2 lines
If you're worried about memory, you can always reap the pool in the background.

Quoted 7 lines

The existing implementation comes from the original PCI DMA code, which had the restriction that free was not called from interrupt contexts.
We now seem to be saying that the newer generic DMA API has different requirements, and these have not been reflected in the ARM implementation. If some of these folk who have the problem want to provide a patch to implement what you've outlined above, I'll certainly review it.

Being able to allocate from interrupt but not free from interrupt does violate the principle of least surprise.

James

Signature · 3 lines

Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.