Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1352898 > unrolled thread

[PATCH 0/7] mtd: nand: sunxi: add support for DMA operations

Started byBoris Brezillon <boris.brezillon@free-electrons.com>
First post2016-03-08 12:20 +0100
Last post2016-03-08 18:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/7] mtd: nand: sunxi: add support for DMA operations Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-03-08 12:20 +0100
    [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-03-08 12:20 +0100
      Re: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-03-08 18:00 +0100
        Re: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-03-08 18:00 +0100

#1352898 — [PATCH 0/7] mtd: nand: sunxi: add support for DMA operations

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-03-08 12:20 +0100
Subject[PATCH 0/7] mtd: nand: sunxi: add support for DMA operations
Message-ID<ranQd-5m-5@gated-at.bofh.it>
Hello,

This patch series aims at adding support for DMA assisted operations to
the sunxi_nand driver.

The first 3 patches are just reworks in the existing driver preparing
things for DMA ->read/write_page() operations. Those ones are mainly
re-arranging existing functions, and moving some code into dedicated
functions so we can reuse them when adding the read/write_page()
implementation.

Patch 4 is an attempt to generalize some logic that are duplicated in a
lot of places. It provides a generic solution to create an SG table
from a buffer (passed by virtual address) and its length.
This generic implementation tries to take all possible constraints into
account, like:
- vmallocated buffers
- alignement requirement/preference
- maximum DMA transfer length

I may have missed other things (is there a need for a minimum DMA
transfer constraint?), so don't hesitate to point problems or missing
elements in this implementation.
Note that other subsystems doing the same kind of thing (like SPI of V4L)
could use this implementation. This is why I've put the SPI and V4L
maintainers in Cc.

Patch 5 is providing functions to map/unmap buffers for DMA operations
at the MTD level. This will hopefully limit the number of open-coded
implementations we're currently seeing in a lot of NAND drivers.
Of course, it's making use of sg_alloc_table_from_buf(), introduced in
patch 4.

Patch 6 and 7 are patching the sunxi NAND driver and its DT binding doc
to add DMA support.

I'm particularly interested in getting feedbacks on patch 4 and 5.
Is there a reason nobody ever tried to create such generic functions
(at the scatterlist and MTD levels), and if there are, could you detail
them?

Thanks,

Boris

Side note: patches touching the sunxi NAND driver are depending on
a series posted yesterday [1].

[1]https://lkml.org/lkml/2016/3/7/444

Boris Brezillon (7):
  mtd: nand: sunxi: move some ECC related operations to their own
    functions
  mtd: nand: sunxi: make OOB retrieval optional
  mtd: nand: sunxi: make cur_off parameter optional in extra oob helpers
  scatterlist: add sg_alloc_table_from_buf() helper
  mtd: provide helper to prepare buffers for DMA operations
  mtd: nand: sunxi: add support for DMA assisted operations
  mtd: nand: sunxi: update DT bindings

 .../devicetree/bindings/mtd/sunxi-nand.txt         |   4 +
 drivers/mtd/mtdcore.c                              |  66 +++
 drivers/mtd/nand/sunxi_nand.c                      | 483 ++++++++++++++++++---
 include/linux/mtd/mtd.h                            |  25 ++
 include/linux/scatterlist.h                        |  24 +
 lib/scatterlist.c                                  | 142 ++++++
 6 files changed, 679 insertions(+), 65 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]


#1352899 — [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-03-08 12:20 +0100
Subject[PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper
Message-ID<ranQe-5m-31@gated-at.bofh.it>
In reply to#1352898
sg_alloc_table_from_buf() provides an easy solution to create an sg_table
from a virtual address pointer. This function takes care of dealing with
vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
DMA transfer size).

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 include/linux/scatterlist.h |  24 ++++++++
 lib/scatterlist.c           | 142 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 556ec1e..4a75362 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -41,6 +41,27 @@ struct sg_table {
 	unsigned int orig_nents;	/* original size of list */
 };
 
