From: Jon Mason <jon.mason@intel.com>
To: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: dan.j.williams@intel.com, vinod.koul@intel.com,
dave.jiang@intel.com, t.figa@samsung.com,
kyungmin.park@samsung.com, linux@arm.linux.org.uk,
linux-kernel@vger.kernel.org, Dan Williams <djbw@fb.com>
Subject: Re: [PATCH v2 11/13] NTB: convert to dmaengine_unmap_data
Date: Fri, 18 Oct 2013 18:06:02 -0700 [thread overview]
Message-ID: <20131019010601.GH25432@jonmason-lab> (raw)
In-Reply-To: <1382117733-16720-12-git-send-email-b.zolnierkie@samsung.com>
On Fri, Oct 18, 2013 at 07:35:31PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Use the generic unmap object to unmap dma buffers.
>
> As NTB can be compiled without DMA_ENGINE support add
Seems like the stubs should be added outside of this patch. Also, the
comment implies that NTB could not be compiled without DMA_ENGINE
support before, which it could be.
> stub functions to <linux/dmaengine.h> for dma_set_unmap(),
> dmaengine_get_unmap_data() and dmaengine_unmap_put().
>
> Cc: Dan Williams <djbw@fb.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Jon Mason <jon.mason@intel.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/ntb/ntb_transport.c | 63 +++++++++++++++++++++++++++++++++------------
> include/linux/dmaengine.h | 15 +++++++++++
> 2 files changed, 61 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 12a9e83..fc6bbf1 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1034,7 +1034,7 @@ static void ntb_async_rx(struct ntb_queue_entry *entry, void *offset,
> struct dma_chan *chan = qp->dma_chan;
> struct dma_device *device;
> size_t pay_off, buff_off;
> - dma_addr_t src, dest;
> + struct dmaengine_unmap_data *unmap;
> dma_cookie_t cookie;
> void *buf = entry->buf;
> unsigned long flags;
> @@ -1054,27 +1054,41 @@ static void ntb_async_rx(struct ntb_queue_entry *entry, void *offset,
> if (!is_dma_copy_aligned(device, pay_off, buff_off, len))
> goto err1;
>
> - dest = dma_map_single(device->dev, buf, len, DMA_FROM_DEVICE);
> - if (dma_mapping_error(device->dev, dest))
> + unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO);
> + if (!unmap)
> goto err1;
>
> - src = dma_map_single(device->dev, offset, len, DMA_TO_DEVICE);
> - if (dma_mapping_error(device->dev, src))
> + unmap->addr[0] = dma_map_page(device->dev, virt_to_page(offset),
> + pay_off, len, DMA_TO_DEVICE);
> + if (dma_mapping_error(device->dev, unmap->addr[0]))
> goto err2;
>
> - flags = DMA_COMPL_DEST_UNMAP_SINGLE | DMA_COMPL_SRC_UNMAP_SINGLE |
> + unmap->to_cnt = 1;
> +
> + unmap->addr[1] = dma_map_page(device->dev, virt_to_page(buf),
> + buff_off, len, DMA_FROM_DEVICE);
> + if (dma_mapping_error(device->dev, unmap->addr[1]))
> + goto err2;
> +
> + unmap->from_cnt = 1;
> +
> + flags = DMA_COMPL_SKIP_SRC_UNMAP | DMA_COMPL_SKIP_DEST_UNMAP |
> DMA_PREP_INTERRUPT;
> - txd = device->device_prep_dma_memcpy(chan, dest, src, len, flags);
> + txd = device->device_prep_dma_memcpy(chan, unmap->addr[1],
> + unmap->addr[0], len, flags);
I must say, as a user I dislike this interface much less than the
previous. Can we abstract all of this away in a helper function
that simply takes the src, dest, and len, then maps and calls
device_prep_dma_memcpy? The driver does not keep track of the
dmaengine_unmap_data, so the helper function could simply return an
error if something isn't happy. Thoughts?
Thanks,
Jon
> if (!txd)
> - goto err3;
> + goto err2;
>
> txd->callback = ntb_rx_copy_callback;
> txd->callback_param = entry;
> + dma_set_unmap(txd, unmap);
>
> cookie = dmaengine_submit(txd);
> if (dma_submit_error(cookie))
> goto err3;
>
> + dmaengine_unmap_put(unmap);
> +
> qp->last_cookie = cookie;
>
> qp->rx_async++;
> @@ -1082,9 +1096,9 @@ static void ntb_async_rx(struct ntb_queue_entry *entry, void *offset,
> return;
>
> err3:
> - dma_unmap_single(device->dev, src, len, DMA_TO_DEVICE);
> + dmaengine_unmap_put(unmap);
> err2:
> - dma_unmap_single(device->dev, dest, len, DMA_FROM_DEVICE);
> + dmaengine_unmap_put(unmap);
> err1:
> /* If the callbacks come out of order, the writing of the index to the
> * last completed will be out of order. This may result in the
> @@ -1245,7 +1259,8 @@ static void ntb_async_tx(struct ntb_transport_qp *qp,
> struct dma_chan *chan = qp->dma_chan;
> struct dma_device *device;
> size_t dest_off, buff_off;
> - dma_addr_t src, dest;
> + struct dmaengine_unmap_data *unmap;
> + dma_addr_t dest;
> dma_cookie_t cookie;
> void __iomem *offset;
> size_t len = entry->len;
> @@ -1273,28 +1288,42 @@ static void ntb_async_tx(struct ntb_transport_qp *qp,
> if (!is_dma_copy_aligned(device, buff_off, dest_off, len))
> goto err;
>
> - src = dma_map_single(device->dev, buf, len, DMA_TO_DEVICE);
> - if (dma_mapping_error(device->dev, src))
> + unmap = dmaengine_get_unmap_data(device->dev, 1, GFP_NOIO);
> + if (!unmap)
> goto err;
>
> - flags = DMA_COMPL_SRC_UNMAP_SINGLE | DMA_PREP_INTERRUPT;
> - txd = device->device_prep_dma_memcpy(chan, dest, src, len, flags);
> + unmap->addr[0] = dma_map_page(device->dev, virt_to_page(buf),
> + buff_off, len, DMA_TO_DEVICE);
> + if (dma_mapping_error(device->dev, unmap->addr[0]))
> + goto err1;
> +
> + unmap->to_cnt = 1;
> +
> + flags = DMA_COMPL_SKIP_SRC_UNMAP | DMA_COMPL_SKIP_DEST_UNMAP |
> + DMA_PREP_INTERRUPT;
> + txd = device->device_prep_dma_memcpy(chan, dest, unmap->addr[0], len,
> + flags);
> if (!txd)
> goto err1;
>
> txd->callback = ntb_tx_copy_callback;
> txd->callback_param = entry;
> + dma_set_unmap(txd, unmap);
>
> cookie = dmaengine_submit(txd);
> if (dma_submit_error(cookie))
> - goto err1;
> + goto err2;
> +
> + dmaengine_unmap_put(unmap);
>
> dma_async_issue_pending(chan);
> qp->tx_async++;
>
> return;
> +err2:
> + dmaengine_unmap_put(unmap);
> err1:
> - dma_unmap_single(device->dev, src, len, DMA_TO_DEVICE);
> + dmaengine_unmap_put(unmap);
> err:
> ntb_memcpy_tx(entry, offset);
> qp->tx_memcpy++;
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index dc98bc5..3782cdb 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -457,6 +457,7 @@ struct dma_async_tx_descriptor {
> #endif
> };
>
> +#ifdef CONFIG_DMA_ENGINE
> static inline void dma_set_unmap(struct dma_async_tx_descriptor *tx,
> struct dmaengine_unmap_data *unmap)
> {
> @@ -467,6 +468,20 @@ static inline void dma_set_unmap(struct dma_async_tx_descriptor *tx,
> struct dmaengine_unmap_data *
> dmaengine_get_unmap_data(struct device *dev, int nr, gfp_t flags);
> void dmaengine_unmap_put(struct dmaengine_unmap_data *unmap);
> +#else
> +static inline void dma_set_unmap(struct dma_async_tx_descriptor *tx,
> + struct dmaengine_unmap_data *unmap)
> +{
> +}
> +static inline struct dmaengine_unmap_data *
> +dmaengine_get_unmap_data(struct device *dev, int nr, gfp_t flags)
> +{
> + return NULL;
> +}
> +static inline void dmaengine_unmap_put(struct dmaengine_unmap_data *unmap)
> +{
> +}
> +#endif
>
> static inline void dma_descriptor_unmap(struct dma_async_tx_descriptor *tx)
> {
> --
> 1.8.2.3
>
next prev parent reply other threads:[~2013-10-19 1:06 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-18 17:35 [PATCH v2 00/13] dmaengine: introduce dmaengine_unmap_data Bartlomiej Zolnierkiewicz
2013-10-18 17:35 ` [PATCH v2 01/13] dmatest: make driver unmap also source buffers by itself Bartlomiej Zolnierkiewicz
2013-10-18 18:06 ` Andy Shevchenko
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 02/13] dmaengine: consolidate memcpy apis Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 03/13] dmaengine: prepare for generic 'unmap' data Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 04/13] dmaengine: reference counted unmap data Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 05/13] async_memcpy: convert to dmaengine_unmap_data Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 06/13] async_xor: " Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 07/13] async_xor_val: " Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 08/13] async_raid6_recov: " Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 09/13] async_pq: " Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 10/13] async_pq_val: " Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 11/13] NTB: " Bartlomiej Zolnierkiewicz
2013-10-19 1:06 ` Jon Mason [this message]
2013-10-22 21:08 ` Dan Williams
2013-10-22 21:29 ` Dan Williams
2013-10-22 23:12 ` Jon Mason
2013-10-23 1:05 ` Dan Williams
2013-10-23 1:18 ` Jon Mason
2013-10-18 17:35 ` [PATCH v2 12/13] dmaengine: remove DMA unmap from drivers Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-18 17:35 ` [PATCH v2 13/13] dmaengine: remove DMA unmap flags Bartlomiej Zolnierkiewicz
2013-10-22 21:08 ` Dan Williams
2013-10-22 23:07 ` Jon Mason
2013-10-28 22:54 ` Mark Brown
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=20131019010601.GH25432@jonmason-lab \
--to=jon.mason@intel.com \
--cc=b.zolnierkie@samsung.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=djbw@fb.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=t.figa@samsung.com \
--cc=vinod.koul@intel.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.