Re: [PATCH 8/9] drm/ttm: Don't count pages in SG BOs against pages_limit

From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>,
	dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
	christian.koenig@amd.com
Subject: Re: [PATCH 8/9] drm/ttm: Don't count pages in SG BOs against pages_limit
Date: Wed, 14 Apr 2021 11:15:20 +0200	[thread overview]
Message-ID: <YHayqD3IbupkY7b+@phenom.ffwll.local> (raw)
In-Reply-To: <60d08770-e4f5-4e8f-0a93-2270ae02aa26@gmail.com>

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@amd.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
> 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.
-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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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