[PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Nuno Sá]

From: Michael Hennerich <michael.hennerich@analog.com>

iio_dmaengine_buffer_submit_block() counts scatterlist entries with
sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
the mapping so that only the first sgt->nents entries carry valid DMA
addresses, with nents < orig_nents.

On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
count is 2, so the loop reads one entry past the mapped set and emits a
garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
vec array (prep returns NULL), the fence is signalled with -ENOMEM,
which a userspace poller cannot observe, and the block is left in
ACTIVE state so every further enqueue of it fails with -EBUSY. The
visible symptom is a stream of zero-filled blocks followed by a wedged
buffer.

Platforms without an IOMMU never hit this because nents == orig_nents.

Size the vec array with sg_nents_for_dma(), which walks the DMA-mapped
view and accounts for max-length splitting, and stop the fill loop once
bytes_used is covered - which is allowed to be smaller than the block
size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().

Assisted-by: Claude:claude-fable-5
Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
Note the Signed-off-by is just because I'm carrying Michael's patch!
---
 drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
index ecc02a427b92..bece45381c8c 100644
--- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
+++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
@@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 	if (block->sg_table) {
 		unsigned long flags;
 
+		/*
+		 * Use the DMA-mapped view of the sg_table: after mapping
+		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
+		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
+		 * are only valid for the first sgt->nents entries. Counting
+		 * with sg_nents_for_len() (CPU lengths) walks past them and
+		 * hands garbage vecs to the DMA engine.
+		 */
 		sgl = block->sg_table->sgl;
-		nents = sg_nents_for_len(sgl, block->bytes_used);
-		if (nents < 0)
-			return nents;
+		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);
 
 		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
 		if (!vecs)
@@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 
 		len_total = block->bytes_used;
 
-		for (i = 0; i < nents; i++) {
+		for (i = 0; i < nents && len_total; i++) {
 			vecs[i].addr = sg_dma_address(sgl);
 			vecs[i].len = min(sg_dma_len(sgl), len_total);
 			len_total -= vecs[i].len;
@@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
 		 * before it can run, so always set the EOT flag.
 		 */
 		flags |= DMA_PREP_LOAD_EOT;
+		nents = i;
+
 		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
 							 vecs, nents, dma_dir,
 							 flags);

---
base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
--

Thanks!
- Nuno Sá

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Jonathan Cameron]

On Tue, 18 Aug 2026 17:45:29 +0100
Nuno Sá <nuno.sa@analog.com> wrote:

> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> iio_dmaengine_buffer_submit_block() counts scatterlist entries with
> sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
> then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
> After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
> the mapping so that only the first sgt->nents entries carry valid DMA
> addresses, with nents < orig_nents.
> 
> On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
> system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
> count is 2, so the loop reads one entry past the mapped set and emits a
> garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
> vec array (prep returns NULL), the fence is signalled with -ENOMEM,
> which a userspace poller cannot observe, and the block is left in
> ACTIVE state so every further enqueue of it fails with -EBUSY. The
> visible symptom is a stream of zero-filled blocks followed by a wedged
> buffer.
> 
> Platforms without an IOMMU never hit this because nents == orig_nents.
> 
> Size the vec array with sg_nents_for_dma(), which walks the DMA-mapped
> view and accounts for max-length splitting, and stop the fill loop once
> bytes_used is covered - which is allowed to be smaller than the block
> size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> 
> Assisted-by: Claude:claude-fable-5
> Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

There are some gremlins nearby in this code...
In the else just of this context seems max_size is computed again
having been done just above the code seen here.

Unless I'm missing something that should be cleaned up as well.

Been a while since I got my head into the scatterlist
stuff, so I might have it wrong below, but I don't think what
you have here actually works if the merging of entries is
larger than the max dma entry the hardware supports.


