LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Nelson, Shannon" <shannon.nelson@intel.com>
To: "Haavard Skinnemoen" <hskinnemoen@norway.atmel.com>,
	"Haavard Skinnemoen" <hskinnemoen@atmel.com>
Cc: "Williams, Dan J" <dan.j.williams@intel.com>,
	<linux-kernel@vger.kernel.org>,
	"David Brownell" <david-b@pacbell.net>, <kernel@avr32linux.org>,
	"Francis Moreau" <francis.moro@gmail.com>,
	"Paul Mundt" <lethal@linux-sh.org>,
	"Vladimir A. Barinov" <vbarinov@ru.mvista.com>,
	"Pierre Ossman" <drzeus-list@drzeus.cx>
Subject: RE: [RFC v3 4/7] dmaengine: Add slave DMA interface
Date: Fri, 15 Feb 2008 09:12:35 -0800	[thread overview]
Message-ID: <BAE9DCEF64577A439B3A37F36F9B691C0407A37A@orsmsx418.amr.corp.intel.com> (raw)
In-Reply-To: <20080215105302.1e4251a3@dhcp-252-066.norway.atmel.com>

>From: Haavard Skinnemoen [mailto:hskinnemoen@norway.atmel.com] 
>Sent: Friday, February 15, 2008 1:53 AM
>To: Haavard Skinnemoen
>Cc: Williams, Dan J; linux-kernel@vger.kernel.org; Nelson, 
>Shannon; David Brownell; kernel@avr32linux.org; Francis 
>Moreau; Paul Mundt; Vladimir A. Barinov; Pierre Ossman
>Subject: Re: [RFC v3 4/7] dmaengine: Add slave DMA interface
>
>On Wed, 13 Feb 2008 20:24:02 +0100
>Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:
>
>> But looking at your latest patch series, I guess we can use the new
>> "next" field instead. It's not like we really need the full
>> capabilities of list_head.
>
>On second thought, if we do this, we would be using the "next" field in
>an undocumented way. It is currently documented as follows:
>
> * @next: at completion submit this descriptor
>
>But that's not how we're going to use it when doing slave transfers:
>We're using it to keep track of all the descriptors that have already
>been submitted.
>
>I think it would actually be better to go the other way and have the
>async_tx API extend the descriptor as well for its private fields. That
>way, we get the pointer we need, but we can document it differently.
>
>So how about we do something along the lines of the patch below? We
>need to update all the users too of course, but apart from making the
>dma_slave_descriptor struct smaller, I don't think it will change the
>actual memory layout at all.
>
>Haavard
>
>diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>index 9bb3407..abaf324 100644
>--- a/include/linux/dmaengine.h
>+++ b/include/linux/dmaengine.h
>@@ -267,8 +267,7 @@ struct dma_client {
> 
> typedef void (*dma_async_tx_callback)(void *dma_async_param);
> /**
>- * struct dma_async_tx_descriptor - async transaction descriptor
>- * ---dma generic offload fields---
>+ * struct dma_descriptor - generic dma offload descriptor
>  * @cookie: tracking cookie for this transaction, set to -EBUSY if
>  *	this tx is sitting on a dependency list
>  * @ack: the descriptor can not be reused until the client 
>acknowledges
>@@ -280,12 +279,8 @@ typedef void 
>(*dma_async_tx_callback)(void *dma_async_param);
>  * @tx_submit: set the prepared descriptor(s) to be executed 
>by the engine
>  * @callback: routine to call after this operation is complete
>  * @callback_param: general parameter to pass to the callback routine
>- * ---async_tx api specific fields---
>- * @next: at completion submit this descriptor
>- * @parent: pointer to the next level up in the dependency chain
>- * @lock: protect the parent and next pointers
>  */
>-struct dma_async_tx_descriptor {
>+struct dma_descriptor {
> 	dma_cookie_t cookie;
> 	int ack;
> 	dma_addr_t phys;
>@@ -294,6 +289,17 @@ struct dma_async_tx_descriptor {
> 	dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
> 	dma_async_tx_callback callback;
> 	void *callback_param;
>+};
>+
>+/**
>+ * struct dma_async_tx_descriptor - async transaction descriptor
>+ * @dma: generic dma descriptor
>+ * @next: at completion submit this descriptor
>+ * @parent: pointer to the next level up in the dependency chain
>+ * @lock: protect the parent and next pointers
>+ */
>+struct dma_async_tx_descriptor {
>+	struct dma_descriptor dma;
> 	struct dma_async_tx_descriptor *next;
> 	struct dma_async_tx_descriptor *parent;
> 	spinlock_t lock;
>@@ -301,13 +307,13 @@ struct dma_async_tx_descriptor {
> 
> /**
>  * struct dma_slave_descriptor - extended DMA descriptor for slave DMA
>- * @async_tx: async transaction descriptor
>- * @client_node: for use by the client, for example when operating on
>- *	scatterlists.
>+ * @dma: generic dma descriptor
>+ * @next: for use by the client, for example to keep track of
>+ *	submitted descriptors when dealing with scatterlists.
>  */
> struct dma_slave_descriptor {
>-	struct dma_async_tx_descriptor txd;
>-	struct list_head client_node;
>+	struct dma_descriptor dma;
>+	struct dma_slave_descriptor *next;
> };
> 
> /**
>

I'll jump in here briefly - I'm okay with the direction this is going,
but I want to be protective of ioatdma performance.  As used in struct
ioat_desc_sw, the cookie and ack elements end up very close to the end
of a cache line and I'd like them to not get pushed out across the
boundry.  I don't think this proposal changes the layout, I'm just
bringing up my concern.

Selfishly,
sln

  reply	other threads:[~2008-02-15 17:17 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-12 16:43 [RFC v3 1/7] dmaengine: Couple DMA channels to their physical DMA device Haavard Skinnemoen
2008-02-12 16:43 ` [RFC v3 2/7] dmaengine: Add dma_client parameter to device_alloc_chan_resources Haavard Skinnemoen
2008-02-12 16:43   ` [RFC v3 3/7] dmaengine: Add dma_chan_is_in_use() function Haavard Skinnemoen
2008-02-12 16:43     ` [RFC v3 4/7] dmaengine: Add slave DMA interface Haavard Skinnemoen
2008-02-12 16:43       ` [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users Haavard Skinnemoen
2008-02-12 16:43         ` [RFC v3 6/7] dmaengine: Driver for the Synopsys DesignWare DMA controller Haavard Skinnemoen
2008-02-12 16:43           ` [RFC v3 7/7] Atmel MCI: Driver for Atmel on-chip MMC controllers Haavard Skinnemoen
2008-02-12 20:43         ` [RFC v3 5/7] dmaengine: Make DMA Engine menu visible for AVR32 users Olof Johansson
2008-02-12 22:13           ` Haavard Skinnemoen
2008-02-12 22:27             ` Dan Williams
2008-02-13  8:44               ` Haavard Skinnemoen
2008-02-13  7:21       ` [RFC v3 4/7] dmaengine: Add slave DMA interface Dan Williams
2008-02-13  8:03         ` Haavard Skinnemoen
2008-02-13 19:07       ` Dan Williams
2008-02-13 19:24         ` Haavard Skinnemoen
2008-02-15  9:53           ` Haavard Skinnemoen
2008-02-15 17:12             ` Nelson, Shannon [this message]
2008-02-18 13:29               ` Haavard Skinnemoen
2008-02-19 18:46                 ` Nelson, Shannon
2008-02-16 20:06             ` Dan Williams
2008-02-18 13:22               ` Haavard Skinnemoen
2008-02-18 22:42                 ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BAE9DCEF64577A439B3A37F36F9B691C0407A37A@orsmsx418.amr.corp.intel.com \
    --to=shannon.nelson@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=david-b@pacbell.net \
    --cc=drzeus-list@drzeus.cx \
    --cc=francis.moro@gmail.com \
    --cc=hskinnemoen@atmel.com \
    --cc=hskinnemoen@norway.atmel.com \
    --cc=kernel@avr32linux.org \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vbarinov@ru.mvista.com \
    --subject='RE: [RFC v3 4/7] dmaengine: Add slave DMA interface' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).