From: "Christian König" <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
To: "Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>,
"John Brooks" <john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org>
Cc: "Marek Olšák" <maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM
Date: Mon, 3 Jul 2017 09:06:53 +0200 [thread overview]
Message-ID: <894e892e-3076-9f52-5dba-71ac135b7d61@vodafone.de> (raw)
In-Reply-To: <3a28996f-2a7c-918c-6c39-65a25d4ea976-otUistvHUpPR7s880joybQ@public.gmane.org>
Am 03.07.2017 um 03:34 schrieb Michel Dänzer:
> On 02/07/17 09:52 PM, Christian König wrote:
>> Am 30.06.2017 um 17:18 schrieb John Brooks:
>>> When a BO is moved to VRAM, clear AMDGPU_BO_FLAG_CPU_ACCESS. This
>>> allows it
>>> to potentially later move to invisible VRAM if the CPU does not access it
>>> again.
>>>
>>> Setting the CPU_ACCESS flag in amdgpu_fault_reserve_notify() also means
>>> that we can remove the loop to restrict lpfn to the end of visible VRAM,
>>> because amdgpu_ttm_placement_init() will do it for us.
>>>
>>> Signed-off-by: John Brooks <john@fastquake.com>
> [...]
>
>>> @@ -446,6 +448,12 @@ static int amdgpu_move_ram_vram(struct
>>> ttm_buffer_object *bo,
>>> if (unlikely(r)) {
>>> goto out_cleanup;
>>> }
>>> +
>>> + /* The page fault handler will re-set this if the CPU accesses
>>> the BO
>>> + * after it's moved.
>>> + */
> Maybe say "amdgpu_bo_fault_reserve_notify" explicitly here instead of
> "The page fault handler".
>
>
>>> + abo->flags &= ~AMDGPU_BO_FLAG_CPU_ACCESS;
>>> +
>> This is the wrong place for clearing the flag. This code path is only
>> called when we move things back in after suspend/resume (or run out of
>> GTT space).
> Surely amdgpu_move_ram_vram is called whenever a BO is moved to VRAM,
No, that isn't even remotely correct. amdgpu_move_ram_vram() is only
called when the BO is moved directly from the system domain to the VRAM
domain.
Normally BOs are only moved from the GTT domain to the VRAM domain,
except after resume and when we ran out of GTT space.
> for any reason. I suggested clearing the flag here to John on IRC. The
> idea is briefly described in the commit log, let me elaborate a bit on that:
>
> When a BO is moved to VRAM which has the AMDGPU_BO_FLAG_CPU_ACCESS flag
> set, it is put in CPU visible VRAM, and the flag is cleared. If the CPU
> doesn't access the BO, the next time it will be moved to VRAM (after it
> was evicted from there, for any reason), the flag will no longer be set,
> and the BO will likely be moved to CPU invisible VRAM.
>
> If the BO is accessed by the CPU again though (no matter where the BO is
> currently located at that time), the flag is set again, and the cycle
> from the previous paragraph starts over.
>
> The end result should be similar as with the timestamp based solution in
> John's earlier series: BOs which are at least occasionally accessed by
> the CPU will tend to be in CPU visible VRAM, those which are never
> accessed by the CPU can be in CPU invisible VRAM.
Yeah, I understand the intention. But the implementation isn't even
remotely correct.
First of all the flag must be cleared in the CS which wants to move the
BO, not in the move functions when the decision where to put it is
already made.
Second currently the flag is set on page fault, but never cleared
because the place where the code to clear it was added is just
completely incorrect (see above).
Instead of messing with all this I suggest that we just add a jiffies
based timeout to the BO when we can clear the flag. For kernel BOs this
timeout is just infinity.
Then we check in amdgpu_cs_bo_validate() before generating the
placements if we could clear the flag and do so based on the timeout.
I can help implementing this when I'm done getting ride of the BO move
size limitation (swapped all of this stuff for that task back into my
brain anyway).
Regards,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2017-07-03 7:06 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-29 14:41 Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED Marek Olšák
[not found] ` <CAAxE2A4x8tkA9DL78EouJqxZ9b2b_PyzN3hg8JJn7AHOpk5b0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-29 14:51 ` Christian König
[not found] ` <8f866ee1-78c0-6707-3f92-b71a2f661457-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-06-29 15:03 ` Marek Olšák
[not found] ` <CAAxE2A49_j6hO_G0XoWwquFzPsoHVELNhakt5NdmVb8=F0YdSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-30 1:36 ` Michel Dänzer
[not found] ` <50b66561-3843-8b70-913e-5692db657640-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-06-30 6:44 ` Christian König
[not found] ` <b7717c04-604f-24c8-2fc7-a94e520845c8-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-06-30 6:51 ` Michel Dänzer
[not found] ` <a1c1f000-16b4-220f-70c5-01b2682a632e-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-06-30 6:59 ` Christian König
[not found] ` <8f60b400-1343-9f7f-d202-f3497fb346ae-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-06-30 7:14 ` Michel Dänzer
[not found] ` <160855e9-2517-ae04-fd9d-c638507109cd-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-06-30 10:34 ` Christian König
[not found] ` <a5afda83-1c47-a8d2-20ab-a5923603e7fb-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-06-30 11:43 ` Marek Olšák
[not found] ` <CAAxE2A6h=0cn_+S9Psw=DkvAqtUdAOZE-kK39LZFpX0xud1gRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-03 10:08 ` Michel Dänzer
[not found] ` <1bb94040-5d80-d1e7-33c9-b09e5282b91c-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-07-03 13:03 ` Marek Olšák
[not found] ` <CAAxE2A4dNoEi=D8FM=+5kEZmq_V7DdLrwFPYxSFx4uFjUSeTvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-07-04 8:09 ` Michel Dänzer
[not found] ` <cf4352ec-6838-f599-cf5c-b6a123301461-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-07-04 13:39 ` Marek Olšák
2017-06-30 15:18 ` [PATCH RFC 0/2] " John Brooks
[not found] ` <1498835932-5053-1-git-send-email-john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org>
2017-06-30 15:18 ` [PATCH RFC 1/2] drm/amdgpu: Add AMDGPU_BO_FLAG_CPU_ACCESS John Brooks
[not found] ` <1498835932-5053-2-git-send-email-john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org>
2017-07-01 15:36 ` Christian König
[not found] ` <33d2358f-b1c8-d774-fe43-19a024c79d24-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-01 16:25 ` [PATCH RFC v2 " John Brooks
2017-06-30 15:18 ` [PATCH RFC 2/2] drm/amdgpu: Set/clear CPU_ACCESS flag on page fault and move to VRAM John Brooks
[not found] ` <1498835932-5053-3-git-send-email-john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org>
2017-06-30 15:31 ` [PATCH RFC v2] " John Brooks
[not found] ` <1498836668-5872-1-git-send-email-john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org>
2017-07-03 9:56 ` Michel Dänzer
2017-07-02 12:52 ` [PATCH RFC 2/2] " Christian König
[not found] ` <4236c636-8b80-7088-6b55-8b9dee1c2b3f-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-03 1:34 ` Michel Dänzer
[not found] ` <3a28996f-2a7c-918c-6c39-65a25d4ea976-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-07-03 7:06 ` Christian König [this message]
[not found] ` <894e892e-3076-9f52-5dba-71ac135b7d61-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-03 9:49 ` Michel Dänzer
[not found] ` <45ae166f-70f5-d05e-a5bb-99429b3de063-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-07-03 11:47 ` Christian König
[not found] ` <bf62998e-0174-f714-9503-d99295213d8f-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-07-04 0:56 ` Michel Dänzer
2017-06-30 1:55 ` Deprecation of AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED Mao, David
[not found] ` <FE84B835-C930-424E-B380-649F4895274C-5C7GfCeVMHo@public.gmane.org>
2017-06-30 3:36 ` Michel Dänzer
[not found] ` <17988a78-85c4-24c2-961a-b07f7222446f-otUistvHUpPR7s880joybQ@public.gmane.org>
2017-06-30 4:00 ` Mao, David
[not found] ` <0C38ED78-D50C-4212-AA7A-0DACBFDB5680-5C7GfCeVMHo@public.gmane.org>
2017-06-30 7:02 ` Michel Dänzer
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=894e892e-3076-9f52-5dba-71ac135b7d61@vodafone.de \
--to=deathsimple-antagkrnahcb1svskn2v4q@public.gmane.org \
--cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=john-xq/Ko7C6e2Bl57MIdRCFDg@public.gmane.org \
--cc=maraeo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=michel-otUistvHUpPR7s880joybQ@public.gmane.org \
/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.