Am 14.04.21 um 14:25 schrieb Daniel Vetter:
On Wed, Apr 14, 2021 at 12:49 PM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
Am 14.04.21 um 12:26 schrieb Daniel Vetter:
On Wed, Apr 14, 2021 at 11:19:41AM +0200, Christian König wrote:
Am 14.04.21 um 11:15 schrieb Daniel Vetter:
On Wed, Apr 14, 2021 at 08:51:51AM +0200, Christian König wrote:
Am 14.04.21 um 08:48 schrieb Felix Kuehling:
Pages in SG BOs were not allocated by TTM. So don't count them against
TTM's pages limit.
Signed-off-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Reviewed-by: Christian König <christian.koenig@xxxxxxx>
Going to pick that one up for inclusion in drm-misc-next.
See my other email, but why do we need this? A bit more explanation is imo
needed here at least, since we still need to guarantee that allocations
don't over the limit in total for all gpu buffers together. At least until
the shrinker has landed.
And this here just opens up the barn door without any explanation why it's
ok.
The SG based BOs might not even be backed by pages. E.g. exported VRAM.
So either they are exported by a driver which should have accounted for the
allocation, exported by TTM which already did the accounting or doesn't even
point to pages at all.
This is really a bug fix to recreate the behavior we had before moving the
accounting to this place.
Throw that into the commit message and a-b: me. Ideally with a Fixes: line
or so pointing at the offending commit that broke stuff. Commit messages
should really go into more detail when there's an entire story behind a
small change like this one.
Sorry I though that this would be obvious :)
I've already pushed the patch in the morning, but going to keep that in
mind for the next time.
I'll keep reminding you to pls elaborate more in commit messages, it's
coming up every once in a while :-)
Well, describing stuff in a commit message which is obvious is just
redundant.
I can of course explain the whole background of the code in question in
the commit message, but for obvious bug fixes like this it is just overkill.
Also in general I think a few days of letting patches soak out there,
especially shared code, is good curtesy. Some folks demand 2 weeks,
which I think is too much, but less than 24h just means you're
guaranteed to leave out half the globe with their feedback. Which
isn't great.
Well for structural changes I certainly agree, but not for a bug fix
which adds a missing check for a special case.
Christian.
Driver code I don't care since there you know all the stakeholders ofc.
-Daniel
Christian.
-Daniel
Christian.
-Daniel
Regards,
Christian.
---
drivers/gpu/drm/ttm/ttm_tt.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 5d8820725b75..e8b8c3257392 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -317,9 +317,12 @@ int ttm_tt_populate(struct ttm_device *bdev,
if (ttm_tt_is_populated(ttm))
return 0;
- atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
- if (bdev->pool.use_dma32)
- atomic_long_add(ttm->num_pages, &ttm_dma32_pages_allocated);
+ if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+ atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
+ if (bdev->pool.use_dma32)
+ atomic_long_add(ttm->num_pages,
+ &ttm_dma32_pages_allocated);
+ }
while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
atomic_long_read(&ttm_dma32_pages_allocated) >
@@ -350,9 +353,12 @@ int ttm_tt_populate(struct ttm_device *bdev,
return 0;
error:
- atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
- if (bdev->pool.use_dma32)
- atomic_long_sub(ttm->num_pages, &ttm_dma32_pages_allocated);
+ if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+ atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
+ if (bdev->pool.use_dma32)
+ atomic_long_sub(ttm->num_pages,
+ &ttm_dma32_pages_allocated);
+ }
return ret;
}
EXPORT_SYMBOL(ttm_tt_populate);
@@ -382,9 +388,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm)
else
ttm_pool_free(&bdev->pool, ttm);
- atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
- if (bdev->pool.use_dma32)
- atomic_long_sub(ttm->num_pages, &ttm_dma32_pages_allocated);
+ if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+ atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
+ if (bdev->pool.use_dma32)
+ atomic_long_sub(ttm->num_pages,
+ &ttm_dma32_pages_allocated);
+ }
ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
}
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cchristian.koenig%40amd.com%7C503f240d409740c1333508d8ff406545%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637539999355330481%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=6sW53%2FGpxk4rZKM7mpHDfgBhreCXY4McypKGqTH13b8%3D&reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
