From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751103AbcGFKP4 (ORCPT <rfc822;w@1wt.eu>);
	Wed, 6 Jul 2016 06:15:56 -0400
Received: from devils.ext.ti.com ([198.47.26.153]:42438 "EHLO
	devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751268AbcGFKPw (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 6 Jul 2016 06:15:52 -0400
Subject: Re: [RFC] [PATCH v2 1/3] scatterlist: Add support to clone
 scatterlist
To: Franklin S Cooper Jr <fcooper@ti.com>, <david.s.gordon@intel.com>,
        <axboe@fb.com>, <akpm@linux-foundation.org>, <ming.l@ssi.samsung.com>,
        <linux-kernel@vger.kernel.org>, <broonie@kernel.org>,
        <linux-spi@vger.kernel.org>, <peter.ujfalusi@ti.com>
References: <1467039249-7816-1-git-send-email-fcooper@ti.com>
 <1467039249-7816-2-git-send-email-fcooper@ti.com>
From: Sekhar Nori <nsekhar@ti.com>
Message-ID: <577CDA45.80408@ti.com>
Date: Wed, 6 Jul 2016 15:45:33 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.8.0
MIME-Version: 1.0
In-Reply-To: <1467039249-7816-2-git-send-email-fcooper@ti.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Monday 27 June 2016 08:24 PM, Franklin S Cooper Jr wrote:

> +/*
> + * sg_table_clone - Duplicate an existing sg_table including chained sgl

This function should probably be called sg_clone_table() to be
consistent with sg_alloc_table(), sg_free_table() etc.

> + * @orig_table:     Original sg_table to be duplicated
> + * @len:            Total length of sg while taking chaining into account
> + * @gfp_mask:       GFP allocation mask
> + *
> + * Description:
> + *   Clone a sg_table along with chained sgl. This cloned copy may be
> + *   modified in some ways while keeping the original table and sgl in tact.
> + *   Also allow the cloned sgl copy to have a smaller length than the original
> + *   which may reduce the sgl total sg entries.
> + *
> + * Returns:
> + *   Pointer to new kmalloced sg_table, ERR_PTR() on error
> + *
> + */
> +struct sg_table *sg_table_clone(struct sg_table *orig_table, u64 len,
> +				gfp_t gfp_mask)
> +{
> +	struct sg_table	*table;
> +
> +	table = kmalloc(sizeof(struct sg_table), gfp_mask);

Can you use sg_alloc_table() to allocate the new table? The way you have
it now, it looks like users will need to use kfree() to free a cloned
table and use sg_free_table() otherwise. It will be nice if
sg_free_table() can be used consistently.

Regards,
Sekhar