> ---
> Note the Signed-off-by is just because I'm carrying Michael's patch!
> ---
>  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index ecc02a427b92..bece45381c8c 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  	if (block->sg_table) {
>  		unsigned long flags;
>  
> +		/*
> +		 * Use the DMA-mapped view of the sg_table: after mapping
> +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> +		 * are only valid for the first sgt->nents entries. Counting
> +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> +		 * hands garbage vecs to the DMA engine.

This feels like too much info after the fix is in place.  Talking about other
stuff that would be wrong is rather unusual.

> +		 */
>  		sgl = block->sg_table->sgl;
> -		nents = sg_nents_for_len(sgl, block->bytes_used);
> -		if (nents < 0)
> -			return nents;
> +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);

So this fun function will generally give us the number of sgl entries, but not
quite always. It will give us how many chunks of up to max_size fit into
a particularly large entry.

>  
>  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
>  		if (!vecs)
> @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  
>  		len_total = block->bytes_used;
>  
> -		for (i = 0; i < nents; i++) {
> +		for (i = 0; i < nents && len_total; i++) {
So this needs to be more clever as we aren't just iterating entrees and filling
them in, some of them could at least in theory be too big to fit
in a single vec - hence you need to do a loop in here that sets
multiple entries if that occurs.

If that can't happen for some other reason then I think you can 
just use block->sgtable->nents instead of the more complex call above.

>  			vecs[i].addr = sg_dma_address(sgl);
>  			vecs[i].len = min(sg_dma_len(sgl), len_total);
>  			len_total -= vecs[i].len;
> @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  		 * before it can run, so always set the EOT flag.
>  		 */
>  		flags |= DMA_PREP_LOAD_EOT;
> +		nents = i;
> +
>  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
>  							 vecs, nents, dma_dir,
>  							 flags);
> 
> ---
> base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> --
> 
> Thanks!
> - Nuno Sá
> 

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Nuno Sá]

On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> On Tue, 18 Aug 2026 17:45:29 +0100
> Nuno Sá <nuno.sa@analog.com> wrote:
> 
> > From: Michael Hennerich <michael.hennerich@analog.com>
> > 
> > iio_dmaengine_buffer_submit_block() counts scatterlist entries with
> > sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
> > then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
> > After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
> > the mapping so that only the first sgt->nents entries carry valid DMA
> > addresses, with nents < orig_nents.
> > 
> > On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
> > system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
> > count is 2, so the loop reads one entry past the mapped set and emits a
> > garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
> > vec array (prep returns NULL), the fence is signalled with -ENOMEM,
> > which a userspace poller cannot observe, and the block is left in
> > ACTIVE state so every further enqueue of it fails with -EBUSY. The
> > visible symptom is a stream of zero-filled blocks followed by a wedged
> > buffer.
> > 
> > Platforms without an IOMMU never hit this because nents == orig_nents.
> > 
> > Size the vec array with sg_nents_for_dma(), which walks the DMA-mapped
> > view and accounts for max-length splitting, and stop the fill loop once
> > bytes_used is covered - which is allowed to be smaller than the block
> > size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> > 
> > Assisted-by: Claude:claude-fable-5
> > Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
> > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> 
> There are some gremlins nearby in this code...
> In the else just of this context seems max_size is computed again
> having been done just above the code seen here.
> 
> Unless I'm missing something that should be cleaned up as well.

Oh yes! Something I already noticed a couple of times but never sent the
patch right away so I kept forgetting about it.
> 
> Been a while since I got my head into the scatterlist
> stuff, so I might have it wrong below, but I don't think what
> you have here actually works if the merging of entries is
> larger than the max dma entry the hardware supports.
> 

In theory might not be a bug but I'm not sure it's a claim we can fully
take as guarantee.