+/**
+ * struct sg_constraints - SG constraints structure
+ *
+ * @max_chunk_len: maximum chunk buffer length. Each SG entry has to be smaller
+ *		   than this value. Zero means no constraint.
+ * @required_alignment: minimum alignment. Is used for both size and pointer
+ *			alignment. If this constraint is not met, the function
+ *			should return -EINVAL.
+ * @preferred_alignment: preferred alignment. Mainly used to optimize
+ *			 throughput when the DMA engine performs better when
+ *			 doing aligned accesses.
+ *
+ * This structure is here to help sg_alloc_table_from_buf() create the optimal
+ * SG list based on DMA engine constraints.
+ */
+struct sg_constraints {
+	size_t max_chunk_len;
+	size_t required_alignment;
+	size_t preferred_alignment;
+};
+
 /*
  * Notes on SG table design.
  *
@@ -265,6 +286,9 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
 	struct page **pages, unsigned int n_pages,
 	unsigned long offset, unsigned long size,
 	gfp_t gfp_mask);
+int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t len,
+			    const struct sg_constraints *constraints,
+			    gfp_t gfp_mask);
 
 size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
 		      size_t buflen, off_t skip, bool to_buffer);
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index bafa993..706b583 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -433,6 +433,148 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
 }
 EXPORT_SYMBOL(sg_alloc_table_from_pages);
 
+static size_t sg_buf_chunk_len(const void *buf, size_t len,
+			       const struct sg_constraints *cons)
+{
+	size_t chunk_len = len;
+
+	if (cons->max_chunk_len)
+		chunk_len = min_t(size_t, chunk_len, cons->max_chunk_len);
+
+	if (is_vmalloc_addr(buf))
+		chunk_len = min_t(size_t, chunk_len,
+				  PAGE_SIZE - offset_in_page(buf));
+
+	if (!IS_ALIGNED((unsigned long)buf, cons->preferred_alignment)) {
+		const void *aligned_buf = PTR_ALIGN(buf,
+						    cons->preferred_alignment);
+		size_t unaligned_len = (unsigned long)(aligned_buf - buf);
+
+		chunk_len = min_t(size_t, chunk_len, unaligned_len);
+	} else if (chunk_len > cons->preferred_alignment) {
+		chunk_len &= ~(cons->preferred_alignment - 1);
+	}
+
+	return chunk_len;
+}
+
+#define sg_for_each_chunk_in_buf(buf, len, chunk_len, constraints)	\
+	for (chunk_len = sg_buf_chunk_len(buf, len, constraints);	\
+	     len;							\
+	     len -= chunk_len, buf += chunk_len,			\
+	     chunk_len = sg_buf_chunk_len(buf, len, constraints))
+
+static int sg_check_constraints(struct sg_constraints *cons,
+				const void *buf, size_t len)
+{
+	if (!cons->required_alignment)
+		cons->required_alignment = 1;
+
+	if (!cons->preferred_alignment)
+		cons->preferred_alignment = cons->required_alignment;
+
+	/* Test if buf and len are properly aligned. */
+	if (!IS_ALIGNED((unsigned long)buf, cons->required_alignment) ||
+	    !IS_ALIGNED(len, cons->required_alignment))
+		return -EINVAL;
+
+	/*
+	 * if the buffer has been vmallocated and required_alignment is
+	 * more than PAGE_SIZE we cannot guarantee it.
+	 */
+	if (is_vmalloc_addr(buf) && cons->required_alignment > PAGE_SIZE)
+		return -EINVAL;
+
+	/*
+	 * max_chunk_len has to be aligned to required_alignment to
+	 * guarantee that all buffer chunks are aligned correctly.
+	 */
+	if (!IS_ALIGNED(cons->max_chunk_len, cons->required_alignment))
+		return -EINVAL;
+
+	/*
+	 * preferred_alignment has to be aligned to required_alignment
+	 * to avoid misalignment of buffer chunks.
+	 */
+	if (!IS_ALIGNED(cons->preferred_alignment, cons->required_alignment))
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * sg_alloc_table_from_buf - create an SG table from a buffer
+ *
+ * @sgt: SG table
+ * @buf: buffer you want to create this SG table from
+ * @len: length of buf
+ * @constraints: optional constraints to take into account when creating
+ *		 the SG table. Can be NULL if no specific constraints are
+ *		 required.
+ * @gfp_mask: type of allocation to use when creating the table
+ *
+ * This function creates an SG table from a buffer, its length and some
+ * SG constraints.
+ *
+ * Note: This function supports vmallocated buffers.
+ */
+int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t len,
+			    const struct sg_constraints *constraints,
+			    gfp_t gfp_mask)
+{
+	struct sg_constraints cons = { };
+	size_t remaining, chunk_len;
+	const void *sg_buf;
+	int i, ret;
+
+	if (constraints)
+		cons = *constraints;
+
+	ret = sg_check_constraints(&cons, buf, len);
+	if (ret)
+		return ret;
+
+	sg_buf = buf;
+	remaining = len;
+	i = 0;
+	sg_for_each_chunk_in_buf(sg_buf, remaining, chunk_len, &cons)
+		i++;
+
+	ret = sg_alloc_table(sgt, i, gfp_mask);
+	if (ret)
+		return ret;
+
+	sg_buf = buf;
+	remaining = len;
+	i = 0;
+	sg_for_each_chunk_in_buf(sg_buf, remaining, chunk_len, &cons) {
+		if (is_vmalloc_addr(sg_buf)) {
+			struct page *vm_page;
+
+			vm_page = vmalloc_to_page(sg_buf);
+			if (!vm_page) {
+				ret = -ENOMEM;
+				goto err_free_table;
+			}
+
+			sg_set_page(&sgt->sgl[i], vm_page, chunk_len,
+				    offset_in_page(sg_buf));
+		} else {
+			sg_set_buf(&sgt->sgl[i], sg_buf, chunk_len);
+		}
+
+		i++;
+	}
+
+	return 0;
+
+err_free_table:
+	sg_free_table(sgt);
+
+	return ret;
+}
+EXPORT_SYMBOL(sg_alloc_table_from_buf);
+
 void __sg_page_iter_start(struct sg_page_iter *piter,
 			  struct scatterlist *sglist, unsigned int nents,
 			  unsigned long pgoffset)
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]


