[PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Alexandru Ardelean]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Sometimes the user needs to split each entry on the mapped scatter list
due to DMA length constrains. This helper returns a number of entities
assuming that each of them is not bigger than supplied maximum length.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Patch was sent in 2016 initially to the DMA engine sub-system.
Link:
  https://patchwork.kernel.org/patch/9389821/
This was part of a larger series:
  https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*

I'm not sure if this is supposed to go into DMAEngine or lib/scatterlist.
It doesn't look like lib/scatterlist is managed by DMAEngine, so (by using
the `get_maintainers.pl` script) I'm sending this patch to this group of
parties.

Thanks
Alex

 include/linux/scatterlist.h |  1 +
 lib/scatterlist.c           | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index b96f0d0b5b8f..4f40455c40e2 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -253,6 +253,7 @@ static inline void sg_init_marker(struct scatterlist *sgl,
 
 int sg_nents(struct scatterlist *sg);
 int sg_nents_for_len(struct scatterlist *sg, u64 len);
+int sg_nents_for_dma(struct scatterlist *sgl, unsigned int sglen, size_t len);
 struct scatterlist *sg_next(struct scatterlist *);
 struct scatterlist *sg_last(struct scatterlist *s, unsigned int);
 void sg_init_table(struct scatterlist *, unsigned int);
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 9ba349e775ef..5a6fc32f485d 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -86,6 +86,32 @@ int sg_nents_for_len(struct scatterlist *sg, u64 len)
 }
 EXPORT_SYMBOL(sg_nents_for_len);
 
