This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over-accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over-accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over-accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over-accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote:
This is an RFC. I'm not sure this is the right solution, but it highlights the problem I'm trying to solve.
The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a 64-bit system with hundreds of GB of system memory and GPU memory, this can become a bottle neck. We're seeing TTM memory allocation failures not because we're truly out of memory, but because we're out of space in the dma32_zone for the acc_size needed for our BO book-keeping.
Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com CC: thellstrom@vmware.com CC: christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index f1567c3..bb05365 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob, glob->zones[glob->num_zones++] = zone; return 0; } -#else +#elifndef CONFIG_64BIT static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob, const struct sysinfo *si) { @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) ret = ttm_mem_init_highmem_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone; -#else +#elifndef CONFIG_64BIT ret = ttm_mem_init_dma32_zone(glob, &si); if (unlikely(ret != 0)) goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
/Thomas
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over- accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
Hmm,
This zone was intended to stop TTM page allocations from exhausting the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means if we drop this check, other devices may stop functioning unexpectedly?
However, in the end I'd expect the kernel page allocation system to make sure there are some pages left in the DMA32 zone, otherwise random non-IO page allocations would also potentially exhaust the DMA32 zone without anybody caring, which means removing this zone wouldn't be any worse than whatever other subsystems may be doing already...
/Thomas
On 2/16/19 12:02 AM, Kuehling, Felix wrote: > This is an RFC. I'm not sure this is the right solution, > but > it > highlights the problem I'm trying to solve. > > The dma32_zone limits the acc_size of all allocated BOs > to > 2GB. > On a > 64-bit system with hundreds of GB of system memory and > GPU > memory, > this can become a bottle neck. We're seeing TTM memory > allocation > failures not because we're truly out of memory, but > because > we're > out of space in the dma32_zone for the acc_size needed > for > our BO > book-keeping. > > Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com > CC: thellstrom@vmware.com > CC: christian.koenig@amd.com > --- > drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > b/drivers/gpu/drm/ttm/ttm_memory.c > index f1567c3..bb05365 100644 > --- a/drivers/gpu/drm/ttm/ttm_memory.c > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > @@ -363,7 +363,7 @@ static int > ttm_mem_init_highmem_zone(struct > ttm_mem_global *glob, > glob->zones[glob->num_zones++] = zone; > return 0; > } > -#else > +#elifndef CONFIG_64BIT > static int ttm_mem_init_dma32_zone(struct > ttm_mem_global > *glob, > const struct sysinfo *si) > { > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct > ttm_mem_global > *glob) > ret = ttm_mem_init_highmem_zone(glob, &si); > if (unlikely(ret != 0)) > goto out_no_zone; > -#else > +#elifndef CONFIG_64BIT > ret = ttm_mem_init_dma32_zone(glob, &si); > if (unlikely(ret != 0)) > goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
Yeah and exactly because of this it doesn't make to much sense to account the housekeeping stuff in both zones.
IIRC the kernel takes perfectly care by itself that kmalloced memory doesn't eat up everything in the DMA32 zone.
Christian.
/Thomas
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over- accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: > Hmm, > > This zone was intended to stop TTM page allocations from > exhausting > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by > default, > which > means if we drop this check, other devices may stop > functioning > unexpectedly? > > However, in the end I'd expect the kernel page allocation > system > to > make sure there are some pages left in the DMA32 zone, > otherwise > random non-IO page allocations would also potentially > exhaust > the > DMA32 zone without anybody caring, which means removing > this > zone > wouldn't be any worse than whatever other subsystems may be > doing > already... > > /Thomas > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote: >> This is an RFC. I'm not sure this is the right solution, >> but >> it >> highlights the problem I'm trying to solve. >> >> The dma32_zone limits the acc_size of all allocated BOs >> to >> 2GB. >> On a >> 64-bit system with hundreds of GB of system memory and >> GPU >> memory, >> this can become a bottle neck. We're seeing TTM memory >> allocation >> failures not because we're truly out of memory, but >> because >> we're >> out of space in the dma32_zone for the acc_size needed >> for >> our BO >> book-keeping. >> >> Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com >> CC: thellstrom@vmware.com >> CC: christian.koenig@amd.com >> --- >> drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c >> b/drivers/gpu/drm/ttm/ttm_memory.c >> index f1567c3..bb05365 100644 >> --- a/drivers/gpu/drm/ttm/ttm_memory.c >> +++ b/drivers/gpu/drm/ttm/ttm_memory.c >> @@ -363,7 +363,7 @@ static int >> ttm_mem_init_highmem_zone(struct >> ttm_mem_global *glob, >> glob->zones[glob->num_zones++] = zone; >> return 0; >> } >> -#else >> +#elifndef CONFIG_64BIT >> static int ttm_mem_init_dma32_zone(struct >> ttm_mem_global >> *glob, >> const struct sysinfo *si) >> { >> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct >> ttm_mem_global >> *glob) >> ret = ttm_mem_init_highmem_zone(glob, &si); >> if (unlikely(ret != 0)) >> goto out_no_zone; >> -#else >> +#elifndef CONFIG_64BIT >> ret = ttm_mem_init_dma32_zone(glob, &si); >> if (unlikely(ret != 0)) >> goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 2/20/19 9:07 AM, Christian König wrote:
Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote: > Another good question is also why the heck the acc_size > counts > towards > the DMA32 zone? DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
Yeah and exactly because of this it doesn't make to much sense to account the housekeeping stuff in both zones.
IMO it makes sense because (given the assumption that kmalloc/vmalloc doesn't care) , accounting only in DMA32 in low memory configurations will not guarantee that kernel is not exhausted. Accounting only in kernel in huge memory configurations will not guarantee that DMA32 is not exhausted.
IIRC the kernel takes perfectly care by itself that kmalloced memory doesn't eat up everything in the DMA32 zone.
If we can somehow verify this, or at least illustrate that it's likely, I'm perfectly fine with either patch from the vmwgfx POW.
Thanks, Thomas
Christian.
/Thomas
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over- accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
> In other words why should the internal bookkeeping pages be > allocated > in > the DMA32 zone? > > That doesn't sounds valid to me in any way, > Christian. > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: >> Hmm, >> >> This zone was intended to stop TTM page allocations from >> exhausting >> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by >> default, >> which >> means if we drop this check, other devices may stop >> functioning >> unexpectedly? >> >> However, in the end I'd expect the kernel page allocation >> system >> to >> make sure there are some pages left in the DMA32 zone, >> otherwise >> random non-IO page allocations would also potentially >> exhaust >> the >> DMA32 zone without anybody caring, which means removing >> this >> zone >> wouldn't be any worse than whatever other subsystems may be >> doing >> already... >> >> /Thomas >> >> >> On 2/16/19 12:02 AM, Kuehling, Felix wrote: >>> This is an RFC. I'm not sure this is the right solution, >>> but >>> it >>> highlights the problem I'm trying to solve. >>> >>> The dma32_zone limits the acc_size of all allocated BOs >>> to >>> 2GB. >>> On a >>> 64-bit system with hundreds of GB of system memory and >>> GPU >>> memory, >>> this can become a bottle neck. We're seeing TTM memory >>> allocation >>> failures not because we're truly out of memory, but >>> because >>> we're >>> out of space in the dma32_zone for the acc_size needed >>> for >>> our BO >>> book-keeping. >>> >>> Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com >>> CC: thellstrom@vmware.com >>> CC: christian.koenig@amd.com >>> --- >>> drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c >>> b/drivers/gpu/drm/ttm/ttm_memory.c >>> index f1567c3..bb05365 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_memory.c >>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c >>> @@ -363,7 +363,7 @@ static int >>> ttm_mem_init_highmem_zone(struct >>> ttm_mem_global *glob, >>> glob->zones[glob->num_zones++] = zone; >>> return 0; >>> } >>> -#else >>> +#elifndef CONFIG_64BIT >>> static int ttm_mem_init_dma32_zone(struct >>> ttm_mem_global >>> *glob, >>> const struct sysinfo *si) >>> { >>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct >>> ttm_mem_global >>> *glob) >>> ret = ttm_mem_init_highmem_zone(glob, &si); >>> if (unlikely(ret != 0)) >>> goto out_no_zone; >>> -#else >>> +#elifndef CONFIG_64BIT >>> ret = ttm_mem_init_dma32_zone(glob, &si); >>> if (unlikely(ret != 0)) >>> goto out_no_zone; _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 20.02.19 um 09:14 schrieb Thomas Hellstrom:
On 2/20/19 9:07 AM, Christian König wrote:
Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom: > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote: >> Another good question is also why the heck the acc_size >> counts >> towards >> the DMA32 zone? > DMA32 TTM pages are accounted in the DMA32 zone. Other pages > are > not. Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
Yeah and exactly because of this it doesn't make to much sense to account the housekeeping stuff in both zones.
IMO it makes sense because (given the assumption that kmalloc/vmalloc doesn't care) , accounting only in DMA32 in low memory configurations will not guarantee that kernel is not exhausted. Accounting only in kernel in huge memory configurations will not guarantee that DMA32 is not exhausted.
DMA32 is not exhausted by TTMs kmalloc/vmalloc because otherwise any other kmalloc/vmalloc could exhaust it as well.
I mean its probably 20+ years ago that I last looked at stuff in this part of the kernel, but I don't think we have ever changed the handling of DMA/DMA32 or otherwise we would be running into massive exhaustion problems already on modern systems.
IIRC the kernel takes perfectly care by itself that kmalloced memory doesn't eat up everything in the DMA32 zone.
If we can somehow verify this, or at least illustrate that it's likely, I'm perfectly fine with either patch from the vmwgfx POW.
Well I actually think that the whole accounting of housekeeping towards the memory zones is completely pointless in the first place. Even if we swap things out we still have the same BO and TTM structures around anyway.
What we should rather to is to make sure that all processes using a BO have their RSS increased because they do so. So when a process tries to exhaust a memory domain it is simply killed by the OOM killer.
Regards, Christian.
Thanks, Thomas
Christian.
/Thomas
Regards, Felix
/Thomas
Christian.
> For small persistent allocations using ttm_mem_global_alloc(), > they > are > accounted also in the DMA32 zone, which may cause over- > accounting > of > that zone, but that's pretty unlikely to be a big problem.. > > /Thomas > > > > > >> In other words why should the internal bookkeeping pages be >> allocated >> in >> the DMA32 zone? >> >> That doesn't sounds valid to me in any way, >> Christian. >> >> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: >>> Hmm, >>> >>> This zone was intended to stop TTM page allocations from >>> exhausting >>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by >>> default, >>> which >>> means if we drop this check, other devices may stop >>> functioning >>> unexpectedly? >>> >>> However, in the end I'd expect the kernel page allocation >>> system >>> to >>> make sure there are some pages left in the DMA32 zone, >>> otherwise >>> random non-IO page allocations would also potentially >>> exhaust >>> the >>> DMA32 zone without anybody caring, which means removing >>> this >>> zone >>> wouldn't be any worse than whatever other subsystems may be >>> doing >>> already... >>> >>> /Thomas >>> >>> >>> On 2/16/19 12:02 AM, Kuehling, Felix wrote: >>>> This is an RFC. I'm not sure this is the right solution, >>>> but >>>> it >>>> highlights the problem I'm trying to solve. >>>> >>>> The dma32_zone limits the acc_size of all allocated BOs >>>> to >>>> 2GB. >>>> On a >>>> 64-bit system with hundreds of GB of system memory and >>>> GPU >>>> memory, >>>> this can become a bottle neck. We're seeing TTM memory >>>> allocation >>>> failures not because we're truly out of memory, but >>>> because >>>> we're >>>> out of space in the dma32_zone for the acc_size needed >>>> for >>>> our BO >>>> book-keeping. >>>> >>>> Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com >>>> CC: thellstrom@vmware.com >>>> CC: christian.koenig@amd.com >>>> --- >>>> drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c >>>> b/drivers/gpu/drm/ttm/ttm_memory.c >>>> index f1567c3..bb05365 100644 >>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c >>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c >>>> @@ -363,7 +363,7 @@ static int >>>> ttm_mem_init_highmem_zone(struct >>>> ttm_mem_global *glob, >>>> glob->zones[glob->num_zones++] = zone; >>>> return 0; >>>> } >>>> -#else >>>> +#elifndef CONFIG_64BIT >>>> static int ttm_mem_init_dma32_zone(struct >>>> ttm_mem_global >>>> *glob, >>>> const struct sysinfo *si) >>>> { >>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct >>>> ttm_mem_global >>>> *glob) >>>> ret = ttm_mem_init_highmem_zone(glob, &si); >>>> if (unlikely(ret != 0)) >>>> goto out_no_zone; >>>> -#else >>>> +#elifndef CONFIG_64BIT >>>> ret = ttm_mem_init_dma32_zone(glob, &si); >>>> if (unlikely(ret != 0)) >>>> goto out_no_zone; > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed... >
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On Wed, 2019-02-20 at 08:35 +0000, Koenig, Christian wrote:
Am 20.02.19 um 09:14 schrieb Thomas Hellstrom:
On 2/20/19 9:07 AM, Christian König wrote:
Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote: > Am 18.02.19 um 10:47 schrieb Thomas Hellstrom: > > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian > > wrote: > > > Another good question is also why the heck the > > > acc_size > > > counts > > > towards > > > the DMA32 zone? > > DMA32 TTM pages are accounted in the DMA32 zone. Other > > pages > > are > > not. > Yeah, I'm perfectly aware of this. But this is for the > accounting > size! > > We have an accounting for the stuff needed additional to > the > pages > backing the BO (e.g. the page and DMA addr array). > > And from the bug description it sounds like we use the > DMA32 zone > for > this accounting which of course is completely nonsense. It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
Yeah and exactly because of this it doesn't make to much sense to account the housekeeping stuff in both zones.
IMO it makes sense because (given the assumption that kmalloc/vmalloc doesn't care) , accounting only in DMA32 in low memory configurations will not guarantee that kernel is not exhausted. Accounting only in kernel in huge memory configurations will not guarantee that DMA32 is not exhausted.
DMA32 is not exhausted by TTMs kmalloc/vmalloc because otherwise any other kmalloc/vmalloc could exhaust it as well.
I mean its probably 20+ years ago that I last looked at stuff in this part of the kernel, but I don't think we have ever changed the handling of DMA/DMA32 or otherwise we would be running into massive exhaustion problems already on modern systems.
IIRC the kernel takes perfectly care by itself that kmalloced memory doesn't eat up everything in the DMA32 zone.
If we can somehow verify this, or at least illustrate that it's likely, I'm perfectly fine with either patch from the vmwgfx POW.
Well I actually think that the whole accounting of housekeeping towards the memory zones is completely pointless in the first place. Even if we swap things out we still have the same BO and TTM structures around anyway.
Well, the whole point of this is to stop malicious user-space apps from exhausting memory, causing a DOS vulnerability, similaraly for example to how processes are stopped from creating to many open file descriptors or other kernel structures. IIRC, for example, you used to be able to call gem open in an infinite loop with the same bo until you brought a random big process down using the OOM killer. With vmwgfx you can't do that. You'd end up swapping out all available BOs and then you'd get an error.
What we should rather to is to make sure that all processes using a BO have their RSS increased because they do so. So when a process tries to exhaust a memory domain it is simply killed by the OOM killer.
Yes, if there is a way to reliably do that and also take care of other kernel structures that are created by user-space, like in the example above that indeed sounds like a better solution.
/Thomas
Regards, Christian.
Thanks, Thomas
Christian.
/Thomas
Regards, Felix
/Thomas
> Christian. > > > For small persistent allocations using > > ttm_mem_global_alloc(), > > they > > are > > accounted also in the DMA32 zone, which may cause over- > > accounting > > of > > that zone, but that's pretty unlikely to be a big > > problem.. > > > > /Thomas > > > > > > > > > > > > > In other words why should the internal bookkeeping > > > pages be > > > allocated > > > in > > > the DMA32 zone? > > > > > > That doesn't sounds valid to me in any way, > > > Christian. > > > > > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: > > > > Hmm, > > > > > > > > This zone was intended to stop TTM page allocations > > > > from > > > > exhausting > > > > the DMA32 zone. IIRC dma_alloc_coherent() uses > > > > DMA32 by > > > > default, > > > > which > > > > means if we drop this check, other devices may stop > > > > functioning > > > > unexpectedly? > > > > > > > > However, in the end I'd expect the kernel page > > > > allocation > > > > system > > > > to > > > > make sure there are some pages left in the DMA32 > > > > zone, > > > > otherwise > > > > random non-IO page allocations would also > > > > potentially > > > > exhaust > > > > the > > > > DMA32 zone without anybody caring, which means > > > > removing > > > > this > > > > zone > > > > wouldn't be any worse than whatever other > > > > subsystems may be > > > > doing > > > > already... > > > > > > > > /Thomas > > > > > > > > > > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote: > > > > > This is an RFC. I'm not sure this is the right > > > > > solution, > > > > > but > > > > > it > > > > > highlights the problem I'm trying to solve. > > > > > > > > > > The dma32_zone limits the acc_size of all > > > > > allocated BOs > > > > > to > > > > > 2GB. > > > > > On a > > > > > 64-bit system with hundreds of GB of system > > > > > memory and > > > > > GPU > > > > > memory, > > > > > this can become a bottle neck. We're seeing TTM > > > > > memory > > > > > allocation > > > > > failures not because we're truly out of memory, > > > > > but > > > > > because > > > > > we're > > > > > out of space in the dma32_zone for the acc_size > > > > > needed > > > > > for > > > > > our BO > > > > > book-keeping. > > > > > > > > > > Signed-off-by: Felix Kuehling < > > > > > Felix.Kuehling@amd.com> > > > > > CC: thellstrom@vmware.com > > > > > CC: christian.koenig@amd.com > > > > > --- > > > > > drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 > > > > > deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > > > > > b/drivers/gpu/drm/ttm/ttm_memory.c > > > > > index f1567c3..bb05365 100644 > > > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c > > > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > > > > > @@ -363,7 +363,7 @@ static int > > > > > ttm_mem_init_highmem_zone(struct > > > > > ttm_mem_global *glob, > > > > > glob->zones[glob->num_zones++] = zone; > > > > > return 0; > > > > > } > > > > > -#else > > > > > +#elifndef CONFIG_64BIT > > > > > static int ttm_mem_init_dma32_zone(struct > > > > > ttm_mem_global > > > > > *glob, > > > > > const struct sysinfo *si) > > > > > { > > > > > @@ -441,7 +441,7 @@ int > > > > > ttm_mem_global_init(struct > > > > > ttm_mem_global > > > > > *glob) > > > > > ret = ttm_mem_init_highmem_zone(glob, > > > > > &si); > > > > > if (unlikely(ret != 0)) > > > > > goto out_no_zone; > > > > > -#else > > > > > +#elifndef CONFIG_64BIT > > > > > ret = ttm_mem_init_dma32_zone(glob, > > > > > &si); > > > > > if (unlikely(ret != 0)) > > > > > goto out_no_zone; > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx@lists.freedesktop.org > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed... > >
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
Another good question is also why the heck the acc_size counts towards the DMA32 zone?
DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
/Thomas
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over- accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
In other words why should the internal bookkeeping pages be allocated in the DMA32 zone?
That doesn't sounds valid to me in any way, Christian.
Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: > Hmm, > > This zone was intended to stop TTM page allocations from > exhausting > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by > default, > which > means if we drop this check, other devices may stop > functioning > unexpectedly? > > However, in the end I'd expect the kernel page allocation > system > to > make sure there are some pages left in the DMA32 zone, > otherwise > random non-IO page allocations would also potentially > exhaust > the > DMA32 zone without anybody caring, which means removing > this > zone > wouldn't be any worse than whatever other subsystems may be > doing > already... > > /Thomas > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote: >> This is an RFC. I'm not sure this is the right solution, >> but >> it >> highlights the problem I'm trying to solve. >> >> The dma32_zone limits the acc_size of all allocated BOs >> to >> 2GB. >> On a >> 64-bit system with hundreds of GB of system memory and >> GPU >> memory, >> this can become a bottle neck. We're seeing TTM memory >> allocation >> failures not because we're truly out of memory, but >> because >> we're >> out of space in the dma32_zone for the acc_size needed >> for >> our BO >> book-keeping. >> >> Signed-off-by: Felix Kuehling Felix.Kuehling@amd.com >> CC: thellstrom@vmware.com >> CC: christian.koenig@amd.com >> --- >> drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c >> b/drivers/gpu/drm/ttm/ttm_memory.c >> index f1567c3..bb05365 100644 >> --- a/drivers/gpu/drm/ttm/ttm_memory.c >> +++ b/drivers/gpu/drm/ttm/ttm_memory.c >> @@ -363,7 +363,7 @@ static int >> ttm_mem_init_highmem_zone(struct >> ttm_mem_global *glob, >> glob->zones[glob->num_zones++] = zone; >> return 0; >> } >> -#else >> +#elifndef CONFIG_64BIT >> static int ttm_mem_init_dma32_zone(struct >> ttm_mem_global >> *glob, >> const struct sysinfo *si) >> { >> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct >> ttm_mem_global >> *glob) >> ret = ttm_mem_init_highmem_zone(glob, &si); >> if (unlikely(ret != 0)) >> goto out_no_zone; >> -#else >> +#elifndef CONFIG_64BIT >> ret = ttm_mem_init_dma32_zone(glob, &si); >> if (unlikely(ret != 0)) >> goto out_no_zone;
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
On Wed, 2019-02-20 at 19:23 +0000, Kuehling, Felix wrote:
On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote: > Another good question is also why the heck the acc_size > counts > towards > the DMA32 zone? DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.
Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
/Thomas
/Thomas
Regards, Felix
/Thomas
Christian.
For small persistent allocations using ttm_mem_global_alloc(), they are accounted also in the DMA32 zone, which may cause over- accounting of that zone, but that's pretty unlikely to be a big problem..
/Thomas
> In other words why should the internal bookkeeping pages > be > allocated > in > the DMA32 zone? > > That doesn't sounds valid to me in any way, > Christian. > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom: > > Hmm, > > > > This zone was intended to stop TTM page allocations > > from > > exhausting > > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by > > default, > > which > > means if we drop this check, other devices may stop > > functioning > > unexpectedly? > > > > However, in the end I'd expect the kernel page > > allocation > > system > > to > > make sure there are some pages left in the DMA32 zone, > > otherwise > > random non-IO page allocations would also potentially > > exhaust > > the > > DMA32 zone without anybody caring, which means removing > > this > > zone > > wouldn't be any worse than whatever other subsystems > > may be > > doing > > already... > > > > /Thomas > > > > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote: > > > This is an RFC. I'm not sure this is the right > > > solution, > > > but > > > it > > > highlights the problem I'm trying to solve. > > > > > > The dma32_zone limits the acc_size of all allocated > > > BOs > > > to > > > 2GB. > > > On a > > > 64-bit system with hundreds of GB of system memory > > > and > > > GPU > > > memory, > > > this can become a bottle neck. We're seeing TTM > > > memory > > > allocation > > > failures not because we're truly out of memory, but > > > because > > > we're > > > out of space in the dma32_zone for the acc_size > > > needed > > > for > > > our BO > > > book-keeping. > > > > > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com > > > > > > > CC: thellstrom@vmware.com > > > CC: christian.koenig@amd.com > > > --- > > > drivers/gpu/drm/ttm/ttm_memory.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c > > > b/drivers/gpu/drm/ttm/ttm_memory.c > > > index f1567c3..bb05365 100644 > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c > > > @@ -363,7 +363,7 @@ static int > > > ttm_mem_init_highmem_zone(struct > > > ttm_mem_global *glob, > > > glob->zones[glob->num_zones++] = zone; > > > return 0; > > > } > > > -#else > > > +#elifndef CONFIG_64BIT > > > static int ttm_mem_init_dma32_zone(struct > > > ttm_mem_global > > > *glob, > > > const struct sysinfo *si) > > > { > > > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct > > > ttm_mem_global > > > *glob) > > > ret = ttm_mem_init_highmem_zone(glob, &si); > > > if (unlikely(ret != 0)) > > > goto out_no_zone; > > > -#else > > > +#elifndef CONFIG_64BIT > > > ret = ttm_mem_init_dma32_zone(glob, &si); > > > if (unlikely(ret != 0)) > > > goto out_no_zone; _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freed...
Am 21.02.19 um 07:47 schrieb Thomas Hellstrom:
On Wed, 2019-02-20 at 19:23 +0000, Kuehling, Felix wrote:
On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
Am 18.02.19 um 10:47 schrieb Thomas Hellstrom: > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote: >> Another good question is also why the heck the acc_size >> counts >> towards >> the DMA32 zone? > DMA32 TTM pages are accounted in the DMA32 zone. Other > pages > are > not. Yeah, I'm perfectly aware of this. But this is for the accounting size!
We have an accounting for the stuff needed additional to the pages backing the BO (e.g. the page and DMA addr array).
And from the bug description it sounds like we use the DMA32 zone for this accounting which of course is completely nonsense.
It's actually accounted in all available zones, since it would be pretty hard to determine exactly where that memory should be accounted. In particular if it's vmalloced. It might be DMA32, it might not. Given the objective of stopping malicious user-space from exhausting the DMA32 zone it was, at the time the code was written, a reasonable approximation. With ever increasing memory sizes, there might be better solutions?
As far as I can see, in TTM, ttm_mem_global_alloc is only used for the acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to use it to account for a few things that are allocated with kmalloc.
So would a better solution be to change ttm_mem_global_alloc to use only the kernel zone?
IMO we need to determine what functionality to keep and then the best solution. The current code does its job, but is obviously too restrictive. Both of the solutions you suggest open up for potential DOS attacks (DMA32 and kernel zones are not mutually exclusive. They overlap).
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Regards, Christian.
/Thomas
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
How common is it to have devices that need DMA32 on an x86_64 system?
Regards, Felix
Regards, Christian.
/Thomas
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
/Thomas
Regards, Felix
Regards, Christian.
/Thomas
On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
OK, then why is it not needed when CONFIG_HIGHMEM is defined?
I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use that to decide whether to account for the DMA32 zone in TTM. It is set on x86_64 and a number of other 64-bit architectures.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
Thanks. I read up a bit on DMA32 and memory zones in general. I found that there is a lowmem_reserve_ratio feature that protects against normal page allocations overflowing into lowmem zones. There is some documentation in Documentation/scsctl/vm.txt (search for lowmem_reserve_ratio). The protected amount of memory in each zone can be seen in /proc/zoneinfo.
With that, can we conclude that we don't need to count ttm_mem_global_alloc against the dma32 zone.
Thanks, Felix
/Thomas
Regards, Felix
Regards, Christian.
/Thomas
Hi,
On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
x86_64? Can we make x86_64 more like HIGHMEM instead?
Regards, Felix
IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
OK, then why is it not needed when CONFIG_HIGHMEM is defined?
I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use that to decide whether to account for the DMA32 zone in TTM. It is set on x86_64 and a number of other 64-bit architectures.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
Thanks. I read up a bit on DMA32 and memory zones in general. I found that there is a lowmem_reserve_ratio feature that protects against normal page allocations overflowing into lowmem zones. There is some documentation in Documentation/scsctl/vm.txt (search for lowmem_reserve_ratio). The protected amount of memory in each zone can be seen in /proc/zoneinfo.
With that, can we conclude that we don't need to count ttm_mem_global_alloc against the dma32 zone.
Yes, it indeed looks like that. But then I would suggest removing the DMA32 zone entirely.
/Thomas
Thanks, Felix
/Thomas
Regards, Felix
Regards, Christian.
/Thomas
Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
Hi,
On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
> x86_64? Can we make x86_64 more like HIGHMEM instead? > > Regards, > Felix > IIRC with x86, the kernel zone is always smaller than any dma32 zone, so we'd always exhaust the kernel zone before dma32 anyway.
Not sure why we have dma32 on x86 without highmem, though. sounds superflous but harmless.
Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
OK, then why is it not needed when CONFIG_HIGHMEM is defined?
I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use that to decide whether to account for the DMA32 zone in TTM. It is set on x86_64 and a number of other 64-bit architectures.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
Thanks. I read up a bit on DMA32 and memory zones in general. I found that there is a lowmem_reserve_ratio feature that protects against normal page allocations overflowing into lowmem zones. There is some documentation in Documentation/scsctl/vm.txt (search for lowmem_reserve_ratio). The protected amount of memory in each zone can be seen in /proc/zoneinfo.
With that, can we conclude that we don't need to count ttm_mem_global_alloc against the dma32 zone.
Yes, it indeed looks like that. But then I would suggest removing the DMA32 zone entirely.
We still need it for the pages we allocate, but we should just stop accounting all the housekeeping to it.
To answer Felix earlier question we really still need DMA32 without IOMMU even on a 64bit system. The problem is simply that you have tons of PCIe hardware which can do only 32bit addressing, for example the audio codecs even on our newest Vega chips.
Regards, Christian.
/Thomas
Thanks, Felix
/Thomas
Regards, Felix
Regards, Christian.
/Thomas
On Fri, 2019-02-22 at 07:10 +0000, Koenig, Christian wrote:
Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
Hi,
On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on > > x86_64? Can we make x86_64 more like HIGHMEM instead? > > > > Regards, > > Felix > > > IIRC with x86, the kernel zone is always smaller than any > dma32 > zone, > so we'd always exhaust the kernel zone before dma32 > anyway. > > Not sure why we have dma32 on x86 without highmem, > though. > sounds > superflous but harmless. Well DMA32 denotes memory which is accessible by devices who can only do 32bit addressing. And IIRC we can actually do DMA32 to highmem since something like 2.4.*.
Because of this it is actually irrelevant if you have highmem or not, what matters for DMA32 is if you have an IOMMU or not.
Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
So even on x86_64 you actually do need the DMA32 zone if you don't have an IOMMU which remaps all memory for devices which can't directly address it.
Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
OK, then why is it not needed when CONFIG_HIGHMEM is defined?
I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use that to decide whether to account for the DMA32 zone in TTM. It is set on x86_64 and a number of other 64-bit architectures.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
Thanks. I read up a bit on DMA32 and memory zones in general. I found that there is a lowmem_reserve_ratio feature that protects against normal page allocations overflowing into lowmem zones. There is some documentation in Documentation/scsctl/vm.txt (search for lowmem_reserve_ratio). The protected amount of memory in each zone can be seen in /proc/zoneinfo.
With that, can we conclude that we don't need to count ttm_mem_global_alloc against the dma32 zone.
Yes, it indeed looks like that. But then I would suggest removing the DMA32 zone entirely.
We still need it for the pages we allocate, but we should just stop accounting all the housekeeping to it.
Why is that? Can't we just account all pages in the kernel zone, and leave it up to the kernel to make sure there are still DMA32 pages left?
/Thomas
On 2019-02-22 8:45 a.m., Thomas Hellstrom wrote:
On Fri, 2019-02-22 at 07:10 +0000, Koenig, Christian wrote:
Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
Hi,
On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
On 2019-02-21 2:59 a.m., Koenig, Christian wrote: > On x86 with HIGHMEM there is no dma32 zone. Why do we need > one > on >>> x86_64? Can we make x86_64 more like HIGHMEM instead? >>> >>> Regards, >>> Felix >>> >> IIRC with x86, the kernel zone is always smaller than any >> dma32 >> zone, >> so we'd always exhaust the kernel zone before dma32 >> anyway. >> >> Not sure why we have dma32 on x86 without highmem, >> though. >> sounds >> superflous but harmless. > Well DMA32 denotes memory which is accessible by devices > who > can > only do > 32bit addressing. And IIRC we can actually do DMA32 to > highmem > since > something like 2.4.*. > > Because of this it is actually irrelevant if you have > highmem > or > not, > what matters for DMA32 is if you have an IOMMU or not. Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
> So even on x86_64 you actually do need the DMA32 zone if > you > don't > have > an IOMMU which remaps all memory for devices which can't > directly > address it. Why is DMA32 special in this way? For example AMD GFX8 GPUs support 40-bit DMA. But we don't have a special zone for that.
If you're running on a non-IOMMU system with physical memory addresses
40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA (which won't work with most graphics drivers), or for dma_alloc_coherent(), it will probably use memory from the DMA32 zone.
OK, then why is it not needed when CONFIG_HIGHMEM is defined?
I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use that to decide whether to account for the DMA32 zone in TTM. It is set on x86_64 and a number of other 64-bit architectures.
How common is it to have devices that need DMA32 on an x86_64 system?
IIRC All devices using dma_alloc_coherent() allocate DMA32 memory unless they explicitly set the dma coherent mask to something larger. Like Christian says, if an IOMMU is present and enabled, the need for the DMA32 zone goes away. In theory at least.
Thanks. I read up a bit on DMA32 and memory zones in general. I found that there is a lowmem_reserve_ratio feature that protects against normal page allocations overflowing into lowmem zones. There is some documentation in Documentation/scsctl/vm.txt (search for lowmem_reserve_ratio). The protected amount of memory in each zone can be seen in /proc/zoneinfo.
With that, can we conclude that we don't need to count ttm_mem_global_alloc against the dma32 zone.
Yes, it indeed looks like that. But then I would suggest removing the DMA32 zone entirely.
We still need it for the pages we allocate, but we should just stop accounting all the housekeeping to it.
Why is that? Can't we just account all pages in the kernel zone, and leave it up to the kernel to make sure there are still DMA32 pages left?
ttm_page_alloc and ttm_page_alloc_dma support allocating from DMA32 explicitly (setting GFP_DMA32). Such allocations could exhaust DMA32 memory, which TTM should prevent by limiting its DMA32 usage. This would still be counted against the dma32 zone by ttm_mem_global_alloc_page.
I'll send out a new patch that counts general kernel allocations against the kernel zone only. I hope this would be acceptable.
Regards, Felix
/Thomas
dri-devel@lists.freedesktop.org
-
Christian König -
Koenig, Christian -
Kuehling, Felix -
Thomas Hellstrom -
Thomas Hellstrom