From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: Huang Rui <ray.huang@amd.com>,
dri-devel <dri-devel@lists.freedesktop.org>,
efault@gmx.de
Subject: Re: [PATCH] drm/ttm: make sure pool pages are cleared
Date: Thu, 11 Feb 2021 16:58:05 +0100 [thread overview]
Message-ID: <YCVUDYYQJ4PtwzVI@phenom.ffwll.local> (raw)
In-Reply-To: <6ea6a18b-e7df-ce8c-903d-a5edd6ef85b6@gmail.com>
On Wed, Feb 10, 2021 at 09:23:52PM +0100, Christian König wrote:
>
>
> Am 10.02.21 um 19:15 schrieb Daniel Vetter:
> > On Wed, Feb 10, 2021 at 5:05 PM Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> > > The old implementation wasn't consistend on this.
> > >
> > > But it looks like we depend on this so better bring it back.
> > >
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > Reported-and-tested-by: Mike Galbraith <efault@gmx.de>
> > > Fixes: d099fc8f540a ("drm/ttm: new TT backend allocation pool v3")
> > Well I think in general there's a bit an issue in ttm with not
> > clearing stuff everywhere. So definitely in favour of clearing stuff.
> > Looking at the code this only fixes the clearing, at alloc time we're
> > still very optional with clearing. I think we should just set
> > __GFP_ZERO unconditionally there too.
>
> No, the alloc handling is actually correct.
>
> We are clearing only when we allocate pages for userspace. Not for the
> kernel nor for eviction when pages are overwritten anyway.
>
> The key point is that the old page pool was ignoring the flag for this in
> some code paths and I wasn't sure if that's still necessary or not.
>
> Turned out it was necessary after all.
Somehow my git grep went wrong and I didn't find the users. You're right,
and I learned a few things more :-)
I'm kinda wondering, should we perhaps move the clearing to the use side,
and then only do when required? Might allow us to save it quite a few
times when we're thrashing around buffers in/out of vram, at the cost of
moving it to the alloc side for other cases.
Just an idea.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-02-11 15:58 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-10 16:05 [PATCH] drm/ttm: make sure pool pages are cleared Christian König
2021-02-10 18:15 ` Daniel Vetter
2021-02-10 20:23 ` Christian König
2021-02-11 15:58 ` Daniel Vetter [this message]
2021-02-11 16:04 ` Christian König
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=YCVUDYYQJ4PtwzVI@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=efault@gmx.de \
--cc=ray.huang@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.