+/**
+ * sg_nents_for_dma - return count of DMA-capable entries in scatterlist
+ * @sgl:	The scatterlist
+ * @sglen:	The current number of entries
+ * @len:	The maximum length of DMA-capable block
+ *
+ * Description:
+ *   Determines the number of entries in @sgl which would be permitted in
+ *   DMA-capable transfer if list had been split accordingly, taking into
+ *   account chaining as well.
+ *
+ * Returns:
+ *   the number of sgl entries needed
+ *
+ **/
+int sg_nents_for_dma(struct scatterlist *sgl, unsigned int sglen, size_t len)
+{
+	struct scatterlist *sg;
+	int i, nents = 0;
+
+	for_each_sg(sgl, sg, sglen, i)
+		nents += DIV_ROUND_UP(sg_dma_len(sg), len);
+	return nents;
+}
+EXPORT_SYMBOL(sg_nents_for_dma);
+
 /**
  * sg_last - return the last scatterlist entry in a list
  * @sgl:	First entry in the scatterlist
-- 
2.17.1

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Andy Shevchenko]

On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Sometimes the user needs to split each entry on the mapped scatter list
> due to DMA length constrains. This helper returns a number of entities
> assuming that each of them is not bigger than supplied maximum length.
>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Hmm... Usually we don't accept generic API without users.
Do you have any use case in mind?

> Patch was sent in 2016 initially to the DMA engine sub-system.
> Link:
>   https://patchwork.kernel.org/patch/9389821/
> This was part of a larger series:
>   https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
>
> I'm not sure if this is supposed to go into DMAEngine or lib/scatterlist.
> It doesn't look like lib/scatterlist is managed by DMAEngine, so (by using
> the `get_maintainers.pl` script) I'm sending this patch to this group of
> parties.

The problem the patch tried to solve is much deeper and correct
solution should be more generic, i.e.
each channel should provide a set of parameters, such as DMA segment
size, to the users (via DMA engine API) and users should prepare the
SG list according to the limits of the channel.
In that case we don't need to re-split/re-allocate given SG list at
all, which would simplify DMA slave drivers and their users.

We discussed this topic back in 2016 with Vinod on LPC, but seems it's
not critical to solve. My case is to improve DMA performance for 8250
UART.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Andy Shevchenko]

+Cc Vinod

On Wed, Feb 27, 2019 at 11:45 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
> <alexandru.ardelean@analog.com> wrote:
> >
> > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> > Sometimes the user needs to split each entry on the mapped scatter list
> > due to DMA length constrains. This helper returns a number of entities
> > assuming that each of them is not bigger than supplied maximum length.
> >
> > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Hmm... Usually we don't accept generic API without users.
> Do you have any use case in mind?
>
> > Patch was sent in 2016 initially to the DMA engine sub-system.
> > Link:
> >   https://patchwork.kernel.org/patch/9389821/
> > This was part of a larger series:
> >   https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
> >
> > I'm not sure if this is supposed to go into DMAEngine or lib/scatterlist.
> > It doesn't look like lib/scatterlist is managed by DMAEngine, so (by using
> > the `get_maintainers.pl` script) I'm sending this patch to this group of
> > parties.
>
> The problem the patch tried to solve is much deeper and correct
> solution should be more generic, i.e.
> each channel should provide a set of parameters, such as DMA segment
> size, to the users (via DMA engine API) and users should prepare the
> SG list according to the limits of the channel.
> In that case we don't need to re-split/re-allocate given SG list at
> all, which would simplify DMA slave drivers and their users.
>
> We discussed this topic back in 2016 with Vinod on LPC, but seems it's
> not critical to solve. My case is to improve DMA performance for 8250
> UART.
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Ardelean, Alexandru]

On Wed, 2019-02-27 at 11:46 +0200, Andy Shevchenko wrote:
> [External]
> 
> 
> +Cc Vinod
> 
> On Wed, Feb 27, 2019 at 11:45 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > 
> > On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
> > <alexandru.ardelean@analog.com> wrote:
> > > 
> > > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > 
> > > Sometimes the user needs to split each entry on the mapped scatter
> > > list
> > > due to DMA length constrains. This helper returns a number of
> > > entities
> > > assuming that each of them is not bigger than supplied maximum
> > > length.
> > > 
> > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > 
> > Hmm... Usually we don't accept generic API without users.
> > Do you have any use case in mind?

Yep: this one:
https://patchwork.kernel.org/patch/10814527/

But, I can rework this patch to work without the helper.

> > 
> > > Patch was sent in 2016 initially to the DMA engine sub-system.
> > > Link:
> > >   https://patchwork.kernel.org/patch/9389821/
> > > This was part of a larger series:
> > >   
> > > https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
> > > 
> > > I'm not sure if this is supposed to go into DMAEngine or
> > > lib/scatterlist.
> > > It doesn't look like lib/scatterlist is managed by DMAEngine, so (by
> > > using
> > > the `get_maintainers.pl` script) I'm sending this patch to this group
> > > of
> > > parties.
> > 
> > The problem the patch tried to solve is much deeper and correct
> > solution should be more generic, i.e.
> > each channel should provide a set of parameters, such as DMA segment
> > size, to the users (via DMA engine API) and users should prepare the
> > SG list according to the limits of the channel.
> > In that case we don't need to re-split/re-allocate given SG list at
> > all, which would simplify DMA slave drivers and their users.
> > 

I don't think I managed to follow [or find] the full length of that
discussion. Or, the conclusion wasn't that obvious to me, from what I
found.
I assumed this may have been dropped/forgotten.

In any case, I am fine with just reworking.

> > We discussed this topic back in 2016 with Vinod on LPC, but seems it's
> > not critical to solve. My case is to improve DMA performance for 8250
> > UART.
> > 
> > --
> > With Best Regards,
> > Andy Shevchenko
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Andy Shevchenko]

On Wed, Feb 27, 2019 at 2:26 PM Ardelean, Alexandru
<Alex.Ardelean@analog.com> wrote:
>
> On Wed, 2019-02-27 at 11:46 +0200, Andy Shevchenko wrote:
> > [External]
> >
> >
> > +Cc Vinod
> >
> > On Wed, Feb 27, 2019 at 11:45 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
> > > <alexandru.ardelean@analog.com> wrote:
> > > >
> > > > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > >
> > > > Sometimes the user needs to split each entry on the mapped scatter
> > > > list
> > > > due to DMA length constrains. This helper returns a number of
> > > > entities
> > > > assuming that each of them is not bigger than supplied maximum
> > > > length.
> > > >
> > > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > >
> > > Hmm... Usually we don't accept generic API without users.
> > > Do you have any use case in mind?
>
> Yep: this one:
> https://patchwork.kernel.org/patch/10814527/
>
> But, I can rework this patch to work without the helper.

Ah, okay, worth to mention this in comments (in addition to what you
put about the patch below).

> > > > Patch was sent in 2016 initially to the DMA engine sub-system.
> > > > Link:
> > > >   https://patchwork.kernel.org/patch/9389821/
> > > > This was part of a larger series:
> > > >
> > > > https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
> > > >
> > > > I'm not sure if this is supposed to go into DMAEngine or
> > > > lib/scatterlist.
> > > > It doesn't look like lib/scatterlist is managed by DMAEngine, so (by
> > > > using
> > > > the `get_maintainers.pl` script) I'm sending this patch to this group
> > > > of
> > > > parties.
> > >
> > > The problem the patch tried to solve is much deeper and correct
> > > solution should be more generic, i.e.
> > > each channel should provide a set of parameters, such as DMA segment
> > > size, to the users (via DMA engine API) and users should prepare the
> > > SG list according to the limits of the channel.
> > > In that case we don't need to re-split/re-allocate given SG list at
> > > all, which would simplify DMA slave drivers and their users.
> > >
>
> I don't think I managed to follow [or find] the full length of that
> discussion. Or, the conclusion wasn't that obvious to me, from what I
> found.
> I assumed this may have been dropped/forgotten.
>
> In any case, I am fine with just reworking.

It's fine to apply it, my point that it help to solve a symptom in a
short-term (however this short-term can be easily a long one due to no
guarantee that all drivers using SG for DMA will be converted).
So, please, update the comments and resubmit as a new version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Ardelean, Alexandru]

On Wed, 2019-02-27 at 16:15 +0200, Andy Shevchenko wrote:
> [External]
> 
> 
> On Wed, Feb 27, 2019 at 2:26 PM Ardelean, Alexandru
> <Alex.Ardelean@analog.com> wrote:
> > 
> > On Wed, 2019-02-27 at 11:46 +0200, Andy Shevchenko wrote:
> > > [External]
> > > 
> > > 
> > > +Cc Vinod
> > > 
> > > On Wed, Feb 27, 2019 at 11:45 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > 
> > > > On Wed, Feb 27, 2019 at 10:51 AM Alexandru Ardelean
> > > > <alexandru.ardelean@analog.com> wrote:
> > > > > 
> > > > > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > 
> > > > > Sometimes the user needs to split each entry on the mapped
> > > > > scatter
> > > > > list
> > > > > due to DMA length constrains. This helper returns a number of
> > > > > entities
> > > > > assuming that each of them is not bigger than supplied maximum
> > > > > length.
> > > > > 
> > > > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > > 
> > > > Hmm... Usually we don't accept generic API without users.
> > > > Do you have any use case in mind?
> > 
> > Yep: this one:
> > https://patchwork.kernel.org/patch/10814527/
> > 
> > But, I can rework this patch to work without the helper.
> 
> Ah, okay, worth to mention this in comments (in addition to what you
> put about the patch below).
> 
> > > > > Patch was sent in 2016 initially to the DMA engine sub-system.
> > > > > Link:
> > > > >   https://patchwork.kernel.org/patch/9389821/
> > > > > This was part of a larger series:
> > > > > 
> > > > > 
https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
> > > > > 
> > > > > I'm not sure if this is supposed to go into DMAEngine or
> > > > > lib/scatterlist.
> > > > > It doesn't look like lib/scatterlist is managed by DMAEngine, so
> > > > > (by
> > > > > using
> > > > > the `get_maintainers.pl` script) I'm sending this patch to this
> > > > > group
> > > > > of
> > > > > parties.
> > > > 
> > > > The problem the patch tried to solve is much deeper and correct
> > > > solution should be more generic, i.e.
> > > > each channel should provide a set of parameters, such as DMA
> > > > segment
> > > > size, to the users (via DMA engine API) and users should prepare
> > > > the
> > > > SG list according to the limits of the channel.
> > > > In that case we don't need to re-split/re-allocate given SG list at
> > > > all, which would simplify DMA slave drivers and their users.
> > > > 
> > 
> > I don't think I managed to follow [or find] the full length of that
> > discussion. Or, the conclusion wasn't that obvious to me, from what I
> > found.
> > I assumed this may have been dropped/forgotten.
> > 
> > In any case, I am fine with just reworking.
> 
> It's fine to apply it, my point that it help to solve a symptom in a
> short-term (however this short-term can be easily a long one due to no
> guarantee that all drivers using SG for DMA will be converted).
> So, please, update the comments and resubmit as a new version.
> 

There's so much uncertainty about this patch, that it makes it easier [and
probably makes more sense] to just rework the AXI DMAC patch to not use
this helper.

This is also in the hope that one day we will get to the more generic API
that you mentioned.

> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper

[Christoph Hellwig]

On Wed, Feb 27, 2019 at 04:15:37PM +0200, Andy Shevchenko wrote:
> > https://patchwork.kernel.org/patch/10814527/
> >
> > But, I can rework this patch to work without the helper.
> 
> Ah, okay, worth to mention this in comments (in addition to what you
> put about the patch below).

Just include the patch adding the helper in the series.