All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: zhoucm1 <zhoucm1@amd.com>,
	christian.koenig@amd.com, "Zhou,
	David(ChunMing)" <David1.Zhou@amd.com>,
	"Liang, Prike" <Prike.Liang@amd.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] ttm: wait mem space if user allow while gpu busy
Date: Wed, 24 Apr 2019 09:30:12 +0200	[thread overview]
Message-ID: <8db16fe8-78fe-7f25-4acd-51e37e645f3b@gmail.com> (raw)
In-Reply-To: <b57d94e6-6e20-d809-7ac2-89a39a57a365@amd.com>


[-- Attachment #1.1: Type: text/plain, Size: 6592 bytes --]

That would change the semantics of ttm_bo_mem_space() and so could 
change the return code in an IOCTL as well. Not a good idea, cause that 
could have a lot of side effects.

Instead in the calling DC code you could check if you get an -ENOMEM and 
then call schedule().

If after the schedule() we see that we have now BOs on the LRU we can 
try again and see if pinning the frame buffer now succeeds.

Christian.

Am 24.04.19 um 09:17 schrieb zhoucm1:
>
> I made a patch as attached.
>
> I'm not sure how to make patch as your proposal, Could you make a 
> patch for that if mine isn't enough?
>
> -David
>
> On 2019年04月24日 15:12, Christian König wrote:
>>> how about just adding a wrapper for pin function as below?
>> I considered this as well and don't think it will work reliable.
>>
>> We could use it as a band aid for this specific problem, but in 
>> general we need to improve the handling in TTM to resolve those kind 
>> of resource conflicts.
>>
>> Regards,
>> Christian.
>>
>> Am 23.04.19 um 17:09 schrieb Zhou, David(ChunMing):
>>> >3. If we have a ticket we grab a reference to the first BO on the 
>>> LRU, drop the LRU lock and try to grab the reservation lock with the 
>>> ticket.
>>>
>>> The BO on LRU is already locked by cs user, can it be dropped here 
>>> by DC user? and then DC user grab its lock with ticket, how does CS 
>>> grab it again?
>>>
>>> If you think waiting in ttm has this risk, how about just adding a 
>>> wrapper for pin function as below?
>>> amdgpu_get_pin_bo_timeout()
>>> {
>>> do {
>>> amdgpo_bo_reserve();
>>> r=amdgpu_bo_pin();
>>>
>>> if(!r)
>>>         break;
>>> amdgpu_bo_unreserve();
>>> timeout--;
>>>
>>> } while(timeout>0);
>>>
>>> }
>>>
>>> -------- Original Message --------
>>> Subject: Re: [PATCH] ttm: wait mem space if user allow while gpu busy
>>> From: Christian König
>>> To: "Zhou, David(ChunMing)" ,"Koenig, Christian" ,"Liang, Prike" 
>>> ,dri-devel@lists.freedesktop.org
>>> CC:
>>>
>>> Well that's not so easy of hand.
>>>
>>> The basic problem here is that when you busy wait at this place you 
>>> can easily run into situations where application A busy waits for B 
>>> while B busy waits for A -> deadlock.
>>>
>>> So what we need here is the deadlock detection logic of the 
>>> ww_mutex. To use this we at least need to do the following steps:
>>>
>>> 1. Reserve the BO in DC using a ww_mutex ticket (trivial).
>>>
>>> 2. If we then run into this EBUSY condition in TTM check if the BO 
>>> we need memory for (or rather the ww_mutex of its reservation 
>>> object) has a ticket assigned.
>>>
>>> 3. If we have a ticket we grab a reference to the first BO on the 
>>> LRU, drop the LRU lock and try to grab the reservation lock with the 
>>> ticket.
>>>
>>> 4. If getting the reservation lock with the ticket succeeded we 
>>> check if the BO is still the first one on the LRU in question (the 
>>> BO could have moved).
>>>
>>> 5. If the BO is still the first one on the LRU in question we try to 
>>> evict it as we would evict any other BO.
>>>
>>> 6. If any of the "If's" above fail we just back off and return -EBUSY.
>>>
>>> Steps 2-5 are certainly not trivial, but doable as far as I can see.
>>>
>>> Have fun :)
>>> Christian.
>>>
>>> Am 23.04.19 um 15:19 schrieb Zhou, David(ChunMing):
>>>> How about adding more condition ctx->resv inline to address your 
>>>> concern? As well as don't wait from same user, shouldn't lead to 
>>>> deadlock.
>>>>
>>>> Otherwise, any other idea?
>>>>
>>>> -------- Original Message --------
>>>> Subject: Re: [PATCH] ttm: wait mem space if user allow while gpu busy
>>>> From: Christian König
>>>> To: "Liang, Prike" ,"Zhou, David(ChunMing)" 
>>>> ,dri-devel@lists.freedesktop.org
>>>> CC:
>>>>
>>>> Well that is certainly a NAK because it can lead to deadlock in the
>>>> memory management.
>>>>
>>>> You can't just busy wait with all those locks held.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> Am 23.04.19 um 03:45 schrieb Liang, Prike:
>>>> > Acked-by: Prike Liang <Prike.Liang@amd.com>
>>>> >
>>>> > Thanks,
>>>> > Prike
>>>> > -----Original Message-----
>>>> > From: Chunming Zhou <david1.zhou@amd.com>
>>>> > Sent: Monday, April 22, 2019 6:39 PM
>>>> > To: dri-devel@lists.freedesktop.org
>>>> > Cc: Liang, Prike <Prike.Liang@amd.com>; Zhou, David(ChunMing) 
>>>> <David1.Zhou@amd.com>
>>>> > Subject: [PATCH] ttm: wait mem space if user allow while gpu busy
>>>> >
>>>> > heavy gpu job could occupy memory long time, which could lead to 
>>>> other user fail to get memory.
>>>> >
>>>> > Change-Id: I0b322d98cd76e5ac32b00462bbae8008d76c5e11
>>>> > Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>>> > ---
>>>> >   drivers/gpu/drm/ttm/ttm_bo.c | 6 ++++--
>>>> >   1 file changed, 4 insertions(+), 2 deletions(-)
>>>> >
>>>> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
>>>> b/drivers/gpu/drm/ttm/ttm_bo.c index 7c484729f9b2..6c596cc24bec 100644
>>>> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> > @@ -830,8 +830,10 @@ static int ttm_bo_mem_force_space(struct 
>>>> ttm_buffer_object *bo,
>>>> >                if (mem->mm_node)
>>>> >                        break;
>>>> >                ret = ttm_mem_evict_first(bdev, mem_type, place, ctx);
>>>> > -             if (unlikely(ret != 0))
>>>> > -                     return ret;
>>>> > +             if (unlikely(ret != 0)) {
>>>> > +                     if (!ctx || ctx->no_wait_gpu || ret != -EBUSY)
>>>> > +                             return ret;
>>>> > +             }
>>>> >        } while (1);
>>>> >        mem->mem_type = mem_type;
>>>> >        return ttm_bo_add_move_fence(bo, man, mem);
>>>> > --
>>>> > 2.17.1
>>>> >
>>>> > _______________________________________________
>>>> > dri-devel mailing list
>>>> > dri-devel@lists.freedesktop.org
>>>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


[-- Attachment #1.2: Type: text/html, Size: 13487 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-04-24  7:30 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-22 10:38 [PATCH] ttm: wait mem space if user allow while gpu busy Chunming Zhou
2019-04-23  1:45 ` Liang, Prike
2019-04-23 13:05   ` Christian König
2019-04-23 13:19     ` Zhou, David(ChunMing)
2019-04-23 14:42       ` [PATCH] " Christian König
2019-04-23 15:09         ` Zhou, David(ChunMing)
2019-04-24  7:12           ` [PATCH] " Christian König
2019-04-24  7:17             ` zhoucm1
2019-04-24  7:30               ` Christian König [this message]
2019-04-24  7:59                 ` zhoucm1
2019-04-24  8:07                   ` Christian König
2019-04-24  8:11                     ` zhoucm1
2019-04-24  8:59                       ` Koenig, Christian
2019-04-24  9:03                         ` zhoucm1
2019-04-24 10:03                           ` Christian König
2019-04-24  8:01         ` Daniel Vetter
2019-04-24  8:06           ` Koenig, Christian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8db16fe8-78fe-7f25-4acd-51e37e645f3b@gmail.com \
    --to=ckoenig.leichtzumerken@gmail.com \
    --cc=David1.Zhou@amd.com \
    --cc=Prike.Liang@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=zhoucm1@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.