Am 12.01.21 um 16:59 schrieb Andrey Grodzovsky:
On 1/12/21 7:32 AM, Christian König wrote:
Am 12.01.21 um 10:10 schrieb Daniel Vetter:
On Mon, Jan 11, 2021 at 03:45:10PM -0500, Andrey Grodzovsky wrote:
On 1/11/21 11:15 AM, Daniel Vetter wrote:
On Mon, Jan 11, 2021 at 05:13:56PM +0100, Daniel Vetter wrote:
On Fri, Jan 08, 2021 at 04:49:55PM +0000, Grodzovsky, Andrey wrote:
Ok then, I guess I will proceed with the dummy pages list
implementation then.
Andrey
________________________________
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: 08 January 2021 09:52
To: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>; Daniel
Vetter <daniel@xxxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
<amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx
<dri-devel@xxxxxxxxxxxxxxxxxxxxx>; daniel.vetter@xxxxxxxx
<daniel.vetter@xxxxxxxx>; robh@xxxxxxxxxx <robh@xxxxxxxxxx>;
l.stach@xxxxxxxxxxxxxx <l.stach@xxxxxxxxxxxxxx>;
yuq825@xxxxxxxxx <yuq825@xxxxxxxxx>; eric@xxxxxxxxxx
<eric@xxxxxxxxxx>; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx
<gregkh@xxxxxxxxxxxxxxxxxxx>; ppaalanen@xxxxxxxxx
<ppaalanen@xxxxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>
Subject: Re: [PATCH v3 01/12] drm: Add dummy page per device or
GEM object
Mhm, I'm not aware of any let over pointer between TTM and GEM
and we
worked quite hard on reducing the size of the amdgpu_bo, so another
extra pointer just for that corner case would suck quite a bit.
We have a ton of other pointers in struct amdgpu_bo (or any of
it's lower
things) which are fairly single-use, so I'm really not much
seeing the
point in making this a special case. It also means the lifetime
management
becomes a bit iffy, since we can't throw away the dummy page then
the last
reference to the bo is released (since we don't track it there),
but only
when the last pointer to the device is released. Potentially this
means a
pile of dangling pages hanging around for too long.
Also if you really, really, really want to have this list, please
don't
reinvent it since we have it already. drmm_ is exactly meant for
resources
that should be freed when the final drm_device reference disappears.
-Daniel
I maybe was eager to early, see i need to explicitly allocate the
dummy page
using page_alloc so
i cannot use drmm_kmalloc for this, so once again like with the
list i need
to wrap it with a container struct
which i can then allocate using drmm_kmalloc and inside there will
be page
pointer. But then
on release it needs to free the page and so i supposedly need to
use drmm_add_action
to free the page before the container struct is released but
drmm_kmalloc
doesn't allow to set
release action on struct allocation. So I created a new
drmm_kmalloc_with_action API function
but then you also need to supply the optional data pointer for the
release
action (the struct page in this case)
and so this all becomes a bit overcomplicated (but doable). Is this
extra
API worth adding ? Maybe it can
be useful in general.
drm_add_action_or_reset (for better control flow) has both a void *
data
and a cleanup function (and it internally allocates the tracking
structure
for that for you). So should work as-is? Allocating a tracking
structure
for our tracking structure for a page would definitely be a bit too
much.
Essentiall drmm_add_action is your kcalloc_with_action function you
want,
as long as all you need is a single void * pointer (we could do the
kzalloc_with_action though, there's enough space, just no need yet
for any
of the current users).
Yeah, but my thinking was that we should use the page LRU for this
and not another container structure.
Christian.
Which specific list did you mean ?
The struct page * you get from get_free_page() already has an lru member
of type list_head.
This way you can link pages together for later destruction without the
need of a container object.
Christian.
Andrey
-Daniel
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
