Received: by 2002:a4f:b056:0:0:0:0:0 with SMTP id m22csp1407916ivi; Fri, 2 Oct 2020 09:21:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwVNrOQogq0gNdmFGLfVQcPQgK55gHec1ftNpwAw62JSZMrvBfa8t1wUJHkDguOR0kCQBWp X-Received: by 2002:a17:906:868c:: with SMTP id g12mr2902759ejx.230.1601655702547; Fri, 02 Oct 2020 09:21:42 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id bh4si1660546ejb.421.2020.10.02.09.21.19; Fri, 02 Oct 2020 09:21:42 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@nvidia.com header.s=n1 header.b=PcBb27So; arc=fail (signature failed); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388076AbgJBQTS (ORCPT + 99 others); Fri, 2 Oct 2020 12:19:18 -0400 Received: from nat-hk.nvidia.com ([203.18.50.4]:27196 "EHLO nat-hk.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726386AbgJBQTR (ORCPT ); Fri, 2 Oct 2020 12:19:17 -0400 Received: from HKMAIL102.nvidia.com (Not Verified[10.18.92.77]) by nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Sat, 03 Oct 2020 00:19:14 +0800 Received: from HKMAIL103.nvidia.com (10.18.16.12) by HKMAIL102.nvidia.com (10.18.16.11) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Fri, 2 Oct 2020 16:19:10 +0000 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.170) by HKMAIL103.nvidia.com (10.18.16.12) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Fri, 2 Oct 2020 16:19:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=K6hPXdODBseL26BhbRZS3p+D48l26Z4dnxkphJQza7gLyRqzZy7yvHP9hRjyeBrPZt/vO/fq1Ua4VK1E0t+eyuI9NlECgVetrM6+I4mj7kFnC5HJOYkLypT08CmSD9E/40B35g/VzoGXJXiZ3U6i2ENzCyAJm7fJFDvs8WWrxwOzhiV4P8olnvBzfDUpaL3ypla9AfAujnPBinvONbHJF//8bm1TEuQ7kojXI8FWiOVJ1KaXS+tjoSzb2YQ5aycnmJDmuYcb0JhEGoUuPBPC2lltuMd7jsF2x5Wqm9FxYEMM9/fa0usoiHba8MhpvTrKEXM5G53jgGAa0Ya21EaP9A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cfhCU4BqWl744V0A8uhrBWr2Km5jVCXnZBEZss7cHLA=; b=l19EYNn+Y9AyRksOLMKLW6U3WE8ZN0dKH/OZ0LThbX9IcM+Pp1GKaZ+SRP8K+EuUT9C3hKp36A/v5zynUeEcmTzpqoKlcIhs8jLZ5qBKlM88HqQ9pw4GeRq+e7skRuYitlurNRQuBlJOcSgzmrEr+717dkd3/tQFdJW1NS3iuI/tFToQMQqYEYHObTqzeTJtoCIaWKFntCZ6Aho70QOcJHNYuaw1MeMBqTVaHXGMMpw6kSAzMQpbuCiPRnLSGJXsxYwBxUtBMLi1subpnt9LwU0cX+l4vms/6z7psQQ9qZSH8iPj8lsANit1hMX/ZkmFV2UfFu5Q7+V5wFZEpeY++Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from DM6PR12MB3834.namprd12.prod.outlook.com (2603:10b6:5:14a::12) by DM6PR12MB2858.namprd12.prod.outlook.com (2603:10b6:5:182::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3412.24; Fri, 2 Oct 2020 16:19:07 +0000 Received: from DM6PR12MB3834.namprd12.prod.outlook.com ([fe80::cdbe:f274:ad65:9a78]) by DM6PR12MB3834.namprd12.prod.outlook.com ([fe80::cdbe:f274:ad65:9a78%7]) with mapi id 15.20.3433.038; Fri, 2 Oct 2020 16:19:07 +0000 Date: Fri, 2 Oct 2020 13:19:05 -0300 From: Jason Gunthorpe To: Maor Gottlieb CC: Leon Romanovsky , Doug Ledford , Maor Gottlieb , Christoph Hellwig , "Daniel Vetter" , David Airlie , , , "Jani Nikula" , Joonas Lahtinen , , , Rodrigo Vivi , "Roland Scheidegger" , Tvrtko Ursulin , VMware Graphics Subject: Re: [PATCH rdma-next v4 1/4] lib/scatterlist: Add support in dynamic allocation of SG table from pages Message-ID: <20201002161905.GJ816047@nvidia.com> References: <20200927064647.3106737-1-leon@kernel.org> <20200927064647.3106737-2-leon@kernel.org> <20201002150227.GA1350139@nvidia.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: BL0PR01CA0026.prod.exchangelabs.com (2603:10b6:208:71::39) To DM6PR12MB3834.namprd12.prod.outlook.com (2603:10b6:5:14a::12) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from mlx.ziepe.ca (156.34.48.30) by BL0PR01CA0026.prod.exchangelabs.com (2603:10b6:208:71::39) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3433.34 via Frontend Transport; Fri, 2 Oct 2020 16:19:06 +0000 Received: from jgg by mlx with local (Exim 4.94) (envelope-from ) id 1kONlx-006214-3p; Fri, 02 Oct 2020 13:19:05 -0300 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1601655554; bh=cfhCU4BqWl744V0A8uhrBWr2Km5jVCXnZBEZss7cHLA=; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:Date: From:To:CC:Subject:Message-ID:References:Content-Type: Content-Disposition:In-Reply-To:X-ClientProxiedBy:MIME-Version: X-MS-Exchange-MessageSentRepresentingType; b=PcBb27SoY/4CZPJ4JoKHJ9MWxRMmFox9A2SURHGQeBMWtluouFUSnzApay5O0AzTR h8RJ3Pu4eUDB+0d3KBPeTvvyx3rC7FCiioe03QSaVGjG8c8PAVtC2lbT62TSA1rJaN pDIIK3pg7Hy6MLuVgKolOzW1uBVAapFWBGHP0l1AteVCfN2lOiqOaw15IkEQnDHNsa Dg1XbyZLZtgHR1Z/oea9GdZPYygJ1eml1MsctroehJT9QH0Cv/C4GAhFbOj3rBIn5T Jp86zoW2g+YYmEQrIUWDvuTNMm5IrmUaUNusbKs13fw61AgEIyvqVVEZ5VMo3jDY+t 1NficvusmJByw== Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 02, 2020 at 07:11:33PM +0300, Maor Gottlieb wrote: > > On 10/2/2020 6:02 PM, Jason Gunthorpe wrote: > > 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 > > Right, should be fixed. > > > > > + 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. > > No, only the last allocation could be less than maximum. (as written in the > comment). The point is that get_next_sg is fully redundent with sg_alloc_table() because it is always used in cases when prv is set. There is zero reason to call sg_alloc_table here in the one case where prv is not set. Further this cleans up the spagehtti goto in the middle of the for loop and avoids allocating an extra chunk if the page list fully fits in prv. Given how much smaller it is I think you should look more carefully. Jason