From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BAD83EB64DC for ; Mon, 17 Jul 2023 13:34:09 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8030810E256; Mon, 17 Jul 2023 13:34:08 +0000 (UTC) Received: from out-17.mta0.migadu.com (out-17.mta0.migadu.com [IPv6:2001:41d0:1004:224b::11]) by gabe.freedesktop.org (Postfix) with ESMTPS id 21F5610E256 for ; Mon, 17 Jul 2023 13:34:06 +0000 (UTC) Content-Type: multipart/alternative; boundary="------------IJqnIXHQHd0oWDiqCHPxgcBt" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1689600843; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=kkckZSA+inAR7TFamVZNuzW5TIQv43tF8fQPgC6ze4U=; b=pFcInVz32lzzxaGc48q9cCI+P8pY1nDNyCYMWa9gpPNdfkwe7TRBiqt1ZScQVKuu9zXtF4 X/01soZdcxtwdtJnAH3+8pQs+08mTOLXTfelaOeZpTu94wT0jhcK0TCDPyg8sopmm+rRwl lmgj0sBlzDjsrGa9jsgak6557stfrfc= Message-ID: Date: Mon, 17 Jul 2023 21:33:55 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v1 1/8] drm/etnaviv: Using the size_t variable to store the number of pages Content-Language: en-US To: Lucas Stach , Russell King , Christian Gmeiner , David Airlie , Daniel Vetter References: <20230623100822.274706-1-sui.jingfeng@linux.dev> <20230623100822.274706-2-sui.jingfeng@linux.dev> <4f80b175f94eaf386354d1f3425208ca6cf20482.camel@pengutronix.de> <4484c007-132c-ce47-fa71-87f33c87fe07@linux.dev> <8b0d82d48ff24f578e7a1c7433e56ddaadc3188b.camel@pengutronix.de> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Sui Jingfeng In-Reply-To: <8b0d82d48ff24f578e7a1c7433e56ddaadc3188b.camel@pengutronix.de> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: loongson-kernel@lists.loongnix.cn, Sui Jingfeng , etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This is a multi-part message in MIME format. --------------IJqnIXHQHd0oWDiqCHPxgcBt Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, On 2023/7/17 18:38, Lucas Stach wrote: > Am Montag, dem 17.07.2023 um 18:12 +0800 schrieb Sui Jingfeng: >> Hi >> >> On 2023/7/17 17:43, Lucas Stach wrote: >>> Hi Jingfeng, >>> >>> Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng: >>>> From: Sui Jingfeng >>>> >>>> Because the etnaviv_gem_new_private() function receives the size_t argument >>>> for the number of pages. And the number of pages should be unsigned. >>>> >>>> Note that Most 32-bit architectures use "unsigned int" size_t, >>>> and all 64-bit architectures use "unsigned long" size_t. >>>> So, let's keep the argument and parameter consistent. >>>> >>> This explanation doesn't add up. npages is just that: a number of >>> pages. Why would it make sense to use size_t here? >> Because the 'size' variable in the etnaviv_gem_prime_import_sg_table() >> function is declared >> >> as size_t type. On 64-bit machine, size_t is actually is 64-bit wide and >> it is*unsigned*. >> >> While 'int' is actually 32-bit wide(at both 32-bit system and 64-bit >> system) and it is*signed*, >> >> So, my point (argument) is that >> >> >> 1) This patch help to avoid the unnecessary 64 bit to 32 bit conversion. >> >> 2) The kvmalloc_array() function also accept  size_t type (see the >> prototype of  kvmalloc_array function include/linux/slab.h) >> >> >> So my patch do helps to keep the code style consistent. >> > But then we go on to call drm_prime_sq_to_page_array(), which takes a > integer as the number of pages parameter, so the parameter types are > inconsistent before and after your patch, it just switches which > function call has to do some conversion. > But the drm_prime_sg_to_page_array() function is going to be depreciated, We probably could modified it also to unified it, that is to take size_t arguments. --------------IJqnIXHQHd0oWDiqCHPxgcBt Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi,

On 2023/7/17 18:38, Lucas Stach wrote:
Am Montag, dem 17.07.2023 um 18:12 +0800 schrieb Sui Jingfeng:

Hi

On 2023/7/17 17:43, Lucas Stach wrote:

Hi Jingfeng,

Am Freitag, dem 23.06.2023 um 18:08 +0800 schrieb Sui Jingfeng:

From: Sui Jingfeng <suijingfeng@loongson.cn>

Because the etnaviv_gem_new_private() function receives the size_t argument
for the number of pages. And the number of pages should be unsigned.

Note that Most 32-bit architectures use "unsigned int" size_t,
and all 64-bit architectures use "unsigned long" size_t.
So, let's keep the argument and parameter consistent.


This explanation doesn't add up. npages is just that: a number of
pages. Why would it make sense to use size_t here?

Because the 'size' variable in the etnaviv_gem_prime_import_sg_table() 
function is declared

as size_t type. On 64-bit machine, size_t is actually is 64-bit wide and 
it is *unsigned*.

While 'int' is actually 32-bit wide(at both 32-bit system and 64-bit 
system) and it is *signed*,

So, my point (argument) is that


1) This patch help to avoid the unnecessary 64 bit to 32 bit conversion.

2) The kvmalloc_array() function also accept  size_t type (see the 
prototype of  kvmalloc_array function include/linux/slab.h)


So my patch do helps to keep the code style consistent.


But then we go on to call drm_prime_sq_to_page_array(), which takes a
integer as the number of pages parameter, so the parameter types are
inconsistent before and after your patch, it just switches which
function call has to do some conversion.

But the drm_prime_sg_to_page_array() function is going to be depreciated,

We probably could modified it also to unified it, that is to take size_t arguments.

--------------IJqnIXHQHd0oWDiqCHPxgcBt--