> 
> > ---
> > Note the Signed-off-by is just because I'm carrying Michael's patch!
> > ---
> >  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > index ecc02a427b92..bece45381c8c 100644
> > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  	if (block->sg_table) {
> >  		unsigned long flags;
> >  
> > +		/*
> > +		 * Use the DMA-mapped view of the sg_table: after mapping
> > +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> > +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> > +		 * are only valid for the first sgt->nents entries. Counting
> > +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> > +		 * hands garbage vecs to the DMA engine.
> 
> This feels like too much info after the fix is in place.  Talking about other
> stuff that would be wrong is rather unusual.
>

Oh well, I guess LLM over commenting as usual.

> > +		 */
> >  		sgl = block->sg_table->sgl;
> > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > -		if (nents < 0)
> > -			return nents;
> > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);
> 
> So this fun function will generally give us the number of sgl entries, but not
> quite always. It will give us how many chunks of up to max_size fit into
> a particularly large entry.
> 
> >  
> >  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
> >  		if (!vecs)
> > @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  
> >  		len_total = block->bytes_used;
> >  
> > -		for (i = 0; i < nents; i++) {
> > +		for (i = 0; i < nents && len_total; i++) {
> So this needs to be more clever as we aren't just iterating entrees and filling
> them in, some of them could at least in theory be too big to fit
> in a single vec - hence you need to do a loop in here that sets
> multiple entries if that occurs.

I see! But I think that merging means that we end up with more nents
(sg_nents_for_len()) entries than DMA ones which was the issue we had
because we were left with vecs with invalid addresses and 0 sized. The other
way around should not happen on the IOMMU path at least given the assumption
that a single sg  entry must not be bigger than max_len [1]. I think in theory
that can actually happen (if we end using a dma provider defaulting to 64K and 
cma dma_bufs) but maybe we can argue that's not our problem? Given
the assumptions of course.

But even if we somehow reach this path with sg_dma_address > max_size
and end up stuffing all of it in say, vec[0], the DMA controller should
also care to make sure it splits each vec if it exceeds the descriptor
max size. That's what both users of (ADI axi_dmac being one them)
.device_prep_peripheral_dma_vec() are doing today. But yes, I agree that
might be a bold assumption.

> 
> If that can't happen for some other reason then I think you can 
> just use block->sgtable->nents instead of the more complex call above.
> 

I was the one suggesting the other call because it seemed what we
actually wanted but I'm fine with just using the above. I'm more tempted
to keep it simple and use 'block->sgtable->nents' rather than jumping in
a more complex subloop without clear evidence we really need it.
Thoughts?

As a fun side note this also made visible another subtle issue. Given
that submitting a block might only happen when enabling the buffer (so
async to enqueueing it) when we hit this issue, we get -ENOMEM and
and go ahead  to wake up poll(). But AFAIK, we can't really propagate
the error code up to userspace so userspace wakes up just to get
garbage and thinking everything is fine. Easy way out would be to treat
this as and hard error and return error when enabling the buffer. But
that also raises the question that other blocks might have been properly
submitted though. Not really sure how to handle this but anyways a problem
for another day :)

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ab2cbeb0ed301a9f0460078e91b09f39958212ef

- Nuno Sá

> >  			vecs[i].addr = sg_dma_address(sgl);
> >  			vecs[i].len = min(sg_dma_len(sgl), len_total);
> >  			len_total -= vecs[i].len;
> > @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> >  		 * before it can run, so always set the EOT flag.
> >  		 */
> >  		flags |= DMA_PREP_LOAD_EOT;
> > +		nents = i;
> > +
> >  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> >  							 vecs, nents, dma_dir,
> >  							 flags);
> > 
> > ---
> > base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> > change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> > --
> > 
> > Thanks!
> > - Nuno Sá
> > 
> 

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Jonathan Cameron]

On Wed, 19 Aug 2026 11:16:22 +0100
Nuno Sá <nuno.sa@analog.com> wrote:

> On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> > On Tue, 18 Aug 2026 17:45:29 +0100
> > Nuno Sá <nuno.sa@analog.com> wrote:
> >   
> > > From: Michael Hennerich <michael.hennerich@analog.com>
> > > 
> > > iio_dmaengine_buffer_submit_block() counts scatterlist entries with
> > > sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
> > > then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
> > > After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
> > > the mapping so that only the first sgt->nents entries carry valid DMA
> > > addresses, with nents < orig_nents.
> > > 
> > > On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
> > > system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
> > > count is 2, so the loop reads one entry past the mapped set and emits a
> > > garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
> > > vec array (prep returns NULL), the fence is signalled with -ENOMEM,
> > > which a userspace poller cannot observe, and the block is left in
> > > ACTIVE state so every further enqueue of it fails with -EBUSY. The
> > > visible symptom is a stream of zero-filled blocks followed by a wedged
> > > buffer.
> > > 
> > > Platforms without an IOMMU never hit this because nents == orig_nents.
> > > 
> > > Size the vec array with sg_nents_for_dma(), which walks the DMA-mapped
> > > view and accounts for max-length splitting, and stop the fill loop once
> > > bytes_used is covered - which is allowed to be smaller than the block
> > > size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> > > 
> > > Assisted-by: Claude:claude-fable-5
> > > Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
> > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>  
> > 
> > There are some gremlins nearby in this code...
> > In the else just of this context seems max_size is computed again
> > having been done just above the code seen here.
> > 
> > Unless I'm missing something that should be cleaned up as well.  
> 
> Oh yes! Something I already noticed a couple of times but never sent the
> patch right away so I kept forgetting about it.
> > 
> > Been a while since I got my head into the scatterlist
> > stuff, so I might have it wrong below, but I don't think what
> > you have here actually works if the merging of entries is
> > larger than the max dma entry the hardware supports.
> >   
> 
> In theory might not be a bug but I'm not sure it's a claim we can fully
> take as guarantee.
> 
> >   
> > > ---
> > > Note the Signed-off-by is just because I'm carrying Michael's patch!
> > > ---
> > >  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > index ecc02a427b92..bece45381c8c 100644
> > > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > >  	if (block->sg_table) {
> > >  		unsigned long flags;
> > >  
> > > +		/*
> > > +		 * Use the DMA-mapped view of the sg_table: after mapping
> > > +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> > > +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> > > +		 * are only valid for the first sgt->nents entries. Counting
> > > +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> > > +		 * hands garbage vecs to the DMA engine.  
> > 
> > This feels like too much info after the fix is in place.  Talking about other
> > stuff that would be wrong is rather unusual.
> >  
> 
> Oh well, I guess LLM over commenting as usual.
> 
> > > +		 */
> > >  		sgl = block->sg_table->sgl;
> > > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > > -		if (nents < 0)
> > > -			return nents;
> > > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);  
> > 
> > So this fun function will generally give us the number of sgl entries, but not
> > quite always. It will give us how many chunks of up to max_size fit into
> > a particularly large entry.
> >   
> > >  
> > >  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
> > >  		if (!vecs)
> > > @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > >  
> > >  		len_total = block->bytes_used;
> > >  
> > > -		for (i = 0; i < nents; i++) {
> > > +		for (i = 0; i < nents && len_total; i++) {  
> > So this needs to be more clever as we aren't just iterating entrees and filling
> > them in, some of them could at least in theory be too big to fit
> > in a single vec - hence you need to do a loop in here that sets
> > multiple entries if that occurs.  
> 
> I see! But I think that merging means that we end up with more nents
> (sg_nents_for_len()) entries than DMA ones which was the issue we had
> because we were left with vecs with invalid addresses and 0 sized.

You can end up with a smaller set of DMA mappings than the CPU ones that
were typically pushed in.  That covers what happens in the IOMMU.
However those can be 'big' and so your DMA engines own ability to handle
a particular size of contiguous block may be the problem and result in
you needing more entrees on the hardware side of things.

> The other
> way around should not happen on the IOMMU path at least given the assumption
> that a single sg  entry must not be bigger than max_len [1].
Hmm - do I understand this right - is the question: If you original CPU side
mappings (the input scatter gather list entries) are less than max_len bytes each
then can we end up with a large set of entries for the DMA Engine after
the merge then split?  If so I think I think the answer is no (as long
as no other constraints like alignment make a mess for us - I gave
up trying to reason those out on a bit of paper).

> I think in theory
> that can actually happen (if we end using a dma provider defaulting to 64K and 
> cma dma_bufs) but maybe we can argue that's not our problem? Given
> the assumptions of course.
> 
> But even if we somehow reach this path with sg_dma_address > max_size
> and end up stuffing all of it in say, vec[0], the DMA controller should
> also care to make sure it splits each vec if it exceeds the descriptor
> max size. That's what both users of (ADI axi_dmac being one them)
> .device_prep_peripheral_dma_vec() are doing today. But yes, I agree that
> might be a bold assumption.

Then why we are we passing in max_size to the function to work out
how many entrees?  If splits are handled elsewhere we don't need that
limit.

> 
> > 
> > If that can't happen for some other reason then I think you can 
> > just use block->sgtable->nents instead of the more complex call above.
> >   
> 
> I was the one suggesting the other call because it seemed what we
> actually wanted but I'm fine with just using the above. I'm more tempted
> to keep it simple and use 'block->sgtable->nents' rather than jumping in
> a more complex subloop without clear evidence we really need it.
> Thoughts?

If they are split below this layer then indeed we don't need the call
and can use the number DMA entries.

> 
> As a fun side note this also made visible another subtle issue. Given
> that submitting a block might only happen when enabling the buffer (so
> async to enqueueing it) when we hit this issue, we get -ENOMEM and
> and go ahead  to wake up poll(). But AFAIK, we can't really propagate
> the error code up to userspace so userspace wakes up just to get
> garbage and thinking everything is fine. Easy way out would be to treat
> this as and hard error and return error when enabling the buffer. But
> that also raises the question that other blocks might have been properly
> submitted though. Not really sure how to handle this but anyways a problem
> for another day :)

Hmm. It is always tricky to cleanly surface errors when we get a problem
somewhere random in DMA.  No idea off the top of my head on how to solve
this case.

Jonathan

> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ab2cbeb0ed301a9f0460078e91b09f39958212ef
> 
> - Nuno Sá
> 
> > >  			vecs[i].addr = sg_dma_address(sgl);
> > >  			vecs[i].len = min(sg_dma_len(sgl), len_total);
> > >  			len_total -= vecs[i].len;
> > > @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > >  		 * before it can run, so always set the EOT flag.
> > >  		 */
> > >  		flags |= DMA_PREP_LOAD_EOT;
> > > +		nents = i;
> > > +
> > >  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> > >  							 vecs, nents, dma_dir,
> > >  							 flags);
> > > 
> > > ---
> > > base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> > > change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> > > --
> > > 
> > > Thanks!
> > > - Nuno Sá
> > >   
> >   

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Nuno Sá]

