| 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: <20100924220419.GC24654@ovro.caltech.edu>
Date: Fri, 24 Sep 2010 15:04:19 -0700
From: "Ira W. Snyder" <iws@...o.caltech.edu>
To: Dan Williams <dan.j.williams@...el.com>
Cc: "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFCv1 1/2] dmaengine: add support for scatterlist to
scatterlist transfers
On Fri, Sep 24, 2010 at 02:53:14PM -0700, Dan Williams wrote:
> On Fri, 2010-09-24 at 14:24 -0700, Ira W. Snyder wrote:
> > On Fri, Sep 24, 2010 at 01:40:56PM -0700, Dan Williams wrote:
> > > I don't think any dma channels gracefully handle descriptors that were
> > > prepped but not submitted. You would probably need to submit the
> > > backlog, poll for completion, and then return the error.
> > > Alternatively, the expectation is that descriptor allocations are
> > > transient, i.e. once previously submitted transactions are completed
> > > the descriptors will return to the available pool. So you could do
> > > what async_tx routines do and just poll for a descriptor.
> > >
> >
> > Can you give me an example? Even some pseudocode would help.
>
> Here is one from do_async_gen_syndrome() in crypto/async_tx/async_pq.c:
>
> /* Since we have clobbered the src_list we are committed
> * to doing this asynchronously. Drivers force forward
> * progress in case they can not provide a descriptor
> */
> for (;;) {
> tx = dma->device_prep_dma_pq(chan, dma_dest,
> &dma_src[src_off],
> pq_src_cnt,
> &coefs[src_off], len,
> dma_flags);
> if (likely(tx))
> break;
> async_tx_quiesce(&submit->depend_tx);
> dma_async_issue_pending(chan);
> }
>
> > The other DMAEngine functions (dma_async_memcpy_*()) don't do anything
> > with the descriptor if submit fails. Take for example
> > dma_async_memcpy_buf_to_buf(). If tx->tx_submit(tx); fails, any code
> > using it has no way to return the descriptor to the free pool.
> >
> > Does tx_submit() implicitly return descriptors to the free pool if it
> > fails?
>
> No, submit() failures are a hold over from when the ioatdma driver used
> to perform additional descriptor allocation at ->submit() time. After
> prep() the expectation is that the engine is just waiting to be told
> "go" and can't fail. The only reason ->submit() retains a return code
> is to support the "cookie" based method for polling for operation
> completion. A dma driver should handle all descriptor submission
> failure scenarios at prep time.
>
Ok, that's more like what I expected. So we still need the try forever
code similar to the above. I can add that for the next version.
> > Ok, I thought the list was clearer, but this is equally easy. How about
> > the following change that does away with the list completely. Then
> > things should work on ioatdma as well.
> >
> > From d59569ff48a89ef5411af3cf2995af7b742c5cd3 Mon Sep 17 00:00:00 2001
> > From: Ira W. Snyder <iws@...o.caltech.edu>
> > Date: Fri, 24 Sep 2010 14:18:09 -0700
> > Subject: [PATCH] dma: improve scatterlist to scatterlist transfer
> >
> > This is an improved algorithm to improve support on the Intel I/OAT
> > driver.
> >
> > Signed-off-by: Ira W. Snyder <iws@...o.caltech.edu>
> > ---
> > drivers/dma/dmaengine.c | 52 +++++++++++++++++++++-----------------------
> > include/linux/dmaengine.h | 3 --
> > 2 files changed, 25 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> > index 57ec1e5..cde775c 100644
> > --- a/drivers/dma/dmaengine.c
> > +++ b/drivers/dma/dmaengine.c
> > @@ -983,10 +983,13 @@ dma_async_memcpy_sg_to_sg(struct dma_chan *chan,
> > struct dma_async_tx_descriptor *tx;
> > dma_cookie_t cookie = -ENOMEM;
> > size_t dst_avail, src_avail;
> > - struct list_head tx_list;
> > + struct scatterlist *sg;
> > size_t transferred = 0;
> > + size_t dst_total = 0;
> > + size_t src_total = 0;
> > dma_addr_t dst, src;
> > size_t len;
> > + int i;
> >
> > if (dst_nents == 0 || src_nents == 0)
> > return -EINVAL;
> > @@ -994,12 +997,17 @@ dma_async_memcpy_sg_to_sg(struct dma_chan *chan,
> > if (dst_sg == NULL || src_sg == NULL)
> > return -EINVAL;
> >
> > + /* get the total count of bytes in each scatterlist */
> > + for_each_sg(dst_sg, sg, dst_nents, i)
> > + dst_total += sg_dma_len(sg);
> > +
> > + for_each_sg(src_sg, sg, src_nents, i)
> > + src_total += sg_dma_len(sg);
> > +
>
> What about overrun or underrun do we not care if src_total != dst_total?
>
> Otherwise looks ok.
>
I don't know if we should care about that. The algorithm handles that
case just fine. It copies the maximum amount it can, which is exactly
min(src_total, dst_total). Whichever scatterlist runs out of entries
first is the shortest.
As a real world example, my driver verifies that both scatterlists have
exactly the right number of bytes available before trying to program the
hardware.
Ira
--
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