Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752133AbdHCJVZ (ORCPT ); Thu, 3 Aug 2017 05:21:25 -0400 Received: from mga02.intel.com ([134.134.136.20]:48135 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751947AbdHCJVX (ORCPT ); Thu, 3 Aug 2017 05:21:23 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,315,1498546800"; d="scan'208";a="1158597839" Subject: Re: [Intel-gfx] [PATCH 3/4] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages To: Andrew Morton Cc: Tvrtko Ursulin , Intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, Masahiro Yamada , Ben Widawsky , "Vetter, Daniel" References: <20170727090504.15812-1-tvrtko.ursulin@linux.intel.com> <20170727090504.15812-4-tvrtko.ursulin@linux.intel.com> <20170802160153.dee32799b03f81b925cf5289@linux-foundation.org> From: Tvrtko Ursulin Message-ID: <4b50eddb-b8ce-7e8e-a3cc-71bec7252b32@linux.intel.com> Date: Thu, 3 Aug 2017 10:21:19 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170802160153.dee32799b03f81b925cf5289@linux-foundation.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4176 Lines: 112 On 03/08/2017 00:01, Andrew Morton wrote: > 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" Oops, I've had this in other patches in the series. Fixed now. >>> 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. Well spotted, I've fixed this as well. >>> + >>> +/** >>> + * 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. On the first suggestion - there are other trivial wrappers in lib/scatterlist.c which could benefit from the same treatment (move to inline) so I opted not to do this in this patch but will send a follow up. > Otherwise it's OK by me, please go ahead as proposed. Thanks a lot! Regards, Tvrtko