On Sun, Aug 23, 2026 at 01:16:18AM +0100, Jonathan Cameron wrote:
> On Wed, 19 Aug 2026 11:16:22 +0100
> Nuno Sá <nuno.sa@analog.com> wrote:
> 
> > On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> > > On Tue, 18 Aug 2026 17:45:29 +0100
> > > Nuno Sá <nuno.sa@analog.com> wrote:
> > >   
> > > > From: Michael Hennerich <michael.hennerich@analog.com>
> > > > 
> > > > iio_dmaengine_buffer_submit_block() counts scatterlist entries with
> > > > sg_nents_for_len(), which walks the CPU-side lengths (sg->length), but
> > > > then consumes the DMA-side fields (sg_dma_address()/sg_dma_len()).
> > > > After dma_map_sgtable() the two views may differ: an IOMMU can coalesce
> > > > the mapping so that only the first sgt->nents entries carry valid DMA
> > > > addresses, with nents < orig_nents.
> > > > 
> > > > On x86 with an IOMMU enabled, a DMABUF block backed by two 1 MiB
> > > > system-heap chunks maps to a single 2 MiB IOVA range. The CPU-side
> > > > count is 2, so the loop reads one entry past the mapped set and emits a
> > > > garbage vec ({addr = ~0, len = 0}). The DMA engine driver rejects the
> > > > vec array (prep returns NULL), the fence is signalled with -ENOMEM,
> > > > which a userspace poller cannot observe, and the block is left in
> > > > ACTIVE state so every further enqueue of it fails with -EBUSY. The
> > > > visible symptom is a stream of zero-filled blocks followed by a wedged
> > > > buffer.
> > > > 
> > > > Platforms without an IOMMU never hit this because nents == orig_nents.
> > > > 
> > > > Size the vec array with sg_nents_for_dma(), which walks the DMA-mapped
> > > > view and accounts for max-length splitting, and stop the fill loop once
> > > > bytes_used is covered - which is allowed to be smaller than the block
> > > > size - passing the reduced count to dmaengine_prep_peripheral_dma_vec().
> > > > 
> > > > Assisted-by: Claude:claude-fable-5
> > > > Fixes: 7a86d469983a ("iio: buffer-dmaengine: Support new DMABUF based userspace API")
> > > > Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>  
> > > 
> > > There are some gremlins nearby in this code...
> > > In the else just of this context seems max_size is computed again
> > > having been done just above the code seen here.
> > > 
> > > Unless I'm missing something that should be cleaned up as well.  
> > 
> > Oh yes! Something I already noticed a couple of times but never sent the
> > patch right away so I kept forgetting about it.
> > > 
> > > Been a while since I got my head into the scatterlist
> > > stuff, so I might have it wrong below, but I don't think what
> > > you have here actually works if the merging of entries is
> > > larger than the max dma entry the hardware supports.
> > >   
> > 
> > In theory might not be a bug but I'm not sure it's a claim we can fully
> > take as guarantee.
> > 
> > >   
> > > > ---
> > > > Note the Signed-off-by is just because I'm carrying Michael's patch!
> > > > ---
> > > >  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 16 ++++++++++++----
> > > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > index ecc02a427b92..bece45381c8c 100644
> > > > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> > > > @@ -104,10 +104,16 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > > >  	if (block->sg_table) {
> > > >  		unsigned long flags;
> > > >  
> > > > +		/*
> > > > +		 * Use the DMA-mapped view of the sg_table: after mapping
> > > > +		 * (e.g. through an IOMMU) the DMA entries (sgt->nents) can be
> > > > +		 * fewer than the CPU entries, and sg_dma_address()/sg_dma_len()
> > > > +		 * are only valid for the first sgt->nents entries. Counting
> > > > +		 * with sg_nents_for_len() (CPU lengths) walks past them and
> > > > +		 * hands garbage vecs to the DMA engine.  
> > > 
> > > This feels like too much info after the fix is in place.  Talking about other
> > > stuff that would be wrong is rather unusual.
> > >  
> > 
> > Oh well, I guess LLM over commenting as usual.
> > 
> > > > +		 */
> > > >  		sgl = block->sg_table->sgl;
> > > > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > > > -		if (nents < 0)
> > > > -			return nents;
> > > > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);  
> > > 
> > > So this fun function will generally give us the number of sgl entries, but not
> > > quite always. It will give us how many chunks of up to max_size fit into
> > > a particularly large entry.
> > >   
> > > >  
> > > >  		vecs = kmalloc_array(nents, sizeof(*vecs), GFP_ATOMIC);
> > > >  		if (!vecs)
> > > > @@ -115,7 +121,7 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > > >  
> > > >  		len_total = block->bytes_used;
> > > >  
> > > > -		for (i = 0; i < nents; i++) {
> > > > +		for (i = 0; i < nents && len_total; i++) {  
> > > So this needs to be more clever as we aren't just iterating entrees and filling
> > > them in, some of them could at least in theory be too big to fit
> > > in a single vec - hence you need to do a loop in here that sets
> > > multiple entries if that occurs.  
> > 
> > I see! But I think that merging means that we end up with more nents
> > (sg_nents_for_len()) entries than DMA ones which was the issue we had
> > because we were left with vecs with invalid addresses and 0 sized.
> 
> You can end up with a smaller set of DMA mappings than the CPU ones that
> were typically pushed in.  That covers what happens in the IOMMU.
> However those can be 'big' and so your DMA engines own ability to handle
> a particular size of contiguous block may be the problem and result in
> you needing more entrees on the hardware side of things.
> 
> > The other
> > way around should not happen on the IOMMU path at least given the assumption
> > that a single sg  entry must not be bigger than max_len [1].
> Hmm - do I understand this right - is the question: If you original CPU side
> mappings (the input scatter gather list entries) are less than max_len bytes each

That seems to be the assumption taken in the IOMMU merge path.
And it seems that there's even some debug configs we can use that
will trigger some warning about this [1].

> then can we end up with a large set of entries for the DMA Engine after
> the merge then split?  If so I think I think the answer is no (as long
> as no other constraints like alignment make a mess for us - I gave
> up trying to reason those out on a bit of paper).
> 
> > I think in theory
> > that can actually happen (if we end using a dma provider defaulting to 64K and 
> > cma dma_bufs) but maybe we can argue that's not our problem? Given
> > the assumptions of course.
> > 
> > But even if we somehow reach this path with sg_dma_address > max_size
> > and end up stuffing all of it in say, vec[0], the DMA controller should
> > also care to make sure it splits each vec if it exceeds the descriptor
> > max size. That's what both users of (ADI axi_dmac being one them)
> > .device_prep_peripheral_dma_vec() are doing today. But yes, I agree that
> > might be a bold assumption.
> 
> Then why we are we passing in max_size to the function to work out
> how many entrees?  If splits are handled elsewhere we don't need that
> limit.
> 

I guess it was what made sense to pass to sg_nents_for_dma() at this
level.

> > 
> > > 
> > > If that can't happen for some other reason then I think you can 
> > > just use block->sgtable->nents instead of the more complex call above.
> > >   
> > 
> > I was the one suggesting the other call because it seemed what we
> > actually wanted but I'm fine with just using the above. I'm more tempted
> > to keep it simple and use 'block->sgtable->nents' rather than jumping in
> > a more complex subloop without clear evidence we really need it.
> > Thoughts?
> 
> If they are split below this layer then indeed we don't need the call
> and can use the number DMA entries.

I want to reinforce that's what happening on both controllers
implementing the vector based callback. But I do think it's a fair
assumption given that there's no way we can actually know the
channel/descriptor size a dma controller can handle. AFAIK, it's not
exported anywhere. At this level we just know about the segment max_ size a
mapping can have.

If we also look at all users of sg_nents_for_dma(), we only see dma
controllers drivers using it (with the len argument being precisely what
looks like a descriptor max size). So yes, I'm more convinced this layer
is the wrong place to use such an helper and I'll just use number of
entries we got from the mappings.

[1]: https://elixir.bootlin.com/linux/v7.2/source/kernel/dma/debug.c#L1230
- Nuno Sá

> 
> > 
> > As a fun side note this also made visible another subtle issue. Given
> > that submitting a block might only happen when enabling the buffer (so
> > async to enqueueing it) when we hit this issue, we get -ENOMEM and
> > and go ahead  to wake up poll(). But AFAIK, we can't really propagate
> > the error code up to userspace so userspace wakes up just to get
> > garbage and thinking everything is fine. Easy way out would be to treat
> > this as and hard error and return error when enabling the buffer. But
> > that also raises the question that other blocks might have been properly
> > submitted though. Not really sure how to handle this but anyways a problem
> > for another day :)
> 
> Hmm. It is always tricky to cleanly surface errors when we get a problem
> somewhere random in DMA.  No idea off the top of my head on how to solve
> this case.
> 
> Jonathan
> 
> > 
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ab2cbeb0ed301a9f0460078e91b09f39958212ef
> > 
> > - Nuno Sá
> > 
> > > >  			vecs[i].addr = sg_dma_address(sgl);
> > > >  			vecs[i].len = min(sg_dma_len(sgl), len_total);
> > > >  			len_total -= vecs[i].len;
> > > > @@ -133,6 +139,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
> > > >  		 * before it can run, so always set the EOT flag.
> > > >  		 */
> > > >  		flags |= DMA_PREP_LOAD_EOT;
> > > > +		nents = i;
> > > > +
> > > >  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
> > > >  							 vecs, nents, dma_dir,
> > > >  							 flags);
> > > > 
> > > > ---
> > > > base-commit: b756b143e5391151e577ae645b1378a43f93c2f5
> > > > change-id: 20260818-iio-buffer-dmabuf-iommu-fic-1b281f15e5a4
> > > > --
> > > > 
> > > > Thanks!
> > > > - Nuno Sá
> > > >   
> > >   
> 

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Andy Shevchenko]

