On Sun, Sep 27, 2020 at 09:46:44AM +0300, Leon Romanovsky wrote:
> +struct scatterlist *__sg_alloc_table_from_pages(struct sg_table *sgt,
> +             struct page **pages, unsigned int n_pages, unsigned int offset,
> +             unsigned long size, unsigned int max_segment,
> +             struct scatterlist *prv, unsigned int left_pages,
> +             gfp_t gfp_mask)
>  {
> -     unsigned int chunks, cur_page, seg_len, i;
> +     unsigned int chunks, cur_page, seg_len, i, prv_len = 0;
> +     struct scatterlist *s = prv;
> +     unsigned int table_size;
> +     unsigned int tmp_nents;
>       int ret;
> -     struct scatterlist *s;
> 
>       if (WARN_ON(!max_segment || offset_in_page(max_segment)))
> -             return -EINVAL;
> +             return ERR_PTR(-EINVAL);
> +     if (IS_ENABLED(CONFIG_ARCH_NO_SG_CHAIN) && prv)
> +             return ERR_PTR(-EOPNOTSUPP);
> +
> +     tmp_nents = prv ? sgt->nents : 0;
> +
> +     if (prv &&
> +         page_to_pfn(sg_page(prv)) + (prv->length >> PAGE_SHIFT) ==

This calculation of the end doesn't consider sg->offset

> +         page_to_pfn(pages[0]))
> +             prv_len = prv->length;
> 
>       /* compute number of contiguous chunks */
>       chunks = 1;
> @@ -410,13 +461,17 @@ int __sg_alloc_table_from_pages(struct sg_table *sgt, 
> struct page **pages,
>               }
>       }
> 
> -     ret = sg_alloc_table(sgt, chunks, gfp_mask);
> -     if (unlikely(ret))
> -             return ret;
> +     if (!prv) {
> +             /* Only the last allocation could be less than the maximum */
> +             table_size = left_pages ? SG_MAX_SINGLE_ALLOC : chunks;
> +             ret = sg_alloc_table(sgt, table_size, gfp_mask);
> +             if (unlikely(ret))
> +                     return ERR_PTR(ret);
> +     }

This is basically redundant right? Now that get_next_sg() can allocate
SGs it can just build them one by one, no need to preallocate.

Actually all the changes the the allocation seem like overkill, just
allocate a single new array directly in get_next_sg() whenever it
needs.

Something like this:

@@ -365,6 +372,37 @@ int sg_alloc_table(struct sg_table *table, unsigned int 
nents, gfp_t gfp_mask)
 }
 EXPORT_SYMBOL(sg_alloc_table);
 
+static struct scatterlist *get_next_sg(struct sg_table *table,
+               struct scatterlist *cur, unsigned long needed_sges,
+               gfp_t gfp_mask)
+{
+       struct scatterlist *new_sg;
+       unsigned int alloc_size;
+
+       if (cur) {
+               struct scatterlist *next_sg = sg_next(cur);
+
+               /* Check if last entry should be keeped for chainning */
+               if (!sg_is_last(next_sg) || needed_sges == 1)
+                       return next_sg;
+       }
+
+       alloc_size = min_t(unsigned long, needed_sges, SG_MAX_SINGLE_ALLOC);
+       new_sg = sg_kmalloc(alloc_size, gfp_mask);
+       if (!new_sg)
+               return ERR_PTR(-ENOMEM);
+       sg_init_table(new_sg, alloc_size);
+       if (cur) {
+               __sg_chain(cur, new_sg);
+               table->orig_nents += alloc_size - 1;
+       } else {
+               table->sgl = new_sg;
+               table->orig_nents = alloc_size;
+               table->nents = 0;
+       }
+       return new_sg;
+}
+
 /**
  * __sg_alloc_table_from_pages - Allocate and initialize an sg table from
  *                              an array of pages
@@ -374,29 +412,64 @@ EXPORT_SYMBOL(sg_alloc_table);
  * @offset:      Offset from start of the first page to the start of a buffer
  * @size:        Number of valid bytes in the buffer (after offset)
  * @max_segment: Maximum size of a scatterlist node in bytes (page aligned)
+ * @prv:        Last populated sge in sgt
+ * @left_pages:  Left pages caller have to set after this call
  * @gfp_mask:   GFP allocation mask
  *
- *  Description:
- *    Allocate and initialize an sg table from a list of pages. Contiguous
- *    ranges of the pages are squashed into a single scatterlist node up to the
- *    maximum size specified in @max_segment. An user may provide an offset at 
a
- *    start and a size of valid data in a buffer specified by the page array.
- *    The returned sg table is released by sg_free_table.
+ * Description:
+ *    If @prv is NULL, allocate and initialize an sg table from a list of 
pages,
+ *    else reuse the scatterlist passed in at @prv.
+ *    Contiguous ranges of the pages are squashed into a single scatterlist
+ *    entry up to the maximum size specified in @max_segment.  A user may
+ *    provide an offset at a start and a size of valid data in a buffer
+ *    specified by the page array.
  *
  * Returns:
- *   0 on success, negative error on failure
+ *   Last SGE in sgt on success, PTR_ERR on otherwise.
+ *   The allocation in @sgt must be released by sg_free_table.
+ *
+ * Notes:
+ *   If this function returns non-0 (eg failure), the caller must call
+ *   sg_free_table() to cleanup any leftover allocations.
  */
