Re: [PATCH v1 1/8] drm/etnaviv: Using the size_t variable to store the number of pages

[Sui Jingfeng <sui.jingfeng@linux.dev>]

[-- Attachment #1: Type: text/plain, Size: 1825 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 2953 bytes --]