Received-SPF: pass (google.com: domain of linux-kernel+bounces-71250-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <124a084e-9f8e-4b38-a0e3-9dfac0312e97@intel.com>
Date: Mon, 19 Feb 2024 11:49:19 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
Content-Language: en-GB
To: =?UTF-8?Q?Christian_K=C3=B6nig?= <christian.koenig@amd.com>,
 Arnd Bergmann <arnd@arndb.de>, Randy Dunlap <rdunlap@infradead.org>,
 Arnd Bergmann <arnd@kernel.org>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>
Cc: Dave Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
 Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>,
 David Gow <davidgow@google.com>, =?UTF-8?Q?Ma=C3=ADra_Canal?=
 <mcanal@igalia.com>, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240216202442.2493031-1-arnd@kernel.org>
 <745e156e-c3ec-427f-98ad-bfc7d3cfd846@infradead.org>
 <4cdc5b58-11c1-490d-8c3b-6352d8f1b8cb@amd.com>
 <fbd0426c-fdd2-4b7f-a13e-072ed5f973de@app.fastmail.com>
 <a293460f-6a40-427f-b5d2-2e701d1af229@amd.com>
From: Matthew Auld <matthew.auld@intel.com>
In-Reply-To: <a293460f-6a40-427f-b5d2-2e701d1af229@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 19/02/2024 11:41, Christian König wrote:
> Am 19.02.24 um 12:29 schrieb Arnd Bergmann:
>> On Mon, Feb 19, 2024, at 12:22, Christian König wrote:
>>> Am 17.02.24 um 02:31 schrieb Randy Dunlap:
>>>> On 2/16/24 12:24, Arnd Bergmann wrote:
>>>>> From: Arnd Bergmann <arnd@arndb.de>
>>>>>
>>>>> The newly added drm_test_buddy_alloc_contiguous() test fails to 
>>>>> link on
>>>>> 32-bit targets because of inadvertent 64-bit calculations:
>>>>>
>>>>> ERROR: modpost: "__aeabi_uldivmod" 
>>>>> [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
>>>>> ERROR: modpost: "__aeabi_ldivmod" 
>>>>> [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
>>>>>
>>>>> >From what I can tell, the numbers cannot possibly overflow a 
>>>>> 32-bit size,
>>>>> so use different types for these.
>>>>>
>>>>> I noticed that the function has another possible flaw in that is mixes
>>>>> what it calls pages with 4KB units. This is a big confusing at best,
>>>>> or possibly broken when built on machines with larger pages.
>>>>>
>>>>> Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
>>>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>>> I've just pushed a similar patch Mathew came up a bit earlier to
>>> drm-misc-fixes.
>>>
>>> Sorry for the noise, I have to catch up on picking up patches for
>>> misc-fixes and misc-next.
>> Ok, thanks.
>>
>> Have you looked at how this code works for larger values of PAGE_SIZE?
>> Is there any need to change other things or will this work with the
>> hardcoded 4KB chunks?
> 
> I haven't looked into the details, but I've pointed out before that 
> using PAGE_SIZE in the buddy or its test cases would be incorrect.
> 
> Background is that the buddy allocator is for devices and those work 
> independent of the CPU PAGE_SIZE. So it can be that on a CPU with 64k 
> pages the buddy still needs to work with 4k.
> 
> Could be that this is work, but could as well be that this is completely 
> broken. Arun and Mathew needs to answer this, I haven't tested it nor 
> reviewed the code.

Yeah, we should not be using PAGE_SIZE or PAGE_SHIFT in drm_buddy.[ch] 
and tests/drm_buddy_test.c. The smallest default page size is SZ_4K for 
drm_buddy. A patch to fix that would be very welcome. If no takers I can 
send something.

> 
> Regards,
> Christian.
> 
>>
>>       Arnd
>