#1353212 — Re: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper

FromLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Date2016-03-08 18:00 +0100
SubjectRe: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper
Message-ID<rat9g-3s9-17@gated-at.bofh.it>
In reply to#1352899
Hi Boris,

Thank you for the patch.

On Tuesday 08 March 2016 12:15:12 Boris Brezillon wrote:
> sg_alloc_table_from_buf() provides an easy solution to create an sg_table
> from a virtual address pointer. This function takes care of dealing with
> vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
> DMA transfer size).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  include/linux/scatterlist.h |  24 ++++++++
>  lib/scatterlist.c           | 142 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 166 insertions(+)
> 
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 556ec1e..4a75362 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -41,6 +41,27 @@ struct sg_table {
>  	unsigned int orig_nents;	/* original size of list */
>  };
> 
> +/**
> + * struct sg_constraints - SG constraints structure
> + *
> + * @max_chunk_len: maximum chunk buffer length. Each SG entry has to be
> smaller
> + *		   than this value. Zero means no constraint.
> + * @required_alignment: minimum alignment. Is used for both size and
> pointer
> + *			alignment. If this constraint is not met, the function
> + *			should return -EINVAL.
> + * @preferred_alignment: preferred alignment. Mainly used to optimize
> + *			 throughput when the DMA engine performs better when
> + *			 doing aligned accesses.
> + *
> + * This structure is here to help sg_alloc_table_from_buf() create the
> optimal
> + * SG list based on DMA engine constraints.
> + */
> +struct sg_constraints {
> +	size_t max_chunk_len;
> +	size_t required_alignment;
> +	size_t preferred_alignment;
> +};
> +
>  /*
>   * Notes on SG table design.
>   *
> @@ -265,6 +286,9 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>  	struct page **pages, unsigned int n_pages,
>  	unsigned long offset, unsigned long size,
>  	gfp_t gfp_mask);
> +int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t
> len,
> +			    const struct sg_constraints *constraints,
> +			    gfp_t gfp_mask);
> 
>  size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void
> *buf, size_t buflen, off_t skip, bool to_buffer);
> diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> index bafa993..706b583 100644
> --- a/lib/scatterlist.c
> +++ b/lib/scatterlist.c
> @@ -433,6 +433,148 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>  }
>  EXPORT_SYMBOL(sg_alloc_table_from_pages);
> 
> +static size_t sg_buf_chunk_len(const void *buf, size_t len,
> +			       const struct sg_constraints *cons)
> +{
> +	size_t chunk_len = len;
> +
> +	if (cons->max_chunk_len)
> +		chunk_len = min_t(size_t, chunk_len, cons->max_chunk_len);
> +
> +	if (is_vmalloc_addr(buf))
> +		chunk_len = min_t(size_t, chunk_len,
> +				  PAGE_SIZE - offset_in_page(buf));

This will lead to page-sized scatter-gather entries even for pages of the 
vmalloc memory that happen to be physically contiguous. That works, but I 
wonder whether we'd want to be more efficient.

> +	if (!IS_ALIGNED((unsigned long)buf, cons->preferred_alignment)) {
> +		const void *aligned_buf = PTR_ALIGN(buf,
> +						    cons->preferred_alignment);
> +		size_t unaligned_len = (unsigned long)(aligned_buf - buf);
> +
> +		chunk_len = min_t(size_t, chunk_len, unaligned_len);
> +	} else if (chunk_len > cons->preferred_alignment) {
> +		chunk_len &= ~(cons->preferred_alignment - 1);
> +	}
> +
> +	return chunk_len;
> +}
> +
> +#define sg_for_each_chunk_in_buf(buf, len, chunk_len, constraints)	\
> +	for (chunk_len = sg_buf_chunk_len(buf, len, constraints);	\
> +	     len;							\
> +	     len -= chunk_len, buf += chunk_len,			\
> +	     chunk_len = sg_buf_chunk_len(buf, len, constraints))
> +
> +static int sg_check_constraints(struct sg_constraints *cons,
> +				const void *buf, size_t len)
> +{
> +	if (!cons->required_alignment)
> +		cons->required_alignment = 1;
> +
> +	if (!cons->preferred_alignment)
> +		cons->preferred_alignment = cons->required_alignment;
> +
> +	/* Test if buf and len are properly aligned. */
> +	if (!IS_ALIGNED((unsigned long)buf, cons->required_alignment) ||
> +	    !IS_ALIGNED(len, cons->required_alignment))
> +		return -EINVAL;
> +
> +	/*
> +	 * if the buffer has been vmallocated and required_alignment is
> +	 * more than PAGE_SIZE we cannot guarantee it.
> +	 */
> +	if (is_vmalloc_addr(buf) && cons->required_alignment > PAGE_SIZE)
> +		return -EINVAL;
> +
> +	/*
> +	 * max_chunk_len has to be aligned to required_alignment to
> +	 * guarantee that all buffer chunks are aligned correctly.
> +	 */
> +	if (!IS_ALIGNED(cons->max_chunk_len, cons->required_alignment))
> +		return -EINVAL;
> +
> +	/*
> +	 * preferred_alignment has to be aligned to required_alignment
> +	 * to avoid misalignment of buffer chunks.
> +	 */
> +	if (!IS_ALIGNED(cons->preferred_alignment, cons->required_alignment))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/**
> + * sg_alloc_table_from_buf - create an SG table from a buffer
> + *
> + * @sgt: SG table
> + * @buf: buffer you want to create this SG table from
> + * @len: length of buf
> + * @constraints: optional constraints to take into account when creating
> + *		 the SG table. Can be NULL if no specific constraints are
> + *		 required.
> + * @gfp_mask: type of allocation to use when creating the table
> + *
> + * This function creates an SG table from a buffer, its length and some
> + * SG constraints.
> + *
> + * Note: This function supports vmallocated buffers.

What other types of memory does it support ? kmalloc() quite obviously, are 
there others ? I think you should explicitly list the memory types that the 
function intends to support.

> + */
> +int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t
> len,
> +			    const struct sg_constraints *constraints,
> +			    gfp_t gfp_mask)
> +{
> +	struct sg_constraints cons = { };
> +	size_t remaining, chunk_len;
> +	const void *sg_buf;
> +	int i, ret;
> +
> +	if (constraints)
> +		cons = *constraints;
> +
> +	ret = sg_check_constraints(&cons, buf, len);
> +	if (ret)
> +		return ret;
> +
> +	sg_buf = buf;
> +	remaining = len;
> +	i = 0;
> +	sg_for_each_chunk_in_buf(sg_buf, remaining, chunk_len, &cons)
> +		i++;
> +
> +	ret = sg_alloc_table(sgt, i, gfp_mask);
> +	if (ret)
> +		return ret;
> +
> +	sg_buf = buf;
> +	remaining = len;
> +	i = 0;
> +	sg_for_each_chunk_in_buf(sg_buf, remaining, chunk_len, &cons) {
> +		if (is_vmalloc_addr(sg_buf)) {
> +			struct page *vm_page;
> +
> +			vm_page = vmalloc_to_page(sg_buf);
> +			if (!vm_page) {
> +				ret = -ENOMEM;
> +				goto err_free_table;
> +			}
> +
> +			sg_set_page(&sgt->sgl[i], vm_page, chunk_len,
> +				    offset_in_page(sg_buf));
> +		} else {
> +			sg_set_buf(&sgt->sgl[i], sg_buf, chunk_len);
> +		}
> +
> +		i++;
> +	}
> +
> +	return 0;
> +
> +err_free_table:
> +	sg_free_table(sgt);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(sg_alloc_table_from_buf);
> +
>  void __sg_page_iter_start(struct sg_page_iter *piter,
>  			  struct scatterlist *sglist, unsigned int nents,
>  			  unsigned long pgoffset)

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]


