Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation

[Matthew Auld]

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@xxxxxxxx>
> > > > >
> > > > > 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@xxxxxxxx>
> > > > Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > > 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