Re: [PATCH v3 01/12] drm: Add dummy page per device or GEM object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux