| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BAE9DCEF64577A439B3A37F36F9B691C0407A37A@orsmsx418.amr.corp.intel.com>
Date: Fri, 15 Feb 2008 09:12:35 -0800
From: "Nelson, Shannon" <shannon.nelson@...el.com>
To: "Haavard Skinnemoen" <hskinnemoen@...way.atmel.com>,
"Haavard Skinnemoen" <hskinnemoen@...el.com>
Cc: "Williams, Dan J" <dan.j.williams@...el.com>,
<linux-kernel@...r.kernel.org>,
"David Brownell" <david-b@...bell.net>, <kernel@...32linux.org>,
"Francis Moreau" <francis.moro@...il.com>,
"Paul Mundt" <lethal@...ux-sh.org>,
"Vladimir A. Barinov" <vbarinov@...mvista.com>,
"Pierre Ossman" <drzeus-list@...eus.cx>
Subject: RE: [RFC v3 4/7] dmaengine: Add slave DMA interface
>From: Haavard Skinnemoen [mailto:hskinnemoen@...way.atmel.com]
>Sent: Friday, February 15, 2008 1:53 AM
>To: Haavard Skinnemoen
>Cc: Williams, Dan J; linux-kernel@...r.kernel.org; Nelson,
>Shannon; David Brownell; kernel@...32linux.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@...el.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists