[PATCH] drm/ttm: fix ttm tt init fail when size exceeds kmalloc limit

Wed Apr 20 11:55:38 UTC 2022

Hi Kevin,

no, the test case should already fail in amdgpu_bo_validate_size().

If we have a system with 2TiB of memory where the test case could 
succeed then we should increase the requested size to something larger.

And if the underlying core Linux kernel functions don't allow 
allocations as large as the requested one we should *NEVER* ever add 
workarounds like this.

It is perfectly expected that this test case is not able to fulfill the 
desired allocation. That it fails during kvmalloc is unfortunate, but 
not a show stopper.

Regards,
Christian.

Am 20.04.22 um 13:32 schrieb Wang, Yang(Kevin):
>
> [AMD Official Use Only]
>
>
> Hi Chris,
>
> you misunderstood background about this case.
>
> although we expect this test case to fail, it should fail at the 
> location where the Bo actual memory is actually allocated. now the 
> code logic will cause the failure to allocate memory to store DMA address.
>
> e.g: the case is failed in 2TB system ram machine, it should be 
> allocated successful, but it is failed.
>
> allocate 1TB BO, the ttm should allocate 1TB/4k * 8 buffer to store 
> allocate result (page address), this should not be failed usually.
>
> There is a similar fix in upstream kernel 5.18, before this fix 
> entered the TTM code, this problem existed in TTM.
>
> kernel/git/torvalds/linux.git - Linux kernel source tree 
> <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.18-rc3&id=a421ef303008b0ceee2cfc625c3246fa7654b0ca>
> mm: allow !GFP_KERNEL allocations for kvmalloc
>
> Best Regards,
> Kevin
>
> ------------------------------------------------------------------------
> *From:* Koenig, Christian <Christian.Koenig at amd.com>
> *Sent:* Wednesday, April 20, 2022 6:53 PM
> *To:* Wang, Yang(Kevin) <KevinYang.Wang at amd.com>; 
> dri-devel at lists.freedesktop.org <dri-devel at lists.freedesktop.org>; 
> amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
> *Subject:* Re: [PATCH] drm/ttm: fix ttm tt init fail when size exceeds 
> kmalloc limit
> Am 20.04.22 um 11:07 schrieb Wang, Yang(Kevin):
>>
>> [AMD Official Use Only]
>>
>>
>>
>> ------------------------------------------------------------------------
>> *From:* Koenig, Christian <Christian.Koenig at amd.com> 
>> <mailto:Christian.Koenig at amd.com>
>> *Sent:* Wednesday, April 20, 2022 5:00 PM
>> *To:* Wang, Yang(Kevin) <KevinYang.Wang at amd.com> 
>> <mailto:KevinYang.Wang at amd.com>; dri-devel at lists.freedesktop.org 
>> <mailto:dri-devel at lists.freedesktop.org> 
>> <dri-devel at lists.freedesktop.org> 
>> <mailto:dri-devel at lists.freedesktop.org>; 
>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org> 
>> <amd-gfx at lists.freedesktop.org> <mailto:amd-gfx at lists.freedesktop.org>
>> *Subject:* Re: [PATCH] drm/ttm: fix ttm tt init fail when size 
>> exceeds kmalloc limit
>> Am 20.04.22 um 10:56 schrieb Yang Wang:
>> > if the __GFP_ZERO is set, the kvmalloc() can't fallback to use 
>> vmalloc()
>>
>> Hui what? Why should kvmalloc() not be able to fallback to vmalloc()
>> when __GFP_ZERO is set?
>>
>> And even that is really the case then that sounds like a bug in 
>> kvmalloc().
>>
>> Regards,
>> Christian.
>>
>> [kevin]:
>> it is really test case from libdrm amdgpu test, which try to allocate 
>> a big BO which will cause ttm tt init fail.
>
>
> LOL! Guys, this test case is intended to fail!
> *
> *The test consists of allocating a buffer so ridiculous large that it 
> should never succeed and be rejected by the kernel driver.
>
> This patch here is a really clear NAK.
>
> Regards,
> Christian.
>
>> it may be a kvmalloc() bug, and this patch can as a workaround 
>> in ttm before this issue fix.
>>
>> void *kvmalloc_node(size_t size, gfp_t flags, int node)
>> {
>> ...
>>         if ((flags & GFP_KERNEL) != GFP_KERNEL)
>>   return kmalloc_node(size, flags, node);
>> ...
>> }
>>
>> Best Regards,
>> Kevin
>>
>> > to allocate memory, when request size is exceeds kmalloc limit, it will
>> > cause allocate memory fail.
>> >
>> > e.g: when ttm want to create a BO with 1TB size, it maybe fail.
>> >
>> > Signed-off-by: Yang Wang <KevinYang.Wang at amd.com> 
>> <mailto:KevinYang.Wang at amd.com>
>> > ---
>> >   drivers/gpu/drm/ttm/ttm_tt.c | 14 +++++++++++---
>> >   1 file changed, 11 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
>> b/drivers/gpu/drm/ttm/ttm_tt.c
>> > index 79c870a3bef8..9f2f3e576b8d 100644
>> > --- a/drivers/gpu/drm/ttm/ttm_tt.c
>> > +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>> > @@ -97,9 +97,12 @@ int ttm_tt_create(struct ttm_buffer_object *bo, 
>> bool zero_alloc)
>> >   static int ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
>> >   {
>> >        ttm->pages = kvmalloc_array(ttm->num_pages, sizeof(void*),
>> > -                     GFP_KERNEL | __GFP_ZERO);
>> > + GFP_KERNEL);
>> >        if (!ttm->pages)
>> >                return -ENOMEM;
>> > +
>> > +     memset(ttm->pages, 0, ttm->num_pages * sizeof(void *));
>> > +
>> >        return 0;
>> >   }
>> >
>> > @@ -108,10 +111,12 @@ static int 
>> ttm_dma_tt_alloc_page_directory(struct ttm_tt *ttm)
>> >        ttm->pages = kvmalloc_array(ttm->num_pages,
>> > sizeof(*ttm->pages) +
>> > sizeof(*ttm->dma_address),
>> > -                                 GFP_KERNEL | __GFP_ZERO);
>> > + GFP_KERNEL);
>> >        if (!ttm->pages)
>> >                return -ENOMEM;
>> >
>> > +     memset(ttm->pages, 0, ttm->num_pages * (sizeof(*ttm->pages) + 
>> sizeof(*ttm->dma_address)));
>> > +
>> >        ttm->dma_address = (void *)(ttm->pages + ttm->num_pages);
>> >        return 0;
>> >   }
>> > @@ -120,9 +125,12 @@ static int 
>> ttm_sg_tt_alloc_page_directory(struct ttm_tt *ttm)
>> >   {
>> >        ttm->dma_address = kvmalloc_array(ttm->num_pages,
>> > sizeof(*ttm->dma_address),
>> > - GFP_KERNEL | __GFP_ZERO);
>> > + GFP_KERNEL);
>> >        if (!ttm->dma_address)
>> >                return -ENOMEM;
>> > +
>> > +     memset(ttm->dma_address, 0, ttm->num_pages * 
>> sizeof(*ttm->dma_address));
>> > +
>> >        return 0;
>> >   }
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220420/86abb2ba/attachment-0001.htm>