[Mesa-dev] [PATCH 03/39] llvmpipe: Fix overflow for 32 bits available memory computation

[Axel Davy]

On 16/05/2016 20:08, Emil Velikov wrote:
> On 15 May 2016 at 11:45, Axel Davy <axel.davy at ens.fr> wrote:
>> And cap to 2 GB on 32 bits.
>>
>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94561
>>
> Nitpick: please use "Bugzilla: htt.."
>
>> Signed-off-by: Axel Davy <axel.davy at ens.fr>
>> ---
>>   src/gallium/auxiliary/os/os_misc.c       | 2 +-
>>   src/gallium/drivers/llvmpipe/lp_screen.c | 5 +++++
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/os/os_misc.c b/src/gallium/auxiliary/os/os_misc.c
>> index d6b83e9..cf2ef95 100644
>> --- a/src/gallium/auxiliary/os/os_misc.c
>> +++ b/src/gallium/auxiliary/os/os_misc.c
>> @@ -117,7 +117,7 @@ os_get_total_physical_memory(uint64_t *size)
>>      const long phys_pages = sysconf(_SC_PHYS_PAGES);
>>      const long page_size = sysconf(_SC_PAGE_SIZE);
>>
>> -   *size = phys_pages * page_size;
>> +   *size = (int64_t)phys_pages * (int64_t)page_size;
>>      return (phys_pages > 0 && page_size > 0);
> Can you please split this out roughly like the following
>
> if (phys_pages =< 0 || page_size =< 0)
>    return false;
>
> *size = (unt64_t)phys_pages * (unt64_t)page_size;
> return true;
>
> I might have butchered it while copying from i915/i965.
>
>>   #elif defined(PIPE_OS_APPLE) || defined(PIPE_OS_BSD)
>>      size_t len = sizeof(*size);
>> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
>> index 4f61de8..2f84b75 100644
>> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
>> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
>> @@ -279,6 +279,11 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
>>         if (!os_get_total_physical_memory(&system_memory))
>>            return 0;
>>
>> +#ifdef PIPE_ARCH_X86
>> +      /* cap to 2 GB on 32 bits system */
>> +      system_memory = MIN2(system_memory, 2048);
>> +#endif
>> +
> The 2GiB feels somewhat arbitrary, can we add a brief comment about it
> ? Then there's the question if other drivers want to apply, and thus
> move, it elsewhere.
I think you can have a 32 bits cpu adressing system with a gpu with more 
than 4GB.
However in the case of sw rendering, you eat the same memory space than 
the app.

2GB was the limit proposed in the bug report. Would you be happy with a 
link to it in the comment ?

Axel

> -Emil
>