From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751982AbdHBXB4 (ORCPT ); Wed, 2 Aug 2017 19:01:56 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:57442 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751944AbdHBXBz (ORCPT ); Wed, 2 Aug 2017 19:01:55 -0400 Date: Wed, 2 Aug 2017 16:01:53 -0700 From: Andrew Morton To: Tvrtko Ursulin Cc: Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, Masahiro Yamada , Ben Widawsky , "Vetter, Daniel" Subject: Re: [Intel-gfx] [PATCH 3/4] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Message-Id: <20170802160153.dee32799b03f81b925cf5289@linux-foundation.org> In-Reply-To: References: <20170727090504.15812-1-tvrtko.ursulin@linux.intel.com> <20170727090504.15812-4-tvrtko.ursulin@linux.intel.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2 Aug 2017 14:06:39 +0100 Tvrtko Ursulin wrote: > > Hi Andrew, > > We have a couple of small lib/scatterlist.c tidies here, plus exporting > the new API which allows drivers to control the maximum coalesced entry > as created by __sg_alloc_table_from_pages. > > I am looking for an ack to merge these three patches via the drm-intel tree. > > > ... > > On 27/07/2017 10:05, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin > > > > Drivers like i915 benefit from being able to control the maxium > > size of the sg coallesced segment while building the scatter- "coalesced" > > gather list. > > > > Introduce and export the __sg_alloc_table_from_pages function > > which will allow it that control. > > > > ... > > > --- a/lib/scatterlist.c > > +++ b/lib/scatterlist.c > > @@ -370,14 +370,15 @@ int sg_alloc_table(struct sg_table *table, unsigned int nents, gfp_t gfp_mask) > > EXPORT_SYMBOL(sg_alloc_table); > > > > /** > > - * sg_alloc_table_from_pages - Allocate and initialize an sg table from > > - * an array of pages > > - * @sgt: The sg table header to use > > - * @pages: Pointer to an array of page pointers > > - * @n_pages: Number of pages in the pages array > > - * @offset: Offset from start of the first page to the start of a buffer > > - * @size: Number of valid bytes in the buffer (after offset) > > - * @gfp_mask: GFP allocation mask > > + * __sg_alloc_table_from_pages - Allocate and initialize an sg table from > > + * an array of pages > > + * @sgt: The sg table header to use > > + * @pages: Pointer to an array of page pointers > > + * @n_pages: Number of pages in the pages array > > + * @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) > > + * @gfp_mask: GFP allocation mask > > * > > * Description: > > * Allocate and initialize an sg table from a list of pages. Contiguous The Description doesn't describe how this differs from sg_alloc_table_from_pages(), although it doesn't seem terribly important. > > + > > +/** > > + * sg_alloc_table_from_pages - Allocate and initialize an sg table from > > + * an array of pages > > + * @sgt: The sg table header to use > > + * @pages: Pointer to an array of page pointers > > + * @n_pages: Number of pages in the pages array > > + * @offset: Offset from start of the first page to the start of a buffer > > + * @size: Number of valid bytes in the buffer (after offset) > > + * @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. A 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. > > + * > > + * Returns: > > + * 0 on success, negative error on failure > > + */ > > +int sg_alloc_table_from_pages(struct sg_table *sgt, struct page **pages, > > + unsigned int n_pages, unsigned int offset, > > + unsigned long size, gfp_t gfp_mask) > > +{ > > + return __sg_alloc_table_from_pages(sgt, pages, n_pages, offset, size, > > + SCATTERLIST_MAX_SEGMENT, gfp_mask); > > +} Making this an inline would save a bunch or stack space in the callers? One could just add the new arg then run around and update the 10ish callers but I guess the new interface is OK. Otherwise it's OK by me, please go ahead as proposed.