[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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


greg@kroah.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|greg@kroah.com              |scsi_drivers-other@kernel-
                   |                            |bugs.osdl.org
          Component|PCI                         |Other
            Product|Drivers                     |SCSI Drivers




-- 
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.

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[James Bottomley]

On Sun, 2008-02-03 at 19:00 -0800, bugme-daemon@bugzilla.kernel.org
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
> 
> 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 <david-b@pacbell.net>
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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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





------- Comment #1 from anonymous@kernel-bugs.osdl.org  2008-02-03 19:55 -------
Reply-To: James.Bottomley@HansenPartnership.com


On Sun, 2008-02-03 at 19:00 -0800, bugme-daemon@bugzilla.kernel.org
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
> 
> 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 <david-b@pacbell.net>
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


-- 
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.

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[David Brownell]

On Sunday 03 February 2008, James Bottomley wrote:
> That's caused by this commit:
> 
> commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> Author: David Brownell <david-b@pacbell.net>
> 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?

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.)


> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1819 bytes --]

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





------- Comment #2 from anonymous@kernel-bugs.osdl.org  2008-02-03 20:36 -------
Reply-To: david-b@pacbell.net

On Sunday 03 February 2008, James Bottomley wrote:
> That's caused by this commit:
> 
> commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> Author: David Brownell <david-b@pacbell.net>
> 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?

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.)


> 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 ...


-- 
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[Russell King]

On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> 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?

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

Yes.

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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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





------- Comment #3 from rmk@arm.linux.org.uk  2008-02-04 00:20 -------
On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> 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?

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

Yes.


-- 
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.

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[James Bottomley]

On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > 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.

> 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?

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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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





------- Comment #4 from anonymous@kernel-bugs.osdl.org  2008-02-09 08:52 -------
Reply-To: James.Bottomley@HansenPartnership.com

On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > 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.

> 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?

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


-- 
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.

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[Russell King]

On Sat, Feb 09, 2008 at 10:51:16AM -0600, James Bottomley wrote:
> On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> > On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > > 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.
> 
> > 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?
> 
> 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.

> 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.

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.

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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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





------- Comment #5 from rmk@arm.linux.org.uk  2008-02-09 09:24 -------
On Sat, Feb 09, 2008 at 10:51:16AM -0600, James Bottomley wrote:
> On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> > On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > > 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.
> 
> > 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?
> 
> 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.

> 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.

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.


-- 
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.

Re: [Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[James Bottomley]

On Sat, 2008-02-09 at 17:21 +0000, Russell King wrote:
> On Sat, Feb 09, 2008 at 10:51:16AM -0600, James Bottomley wrote:
> > On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> > > On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > > > 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.
> > 
> > > 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?
> > 
> > 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.

> > 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.
> 
> 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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugme-daemon]

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





------- Comment #6 from anonymous@kernel-bugs.osdl.org  2008-02-09 09:34 -------
Reply-To: James.Bottomley@HansenPartnership.com

On Sat, 2008-02-09 at 17:21 +0000, Russell King wrote:
> On Sat, Feb 09, 2008 at 10:51:16AM -0600, James Bottomley wrote:
> > On Mon, 2008-02-04 at 08:19 +0000, Russell King wrote:
> > > On Sun, Feb 03, 2008 at 09:55:37PM -0600, James Bottomley wrote:
> > > > 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.
> > 
> > > 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?
> > 
> > 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.

> > 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.
> 
> 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


-- 
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.

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugzilla-daemon]

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


Alan <alan@lxorguk.ukuu.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |alan@lxorguk.ukuu.org.uk
         Resolution|                            |OBSOLETE




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

[Bug 9880] dma_free_coherent in arcmsr when calling areca tools

[bugzilla-daemon]

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


Alan <alan@lxorguk.ukuu.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED




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