On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> On Tue, 18 Aug 2026 17:45:29 +0100
> Nuno Sá <nuno.sa@analog.com> wrote:

...

> >  		sgl = block->sg_table->sgl;
> > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > -		if (nents < 0)
> > -			return nents;
> > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);
> 
> So this fun function will generally give us the number of sgl entries, but not
> quite always. It will give us how many chunks of up to max_size fit into
> a particularly large entry.

It gives the number of SG entries needed for the case if each of them will
satisfy the limit. Whatever following code does, it may allocate a new SG
list based on the number returned by this function and resplit.

> > -		for (i = 0; i < nents; i++) {
> > +		for (i = 0; i < nents && len_total; i++) {
> So this needs to be more clever as we aren't just iterating entrees and filling
> them in, some of them could at least in theory be too big to fit
> in a single vec - hence you need to do a loop in here that sets
> multiple entries if that occurs.
> 
> If that can't happen for some other reason then I think you can 
> just use block->sgtable->nents instead of the more complex call above.

Perhaps sg_split() is what people are looking for in this case?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: buffer-dmaengine: fix sg entry iteration when building dma_vecs

[Jonathan Cameron]

On Wed, 19 Aug 2026 23:54:31 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Aug 19, 2026 at 01:41:04AM +0100, Jonathan Cameron wrote:
> > On Tue, 18 Aug 2026 17:45:29 +0100
> > Nuno Sá <nuno.sa@analog.com> wrote:  
> 
> ...
> 
> > >  		sgl = block->sg_table->sgl;
> > > -		nents = sg_nents_for_len(sgl, block->bytes_used);
> > > -		if (nents < 0)
> > > -			return nents;
> > > +		nents = sg_nents_for_dma(sgl, block->sg_table->nents, max_size);  
> > 
> > So this fun function will generally give us the number of sgl entries, but not
> > quite always. It will give us how many chunks of up to max_size fit into
> > a particularly large entry.  
> 
> It gives the number of SG entries needed for the case if each of them will
> satisfy the limit. Whatever following code does, it may allocate a new SG
> list based on the number returned by this function and resplit.

Yup. That's what would be needed if the condition can actually occur.
Also, Nuno is (I think) suggesting the dma engine driver itself
deals with that splitting when it is needed.  So hopefully this
is a place where we can apply the someone else's problem field ;)

> 
> > > -		for (i = 0; i < nents; i++) {
> > > +		for (i = 0; i < nents && len_total; i++) {  
> > So this needs to be more clever as we aren't just iterating entrees and filling
> > them in, some of them could at least in theory be too big to fit
> > in a single vec - hence you need to do a loop in here that sets
> > multiple entries if that occurs.
> > 
> > If that can't happen for some other reason then I think you can 
> > just use block->sgtable->nents instead of the more complex call above.  
> 
> Perhaps sg_split() is what people are looking for in this case?

I'd forgotten that nugget of fun existed. There is a patch on list
to drop the one driver I ever used it in - bringing the total users
down to 4! :) I don't think it applies here though as we are talking
splitting one entry of the list, not the whole list.
That list splitting is for when you have a constraint on the total max DMA
done in one request. 

Jonathan


>