#1353222 — Re: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper

FromBoris Brezillon <boris.brezillon@free-electrons.com>
Date2016-03-08 18:00 +0100
SubjectRe: [PATCH 4/7] scatterlist: add sg_alloc_table_from_buf() helper
Message-ID<rat9g-3s9-35@gated-at.bofh.it>
In reply to#1353212
Hi Laurent,

On Tue, 08 Mar 2016 18:51:49 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Boris,
> 
> Thank you for the patch.
> 
> On Tuesday 08 March 2016 12:15:12 Boris Brezillon wrote:
> > sg_alloc_table_from_buf() provides an easy solution to create an sg_table
> > from a virtual address pointer. This function takes care of dealing with
> > vmallocated buffers, buffer alignment, or DMA engine limitations (maximum
> > DMA transfer size).
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  include/linux/scatterlist.h |  24 ++++++++
> >  lib/scatterlist.c           | 142 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 166 insertions(+)
> > 
> > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> > index 556ec1e..4a75362 100644
> > --- a/include/linux/scatterlist.h
> > +++ b/include/linux/scatterlist.h
> > @@ -41,6 +41,27 @@ struct sg_table {
> >  	unsigned int orig_nents;	/* original size of list */
> >  };
> > 
> > +/**
> > + * struct sg_constraints - SG constraints structure
> > + *
> > + * @max_chunk_len: maximum chunk buffer length. Each SG entry has to be
> > smaller
> > + *		   than this value. Zero means no constraint.
> > + * @required_alignment: minimum alignment. Is used for both size and
> > pointer
> > + *			alignment. If this constraint is not met, the function
> > + *			should return -EINVAL.
> > + * @preferred_alignment: preferred alignment. Mainly used to optimize
> > + *			 throughput when the DMA engine performs better when
> > + *			 doing aligned accesses.
> > + *
> > + * This structure is here to help sg_alloc_table_from_buf() create the
> > optimal
> > + * SG list based on DMA engine constraints.
> > + */
> > +struct sg_constraints {
> > +	size_t max_chunk_len;
> > +	size_t required_alignment;
> > +	size_t preferred_alignment;
> > +};
> > +
> >  /*
> >   * Notes on SG table design.
> >   *
> > @@ -265,6 +286,9 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
> >  	struct page **pages, unsigned int n_pages,
> >  	unsigned long offset, unsigned long size,
> >  	gfp_t gfp_mask);
> > +int sg_alloc_table_from_buf(struct sg_table *sgt, const void *buf, size_t
> > len,
> > +			    const struct sg_constraints *constraints,
> > +			    gfp_t gfp_mask);
> > 
> >  size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void
> > *buf, size_t buflen, off_t skip, bool to_buffer);
> > diff --git a/lib/scatterlist.c b/lib/scatterlist.c
> > index bafa993..706b583 100644
> > --- a/lib/scatterlist.c
> > +++ b/lib/scatterlist.c
> > @@ -433,6 +433,148 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
> >  }
> >  EXPORT_SYMBOL(sg_alloc_table_from_pages);
> > 
> > +static size_t sg_buf_chunk_len(const void *buf, size_t len,
> > +			       const struct sg_constraints *cons)
> > +{
> > +	size_t chunk_len = len;
> > +
> > +	if (cons->max_chunk_len)
> > +		chunk_len = min_t(size_t, chunk_len, cons->max_chunk_len);
> > +
> > +	if (is_vmalloc_addr(buf))
> > +		chunk_len = min_t(size_t, chunk_len,
> > +				  PAGE_SIZE - offset_in_page(buf));
> 
> This will lead to page-sized scatter-gather entries even for pages of the 
> vmalloc memory that happen to be physically contiguous. That works, but I 
> wonder whether we'd want to be more efficient.

Hm, I thought dma_map_sg() was taking care of merging pĥysically
contiguous memory regions, but maybe I'm wrong. Anyway, that's
definitely something I can add at this level.

> 
> > +	if (!IS_ALIGNED((unsigned long)buf, cons->preferred_alignment)) {
> > +		const void *aligned_buf = PTR_ALIGN(buf,
> > +						    cons->preferred_alignment);
> > +		size_t unaligned_len = (unsigned long)(aligned_buf - buf);
> > +
> > +		chunk_len = min_t(size_t, chunk_len, unaligned_len);
> > +	} else if (chunk_len > cons->preferred_alignment) {
> > +		chunk_len &= ~(cons->preferred_alignment - 1);
> > +	}
> > +
> > +	return chunk_len;
> > +}
> > +
> > +#define sg_for_each_chunk_in_buf(buf, len, chunk_len, constraints)	\
> > +	for (chunk_len = sg_buf_chunk_len(buf, len, constraints);	\
> > +	     len;							\
> > +	     len -= chunk_len, buf += chunk_len,			\
> > +	     chunk_len = sg_buf_chunk_len(buf, len, constraints))
> > +
> > +static int sg_check_constraints(struct sg_constraints *cons,
> > +				const void *buf, size_t len)
> > +{
> > +	if (!cons->required_alignment)
> > +		cons->required_alignment = 1;
> > +
> > +	if (!cons->preferred_alignment)
> > +		cons->preferred_alignment = cons->required_alignment;
> > +
> > +	/* Test if buf and len are properly aligned. */
> > +	if (!IS_ALIGNED((unsigned long)buf, cons->required_alignment) ||
> > +	    !IS_ALIGNED(len, cons->required_alignment))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * if the buffer has been vmallocated and required_alignment is
> > +	 * more than PAGE_SIZE we cannot guarantee it.
> > +	 */
> > +	if (is_vmalloc_addr(buf) && cons->required_alignment > PAGE_SIZE)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * max_chunk_len has to be aligned to required_alignment to
> > +	 * guarantee that all buffer chunks are aligned correctly.
> > +	 */
> > +	if (!IS_ALIGNED(cons->max_chunk_len, cons->required_alignment))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * preferred_alignment has to be aligned to required_alignment
> > +	 * to avoid misalignment of buffer chunks.
> > +	 */
> > +	if (!IS_ALIGNED(cons->preferred_alignment, cons->required_alignment))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * sg_alloc_table_from_buf - create an SG table from a buffer
> > + *
> > + * @sgt: SG table
> > + * @buf: buffer you want to create this SG table from
> > + * @len: length of buf
> > + * @constraints: optional constraints to take into account when creating
> > + *		 the SG table. Can be NULL if no specific constraints are
> > + *		 required.
> > + * @gfp_mask: type of allocation to use when creating the table
> > + *
> > + * This function creates an SG table from a buffer, its length and some
> > + * SG constraints.
> > + *
> > + * Note: This function supports vmallocated buffers.
> 
> What other types of memory does it support ? kmalloc() quite obviously, are 
> there others ? I think you should explicitly list the memory types that the 
> function intends to support.

Sure, I can explicitly list all memory types. Do you see any other kind
of memory other than the vmalloced and physically-contiguous ones?

Thanks for your review.

Boris
-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web