Compound page metadata is necessary for page reference counting to work properly on pages besides the head page. Without it, put_page corresponding to the last outstanding get_page call on a tail page will end up freeing that page, even if the bo still references the page.
Signed-off-by: David Stevens stevensd@chromium.org --- drivers/gpu/drm/ttm/ttm_pool.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index 7b2f60616750..09239b93dc2c 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -83,7 +83,6 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags, gfp_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM; gfp_flags &= ~__GFP_MOVABLE; - gfp_flags &= ~__GFP_COMP; }
if (!pool->use_dma_alloc) {
Am 13.01.21 um 09:47 schrieb David Stevens:
Compound page metadata is necessary for page reference counting to work properly on pages besides the head page. Without it, put_page corresponding to the last outstanding get_page call on a tail page will end up freeing that page, even if the bo still references the page.
NAK, I should add a comment since I can't count any more how many times people came up with this.
Calling put_page() on a TTM allocated page is strictly illegal in the first place since they are not reference counted.
Please don't tell me somebody once more tried to mmap() pages from a DMA-buf created sg_table into a process address space?
Regards, Christian.
Signed-off-by: David Stevens stevensd@chromium.org
drivers/gpu/drm/ttm/ttm_pool.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index 7b2f60616750..09239b93dc2c 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -83,7 +83,6 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags, gfp_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM; gfp_flags &= ~__GFP_MOVABLE;
gfp_flags &= ~__GFP_COMP;}
if (!pool->use_dma_alloc) {
On Wed, Jan 13, 2021 at 5:58 PM Christian König christian.koenig@amd.com wrote:
Am 13.01.21 um 09:47 schrieb David Stevens:
Compound page metadata is necessary for page reference counting to work properly on pages besides the head page. Without it, put_page corresponding to the last outstanding get_page call on a tail page will end up freeing that page, even if the bo still references the page.
NAK, I should add a comment since I can't count any more how many times people came up with this.
Calling put_page() on a TTM allocated page is strictly illegal in the first place since they are not reference counted.
Please don't tell me somebody once more tried to mmap() pages from a DMA-buf created sg_table into a process address space?
I ran into this on a system using the currently in development virtio-gpu blob resources [1]. The implementation passes dma buffers allocated by the host gpu to KVM_SET_USER_MEMORY_REGION, so the guest can directly access the buffers without the need for an intermediate copy.
[1] https://patchwork.kernel.org/project/dri-devel/cover/20200814024000.2485-1-g...
Regards, Christian.
Signed-off-by: David Stevens stevensd@chromium.org
drivers/gpu/drm/ttm/ttm_pool.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index 7b2f60616750..09239b93dc2c 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -83,7 +83,6 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags, gfp_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM; gfp_flags &= ~__GFP_MOVABLE;
gfp_flags &= ~__GFP_COMP; } if (!pool->use_dma_alloc) {
Am 13.01.21 um 10:33 schrieb David Stevens:
On Wed, Jan 13, 2021 at 5:58 PM Christian König christian.koenig@amd.com wrote:
Am 13.01.21 um 09:47 schrieb David Stevens:
Compound page metadata is necessary for page reference counting to work properly on pages besides the head page. Without it, put_page corresponding to the last outstanding get_page call on a tail page will end up freeing that page, even if the bo still references the page.
NAK, I should add a comment since I can't count any more how many times people came up with this.
Calling put_page() on a TTM allocated page is strictly illegal in the first place since they are not reference counted.
Please don't tell me somebody once more tried to mmap() pages from a DMA-buf created sg_table into a process address space?
I ran into this on a system using the currently in development virtio-gpu blob resources [1]. The implementation passes dma buffers allocated by the host gpu to KVM_SET_USER_MEMORY_REGION, so the guest can directly access the buffers without the need for an intermediate copy.
Yeah, that is completely illegal. They risk memory corruption with that.
You should probably report back to the author of those patches and not that the dma_buf_mmap() functions need to be used for this use case.
Regards, Christian.
[1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork....
Regards, Christian.
Signed-off-by: David Stevens stevensd@chromium.org
drivers/gpu/drm/ttm/ttm_pool.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index 7b2f60616750..09239b93dc2c 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -83,7 +83,6 @@ static struct page *ttm_pool_alloc_page(struct ttm_pool *pool, gfp_t gfp_flags, gfp_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM; gfp_flags &= ~__GFP_MOVABLE;
gfp_flags &= ~__GFP_COMP; } if (!pool->use_dma_alloc) {
dri-devel@lists.freedesktop.org
-
Christian König -
David Stevens