-int __sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages,
-                               unsigned int n_pages, unsigned int offset,
-                               unsigned long size, unsigned int max_segment,
-                               gfp_t gfp_mask)
+struct scatterlist *__sg_alloc_table_from_pages(struct sg_table *sgt,
+               struct page **pages, unsigned int n_pages, unsigned int offset,
+               unsigned long size, unsigned int max_segment,
+               struct scatterlist *prv, unsigned int left_pages,
+               gfp_t gfp_mask)
 {
-       unsigned int chunks, cur_page, seg_len, i;
-       int ret;
-       struct scatterlist *s;
+       unsigned int chunks, cur_page, seg_len, i, prv_len = 0;
+       unsigned int added_nents = 0;
+       struct scatterlist *s = prv;
 
        if (WARN_ON(!max_segment || offset_in_page(max_segment)))
-               return -EINVAL;
+               return ERR_PTR(-EINVAL);
+       if (IS_ENABLED(CONFIG_ARCH_NO_SG_CHAIN) && prv)
+               return ERR_PTR(-EOPNOTSUPP);
+
+       if (prv) {
+               unsigned long paddr = (page_to_pfn(sg_page(prv)) * PAGE_SIZE +
+                                      prv->offset + prv->length) /
+                                     PAGE_SIZE;
+
+               if (WARN_ON(offset))
+                       return ERR_PTR(-EINVAL);
+
+               /* Merge contiguous pages into the last SG */
+               prv_len = prv->length;
+               while (n_pages && page_to_pfn(pages[0]) == paddr) {
+                       if (prv->length + PAGE_SIZE > max_segment)
+                               break;
+                       prv->length += PAGE_SIZE;
+                       paddr++;
+                       pages++;
+                       n_pages--;
+               }
+               if (!n_pages) {
+                       sg_mark_end(sg_next(prv));
+                       return prv;
+               }
+       }
 
        /* compute number of contiguous chunks */
        chunks = 1;
@@ -410,13 +483,9 @@ int __sg_alloc_table_from_pages(struct sg_table *sgt, 
struct page **pages,
                }
        }
 
-       ret = sg_alloc_table(sgt, chunks, gfp_mask);
-       if (unlikely(ret))
-               return ret;
-
        /* merging chunks and putting them into the scatterlist */
        cur_page = 0;
-       for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
+       for (i = 0; i < chunks; i++) {
                unsigned int j, chunk_size;
 
                /* look for the end of the current chunk */
@@ -429,15 +498,28 @@ int __sg_alloc_table_from_pages(struct sg_table *sgt, 
struct page **pages,
                                break;
                }
 
+               /* Pass how many chunks might be left */
+               s = get_next_sg(sgt, s, chunks - i + left_pages, gfp_mask);
+               if (IS_ERR(s)) {
+                       /*
+                        * Adjust entry length to be as before function was
+                        * called.
+                        */
+                       if (prv)
+                               prv->length = prv_len;
+                       return s;
+               }
                chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;
                sg_set_page(s, pages[cur_page],
                            min_t(unsigned long, size, chunk_size), offset);
+               added_nents++;
                size -= chunk_size;
                offset = 0;
                cur_page = j;
        }
-
-       return 0;
+       sgt->nents += added_nents;
+       sg_mark_end(s);
+       return s;
 }
 EXPORT_SYMBOL(__sg_alloc_table_from_pages);
 

Reply via email to