* [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
@ 2024-02-16 20:24 Arnd Bergmann
2024-02-17 1:31 ` Randy Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2024-02-16 20:24 UTC (permalink / raw
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: Arnd Bergmann, David Airlie, Daniel Vetter,
Arunpravin Paneer Selvam, Christian König, David Gow,
Maíra Canal, Matthew Auld, dri-devel, linux-kernel
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>
---
drivers/gpu/drm/tests/drm_buddy_test.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
index fee6bec757d1..50a5f98cd5bd 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -21,7 +21,8 @@ static inline u64 get_size(int order, u64 chunk_size)
static void drm_test_buddy_alloc_contiguous(struct kunit *test)
{
- u64 mm_size, ps = SZ_4K, i, n_pages, total;
+ u64 mm_size, total;
+ u32 i, ps = SZ_4K, n_pages;
struct drm_buddy_block *block;
struct drm_buddy mm;
LIST_HEAD(left);
@@ -29,7 +30,8 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
LIST_HEAD(right);
LIST_HEAD(allocated);
- mm_size = 16 * 3 * SZ_4K;
+ n_pages = 16 * 3;
+ mm_size = n_pages * SZ_4K;
KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
@@ -42,7 +44,6 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
*/
i = 0;
- n_pages = mm_size / ps;
do {
struct list_head *list;
int slot = i % 3;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
2024-02-16 20:24 [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation Arnd Bergmann
@ 2024-02-17 1:31 ` Randy Dunlap
2024-02-19 11:22 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2024-02-17 1:31 UTC (permalink / raw
To: Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: Arnd Bergmann, David Airlie, Daniel Vetter,
Arunpravin Paneer Selvam, Christian König, David Gow,
Maíra Canal, Matthew Auld, dri-devel, linux-kernel
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>
Thanks.
> ---
> drivers/gpu/drm/tests/drm_buddy_test.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
> index fee6bec757d1..50a5f98cd5bd 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -21,7 +21,8 @@ static inline u64 get_size(int order, u64 chunk_size)
>
> static void drm_test_buddy_alloc_contiguous(struct kunit *test)
> {
> - u64 mm_size, ps = SZ_4K, i, n_pages, total;
> + u64 mm_size, total;
> + u32 i, ps = SZ_4K, n_pages;
> struct drm_buddy_block *block;
> struct drm_buddy mm;
> LIST_HEAD(left);
> @@ -29,7 +30,8 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
> LIST_HEAD(right);
> LIST_HEAD(allocated);
>
> - mm_size = 16 * 3 * SZ_4K;
> + n_pages = 16 * 3;
> + mm_size = n_pages * SZ_4K;
>
> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
>
> @@ -42,7 +44,6 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
> */
>
> i = 0;
> - n_pages = mm_size / ps;
> do {
> struct list_head *list;
> int slot = i % 3;
--
#Randy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
2024-02-17 1:31 ` Randy Dunlap
@ 2024-02-19 11:22 ` Christian König
2024-02-19 11:29 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2024-02-19 11:22 UTC (permalink / raw
To: Randy Dunlap, Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: Arnd Bergmann, David Airlie, Daniel Vetter,
Arunpravin Paneer Selvam, David Gow, Maíra Canal,
Matthew Auld, dri-devel, linux-kernel
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.
Christian.
>
> Thanks.
>
>> ---
>> drivers/gpu/drm/tests/drm_buddy_test.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
>> index fee6bec757d1..50a5f98cd5bd 100644
>> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
>> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
>> @@ -21,7 +21,8 @@ static inline u64 get_size(int order, u64 chunk_size)
>>
>> static void drm_test_buddy_alloc_contiguous(struct kunit *test)
>> {
>> - u64 mm_size, ps = SZ_4K, i, n_pages, total;
>> + u64 mm_size, total;
>> + u32 i, ps = SZ_4K, n_pages;
>> struct drm_buddy_block *block;
>> struct drm_buddy mm;
>> LIST_HEAD(left);
>> @@ -29,7 +30,8 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
>> LIST_HEAD(right);
>> LIST_HEAD(allocated);
>>
>> - mm_size = 16 * 3 * SZ_4K;
>> + n_pages = 16 * 3;
>> + mm_size = n_pages * SZ_4K;
>>
>> KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
>>
>> @@ -42,7 +44,6 @@ static void drm_test_buddy_alloc_contiguous(struct kunit *test)
>> */
>>
>> i = 0;
>> - n_pages = mm_size / ps;
>> do {
>> struct list_head *list;
>> int slot = i % 3;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
2024-02-19 11:22 ` Christian König
@ 2024-02-19 11:29 ` Arnd Bergmann
2024-02-19 11:41 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2024-02-19 11:29 UTC (permalink / raw
To: Christian König, Randy Dunlap, Arnd Bergmann,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: Dave Airlie, Daniel Vetter, Arunpravin Paneer Selvam, David Gow,
Maíra Canal, Matthew Auld, dri-devel, linux-kernel
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?
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
2024-02-19 11:29 ` Arnd Bergmann
@ 2024-02-19 11:41 ` Christian König
2024-02-19 11:49 ` Matthew Auld
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2024-02-19 11:41 UTC (permalink / raw
To: Arnd Bergmann, Randy Dunlap, Arnd Bergmann, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann
Cc: Dave Airlie, Daniel Vetter, Arunpravin Paneer Selvam, David Gow,
Maíra Canal, Matthew Auld, dri-devel, linux-kernel
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.
Regards,
Christian.
>
> Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
2024-02-19 11:41 ` Christian König
@ 2024-02-19 11:49 ` Matthew Auld
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Auld @ 2024-02-19 11:49 UTC (permalink / raw
To: Christian König, Arnd Bergmann, Randy Dunlap, Arnd Bergmann,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: Dave Airlie, Daniel Vetter, Arunpravin Paneer Selvam, David Gow,
Maíra Canal, dri-devel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-19 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-16 20:24 [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation Arnd Bergmann
2024-02-17 1:31 ` Randy Dunlap
2024-02-19 11:22 ` Christian König
2024-02-19 11:29 ` Arnd Bergmann
2024-02-19 11:41 ` Christian König
2024-02-19 11:49 ` Matthew Auld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).