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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>

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.


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux