All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] prevent OOM triggered by TTM
@ 2018-02-06  9:04 Roger He
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: Roger He, Christian.Koenig

currently ttm code has no any allocation limit. So it allows pages 
allocatation unlimited until OOM. Because if swap space is full
of swapped pages and then system memory will be filled up with ttm
pages. and then any memory allocation request will trigger OOM.


the following patches is for prevent OOM triggered by TTM.
the basic idea is when allocating TTM pages, check the free swap space
firt. if it is less than the fixe limit, reject the allocation request.
but there are two exceptions which should allow it regardless of zone 
memory account limit.
a. page fault
   for ttm_mem_global_reserve if serving for page fault routine,
   because page fault routing already grabbed system memory so the 
   allowance of this exception is harmless. Otherwise, it will trigger
    OOM killer.
b. suspend
   anyway, we should allow suspend success always.


at last, if bdev.no_retry is false (by defaut), keep the original behavior
no any change.

Roger He (5):
  drm/ttm: check if the free swap space is under limit 256MB
  drm/ttm: keep original behavior except with flag no_retry
  drm/ttm: use bit flag to replace allow_reserved_eviction in
    ttm_operation_ctx
  drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY
  drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm

 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 10 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  8 +++--
 drivers/gpu/drm/nouveau/nouveau_drm.c       |  2 +-
 drivers/gpu/drm/qxl/qxl_object.c            |  4 +--
 drivers/gpu/drm/radeon/radeon_device.c      |  6 ++--
 drivers/gpu/drm/radeon/radeon_object.c      |  6 ++--
 drivers/gpu/drm/radeon/radeon_object.h      |  3 +-
 drivers/gpu/drm/ttm/ttm_bo.c                | 19 +++++++----
 drivers/gpu/drm/ttm/ttm_bo_vm.c             |  6 ++--
 drivers/gpu/drm/ttm/ttm_memory.c            | 51 ++++++++++++++++++++++++++---
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c    |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c         |  6 ++--
 include/drm/ttm/ttm_bo_api.h                | 14 ++++++--
 include/drm/ttm/ttm_memory.h                |  6 ++++
 18 files changed, 111 insertions(+), 43 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] drm/ttm: check if the free swap space is under limit 256MB
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
@ 2018-02-06  9:04   ` Roger He
  2018-02-06  9:04   ` [PATCH 2/5] drm/ttm: keep original behavior except with flag no_retry Roger He
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Roger He, thomas-4+hqylr40dJg9hUCZPvPmw, Christian.Koenig-5C7GfCeVMHo

for avoid OOM. if free swap space is less than 256MB, reject
the TTM page allocation. Otherwise, swap space will be full
of swapped pages and then system memory will be filled up
with ttm pages. and then any memory allocation request will
trigger OOM.

to cover two cases:
a. If total swap space > 256MB and at that time free swap
   space is under 256MB but available system mem > its
   lower limit, allow TTM allocation;

b. if total swap space < 256 or no swap disk at all, check
   the available system mem, if it is bigger than its
   threshold, allow TTM allocation.

Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/ttm/ttm_memory.c | 31 ++++++++++++++++++++++++++++++-
 include/drm/ttm/ttm_memory.h     |  3 +++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index aa0c381..e19e727 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -36,6 +36,7 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/swap.h>
 
 #define TTM_MEMORY_ALLOC_RETRIES 4
 
@@ -374,7 +375,8 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
 	}
 
 	si_meminfo(&si);
-
+	/* keep consistent with each zone->emer_mem */
+	glob->sys_mem_limit = si.totalram >> 2;
 	ret = ttm_mem_init_kernel_zone(glob, &si);
 	if (unlikely(ret != 0))
 		goto out_no_zone;
@@ -469,6 +471,30 @@ void ttm_mem_global_free(struct ttm_mem_global *glob,
 }
 EXPORT_SYMBOL(ttm_mem_global_free);
 
+
+#define FREE_SWAP_SPACE (256 << 8)
+/*
+ * check if the free swap space is under limit 256MB.
+ *
+ * a. if free swap space is under 256MB but available system mem is
+ * bigger than its lower limit, allow TTM allocation;
+ *
+ * b. if no swap disk at all or less than 256MB at the beginning, check
+ * the available system mem, if bigger than its threshold, allow TTM
+ * allocation.
+ */
+static bool
+ttm_check_over_swaplimit(struct ttm_mem_global *glob)
+{
+	bool ret = false;
+
+	if (get_nr_swap_pages() < FREE_SWAP_SPACE
+	    && si_mem_available() < glob->sys_mem_limit)
+		ret = true;
+
+	return ret;
+}
+
 static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
 				  struct ttm_mem_zone *single_zone,
 				  uint64_t amount, bool reserve)
@@ -478,6 +504,9 @@ static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
 	unsigned int i;
 	struct ttm_mem_zone *zone;
 
+	if (ttm_check_over_swaplimit(glob))
+		return ret;
+
 	spin_lock(&glob->lock);
 	for (i = 0; i < glob->num_zones; ++i) {
 		zone = glob->zones[i];
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index 8936285..ad7cf63 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -49,6 +49,8 @@
  * @work: The workqueue callback for the shrink queue.
  * @lock: Lock to protect the @shrink - and the memory accounting members,
  * that is, essentially the whole structure with some exceptions.
+ * @sys_mem_limit: if free ram is under this threshold and free swap space is
+ * less than its limit, reject any TTM allocation request
  * @zones: Array of pointers to accounting zones.
  * @num_zones: Number of populated entries in the @zones array.
  * @zone_kernel: Pointer to the kernel zone.
@@ -67,6 +69,7 @@ struct ttm_mem_global {
 	struct workqueue_struct *swap_queue;
 	struct work_struct work;
 	spinlock_t lock;
+	uint64_t sys_mem_limit;
 	struct ttm_mem_zone *zones[TTM_MEM_MAX_ZONES];
 	unsigned int num_zones;
 	struct ttm_mem_zone *zone_kernel;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/5] drm/ttm: keep original behavior except with flag no_retry
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
  2018-02-06  9:04   ` [PATCH 1/5] drm/ttm: check if the free swap space is under limit 256MB Roger He
@ 2018-02-06  9:04   ` Roger He
  2018-02-06  9:04   ` [PATCH 3/5] drm/ttm: use bit flag to replace allow_reserved_eviction in ttm_operation_ctx Roger He
  2018-02-06  9:04   ` [PATCH 4/5] drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY Roger He
  3 siblings, 0 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Roger He, thomas-4+hqylr40dJg9hUCZPvPmw, Christian.Koenig-5C7GfCeVMHo

set the no_retry flag in struct ttm_mem_global and init it
after ttm_mem_global_init

Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 +++++---
 drivers/gpu/drm/ttm/ttm_memory.c        | 3 +++
 include/drm/ttm/ttm_memory.h            | 3 +++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 95f9901..f740248 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -75,6 +75,7 @@ static void amdgpu_ttm_mem_global_release(struct drm_global_reference *ref)
 static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
 {
 	struct drm_global_reference *global_ref;
+	struct ttm_mem_global *mem_glob;
 	struct amdgpu_ring *ring;
 	struct drm_sched_rq *rq;
 	int r;
@@ -91,6 +92,8 @@ static int amdgpu_ttm_global_init(struct amdgpu_device *adev)
 			  "subsystem.\n");
 		goto error_mem;
 	}
+	mem_glob = (struct ttm_mem_global *)global_ref->object;
+	mem_glob->no_retry = adev->mman.bdev.no_retry;
 
 	adev->mman.bo_global_ref.mem_glob =
 		adev->mman.mem_global_ref.object;
@@ -1371,6 +1374,8 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	int r;
 	u64 vis_vram_limit;
 
+	/* We opt to avoid OOM on system pages allocations */
+	adev->mman.bdev.no_retry = true;
 	r = amdgpu_ttm_global_init(adev);
 	if (r) {
 		return r;
@@ -1388,9 +1393,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	}
 	adev->mman.initialized = true;
 
-	/* We opt to avoid OOM on system pages allocations */
-	adev->mman.bdev.no_retry = true;
-
 	r = ttm_bo_init_mm(&adev->mman.bdev, TTM_PL_VRAM,
 				adev->gmc.real_vram_size >> PAGE_SHIFT);
 	if (r) {
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index e19e727..3f00ed8 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -488,6 +488,9 @@ ttm_check_over_swaplimit(struct ttm_mem_global *glob)
 {
 	bool ret = false;
 
+	if (!glob->no_retry)
+		return ret;
+
 	if (get_nr_swap_pages() < FREE_SWAP_SPACE
 	    && si_mem_available() < glob->sys_mem_limit)
 		ret = true;
diff --git a/include/drm/ttm/ttm_memory.h b/include/drm/ttm/ttm_memory.h
index ad7cf63..7fee446 100644
--- a/include/drm/ttm/ttm_memory.h
+++ b/include/drm/ttm/ttm_memory.h
@@ -56,6 +56,8 @@
  * @zone_kernel: Pointer to the kernel zone.
  * @zone_highmem: Pointer to the highmem zone if there is one.
  * @zone_dma32: Pointer to the dma32 zone if there is one.
+ * @no_retry: if free swap space is under the fixed limit, no retry for
+ * avoid OOM
  *
  * Note that this structure is not per device. It should be global for all
  * graphics devices.
@@ -78,6 +80,7 @@ struct ttm_mem_global {
 #else
 	struct ttm_mem_zone *zone_dma32;
 #endif
+	bool no_retry;
 };
 
 extern int ttm_mem_global_init(struct ttm_mem_global *glob);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/5] drm/ttm: use bit flag to replace allow_reserved_eviction in ttm_operation_ctx
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
  2018-02-06  9:04   ` [PATCH 1/5] drm/ttm: check if the free swap space is under limit 256MB Roger He
  2018-02-06  9:04   ` [PATCH 2/5] drm/ttm: keep original behavior except with flag no_retry Roger He
@ 2018-02-06  9:04   ` Roger He
  2018-02-06  9:04   ` [PATCH 4/5] drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY Roger He
  3 siblings, 0 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Roger He, thomas-4+hqylr40dJg9hUCZPvPmw, Christian.Koenig-5C7GfCeVMHo

for saving memory and more bit flag can be used in future

Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
 drivers/gpu/drm/ttm/ttm_bo.c               | 3 ++-
 include/drm/ttm/ttm_bo_api.h               | 7 +++++--
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index eaa3cb0..dc34b50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -346,8 +346,8 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
 	struct ttm_operation_ctx ctx = {
 		.interruptible = true,
 		.no_wait_gpu = false,
-		.allow_reserved_eviction = false,
-		.resv = bo->tbo.resv
+		.resv = bo->tbo.resv,
+		.flags = 0
 	};
 	uint32_t domain;
 	int r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 512612e..0338ef6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -341,8 +341,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 	struct ttm_operation_ctx ctx = {
 		.interruptible = !kernel,
 		.no_wait_gpu = false,
-		.allow_reserved_eviction = true,
-		.resv = resv
+		.resv = resv,
+		.flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
 	};
 	struct amdgpu_bo *bo;
 	enum ttm_bo_type type;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index d90b1cf1..a907311 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -730,7 +730,8 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo,
 	*locked = false;
 	if (bo->resv == ctx->resv) {
 		reservation_object_assert_held(bo->resv);
-		if (ctx->allow_reserved_eviction || !list_empty(&bo->ddestroy))
+		if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT
+		    || !list_empty(&bo->ddestroy))
 			ret = true;
 	} else {
 		*locked = reservation_object_trylock(bo->resv);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 2cd025c..872ff6c 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -263,8 +263,8 @@ struct ttm_bo_kmap_obj {
  *
  * @interruptible: Sleep interruptible if sleeping.
  * @no_wait_gpu: Return immediately if the GPU is busy.
- * @allow_reserved_eviction: Allow eviction of reserved BOs.
  * @resv: Reservation object to allow reserved evictions with.
+ * @flags: Including the following flags
  *
  * Context for TTM operations like changing buffer placement or general memory
  * allocation.
@@ -272,11 +272,14 @@ struct ttm_bo_kmap_obj {
 struct ttm_operation_ctx {
 	bool interruptible;
 	bool no_wait_gpu;
-	bool allow_reserved_eviction;
 	struct reservation_object *resv;
 	uint64_t bytes_moved;
+	uint32_t flags;
 };
 
+/* Allow eviction of reserved BOs */
+#define TTM_OPT_FLAG_ALLOW_RES_EVICT	0x1
+
 /**
  * ttm_bo_reference - reference a struct ttm_buffer_object
  *
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/5] drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
                     ` (2 preceding siblings ...)
  2018-02-06  9:04   ` [PATCH 3/5] drm/ttm: use bit flag to replace allow_reserved_eviction in ttm_operation_ctx Roger He
@ 2018-02-06  9:04   ` Roger He
  3 siblings, 0 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Roger He, thomas-4+hqylr40dJg9hUCZPvPmw, Christian.Koenig-5C7GfCeVMHo

set TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY when we are servicing for page
fault routine.

for ttm_mem_global_reserve if in page fault routine, allow the gtt
pages reservation always. because page fault routing already grabbed
system memory and the allowance of this exception is harmless.
Otherwise, it will trigger OOM killer.

v2: keep original behavior except ttm bo with flag no_retry

Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c          |  6 ++++--
 drivers/gpu/drm/ttm/ttm_memory.c         | 25 +++++++++++++++++--------
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c |  1 -
 include/drm/ttm/ttm_bo_api.h             |  4 +++-
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 716e724..240c462 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -224,7 +224,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 		cvma.vm_page_prot = ttm_io_prot(bo->mem.placement,
 						cvma.vm_page_prot);
 	} else {
-		struct ttm_operation_ctx ctx = {
+		struct ttm_operation_ctx ttm_opt_ctx = {
 			.interruptible = false,
 			.no_wait_gpu = false
 		};
@@ -233,8 +233,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
 		cvma.vm_page_prot = ttm_io_prot(bo->mem.placement,
 						cvma.vm_page_prot);
 
+		if (ttm->page_flags & TTM_PAGE_FLAG_NO_RETRY)
+			ttm_opt_ctx.flags |= TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY;
 		/* Allocate all page at once, most common usage */
-		if (ttm->bdev->driver->ttm_tt_populate(ttm, &ctx)) {
+		if (ttm->bdev->driver->ttm_tt_populate(ttm, &ttm_opt_ctx)) {
 			ret = VM_FAULT_OOM;
 			goto out_io_unlock;
 		}
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index 3f00ed8..5e357b4 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -488,9 +488,6 @@ ttm_check_over_swaplimit(struct ttm_mem_global *glob)
 {
 	bool ret = false;
 
-	if (!glob->no_retry)
-		return ret;
-
 	if (get_nr_swap_pages() < FREE_SWAP_SPACE
 	    && si_mem_available() < glob->sys_mem_limit)
 		ret = true;
@@ -500,17 +497,28 @@ ttm_check_over_swaplimit(struct ttm_mem_global *glob)
 
 static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
 				  struct ttm_mem_zone *single_zone,
-				  uint64_t amount, bool reserve)
+				  uint64_t amount, bool reserve,
+				  bool allow_alloc_anyway)
 {
 	uint64_t limit;
 	int ret = -ENOMEM;
 	unsigned int i;
 	struct ttm_mem_zone *zone;
 
-	if (ttm_check_over_swaplimit(glob))
+	if (glob->no_retry && !allow_alloc_anyway
+	    && ttm_check_over_swaplimit(glob))
 		return ret;
 
 	spin_lock(&glob->lock);
+	/*
+	 * to cover two special cases:
+	 * a. if serving page_fault allow reservation anyway since
+	 * it already allocated system pages. Otherwise it will trigger OOM.
+	 * b. if serving suspend, allow reservation anyway as well.
+	 */
+	if (glob->no_retry && allow_alloc_anyway)
+		goto reserve_direct;
+
 	for (i = 0; i < glob->num_zones; ++i) {
 		zone = glob->zones[i];
 		if (single_zone && zone != single_zone)
@@ -523,6 +531,7 @@ static int ttm_mem_global_reserve(struct ttm_mem_global *glob,
 			goto out_unlock;
 	}
 
+reserve_direct:
 	if (reserve) {
 		for (i = 0; i < glob->num_zones; ++i) {
 			zone = glob->zones[i];
@@ -547,10 +556,10 @@ static int ttm_mem_global_alloc_zone(struct ttm_mem_global *glob,
 				     struct ttm_operation_ctx *ctx)
 {
 	int count = TTM_MEMORY_ALLOC_RETRIES;
+	bool alloc_anyway = ctx->flags & TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY;
 
-	while (unlikely(ttm_mem_global_reserve(glob,
-					       single_zone,
-					       memory, true)
+	while (unlikely(ttm_mem_global_reserve(glob, single_zone, memory,
+					       true, alloc_anyway)
 			!= 0)) {
 		if (ctx->no_wait_gpu)
 			return -ENOMEM;
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index b122f6e..354e0e1 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -944,7 +944,6 @@ int ttm_dma_populate(struct ttm_dma_tt *ttm_dma, struct device *dev,
 	i = 0;
 
 	type = ttm_to_type(ttm->page_flags, ttm->caching_state);
-
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	if (ttm->page_flags & TTM_PAGE_FLAG_DMA32)
 		goto skip_huge;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 872ff6c..f7304c5 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -278,7 +278,9 @@ struct ttm_operation_ctx {
 };
 
 /* Allow eviction of reserved BOs */
-#define TTM_OPT_FLAG_ALLOW_RES_EVICT	0x1
+#define TTM_OPT_FLAG_ALLOW_RES_EVICT		0x1
+/* when serving page fault, allow alloc anyway */
+#define TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY		0x2
 
 /**
  * ttm_bo_reference - reference a struct ttm_buffer_object
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/5] drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm
  2018-02-06  9:04 [PATCH 0/5] prevent OOM triggered by TTM Roger He
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
@ 2018-02-06  9:04 ` Roger He
  2018-02-06  9:51 ` [PATCH 0/5] prevent OOM triggered by TTM Christian König
  2018-02-07  6:43 ` Thomas Hellstrom
  3 siblings, 0 replies; 15+ messages in thread
From: Roger He @ 2018-02-06  9:04 UTC (permalink / raw)
  To: amd-gfx, dri-devel; +Cc: Roger He, Christian.Koenig

if true for it, allocate TTM pages regardless of zone global memory
account limit.

that is for another special case: suspend.
doesn't care the zone global memory account limit for this case.

Signed-off-by: Roger He <Hongbo.He@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  |  6 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c       |  2 +-
 drivers/gpu/drm/qxl/qxl_object.c            |  4 ++--
 drivers/gpu/drm/radeon/radeon_device.c      |  6 +++---
 drivers/gpu/drm/radeon/radeon_object.c      |  6 ++++--
 drivers/gpu/drm/radeon/radeon_object.h      |  3 ++-
 drivers/gpu/drm/ttm/ttm_bo.c                | 16 ++++++++++------
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c         |  6 +++---
 include/drm/ttm/ttm_bo_api.h                |  5 ++++-
 12 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index ee76b46..59ee12c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -763,7 +763,7 @@ static int amdgpu_debugfs_evict_vram(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct amdgpu_device *adev = dev->dev_private;
 
-	seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev));
+	seq_printf(m, "(%d)\n", amdgpu_bo_evict_vram(adev, true));
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 850453e..44b0e2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2168,7 +2168,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)
 		}
 	}
 	/* evict vram memory */
-	amdgpu_bo_evict_vram(adev);
+	amdgpu_bo_evict_vram(adev, true);
 
 	amdgpu_fence_driver_suspend(adev);
 
@@ -2178,7 +2178,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon)
 	 * This second call to evict vram is to evict the gart page table
 	 * using the CPU.
 	 */
-	amdgpu_bo_evict_vram(adev);
+	amdgpu_bo_evict_vram(adev, true);
 
 	pci_save_state(dev->pdev);
 	if (suspend) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 0338ef6..91f7b2f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -803,14 +803,16 @@ int amdgpu_bo_unpin(struct amdgpu_bo *bo)
 	return r;
 }
 
-int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
+int amdgpu_bo_evict_vram(struct amdgpu_device *adev,
+		bool allow_alloc_anyway)
 {
 	/* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
 	if (0 && (adev->flags & AMD_IS_APU)) {
 		/* Useless to evict on IGP chips */
 		return 0;
 	}
-	return ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_VRAM);
+	return ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_VRAM,
+			       allow_alloc_anyway);
 }
 
 static const char *amdgpu_vram_names[] = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index c2b02f5..91d897e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -227,7 +227,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
 			     u64 min_offset, u64 max_offset,
 			     u64 *gpu_addr);
 int amdgpu_bo_unpin(struct amdgpu_bo *bo);
-int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
+int amdgpu_bo_evict_vram(struct amdgpu_device *adev, bool allow_alloc_anyway);
 int amdgpu_bo_init(struct amdgpu_device *adev);
 void amdgpu_bo_fini(struct amdgpu_device *adev);
 int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 8d4a5be..c9627ef 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -702,7 +702,7 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime)
 	}
 
 	NV_DEBUG(drm, "evicting buffers...\n");
-	ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
+	ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM, true);
 
 	NV_DEBUG(drm, "waiting for kernel channels to go idle...\n");
 	if (drm->cechan) {
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index f6b80fe..d8d26c8 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -350,10 +350,10 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo *bo)
 
 int qxl_surf_evict(struct qxl_device *qdev)
 {
-	return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_PRIV);
+	return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_PRIV, true);
 }
 
 int qxl_vram_evict(struct qxl_device *qdev)
 {
-	return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_VRAM);
+	return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_VRAM, true);
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8d3e3d2..c11ee06 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1522,7 +1522,7 @@ void radeon_device_fini(struct radeon_device *rdev)
 	DRM_INFO("radeon: finishing device.\n");
 	rdev->shutdown = true;
 	/* evict vram memory */
-	radeon_bo_evict_vram(rdev);
+	radeon_bo_evict_vram(rdev, true);
 	radeon_fini(rdev);
 	if (!pci_is_thunderbolt_attached(rdev->pdev))
 		vga_switcheroo_unregister_client(rdev->pdev);
@@ -1607,7 +1607,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 		}
 	}
 	/* evict vram memory */
-	radeon_bo_evict_vram(rdev);
+	radeon_bo_evict_vram(rdev, true);
 
 	/* wait for gpu to finish processing current batch */
 	for (i = 0; i < RADEON_NUM_RINGS; i++) {
@@ -1626,7 +1626,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 	 * This second call to evict vram is to evict the gart page table
 	 * using the CPU.
 	 */
-	radeon_bo_evict_vram(rdev);
+	radeon_bo_evict_vram(rdev, true);
 
 	radeon_agp_suspend(rdev);
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 15404af..aa1ece5 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -420,7 +420,8 @@ int radeon_bo_unpin(struct radeon_bo *bo)
 	return r;
 }
 
-int radeon_bo_evict_vram(struct radeon_device *rdev)
+int
+radeon_bo_evict_vram(struct radeon_device *rdev, bool allow_alloc_anyway)
 {
 	/* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
 	if (0 && (rdev->flags & RADEON_IS_IGP)) {
@@ -428,7 +429,8 @@ int radeon_bo_evict_vram(struct radeon_device *rdev)
 			/* Useless to evict on IGP chips */
 			return 0;
 	}
-	return ttm_bo_evict_mm(&rdev->mman.bdev, TTM_PL_VRAM);
+	return ttm_bo_evict_mm(&rdev->mman.bdev, TTM_PL_VRAM,
+			       allow_alloc_anyway);
 }
 
 void radeon_bo_force_delete(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 9ffd821..1093112 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -136,7 +136,8 @@ extern int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr);
 extern int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain,
 				    u64 max_offset, u64 *gpu_addr);
 extern int radeon_bo_unpin(struct radeon_bo *bo);
-extern int radeon_bo_evict_vram(struct radeon_device *rdev);
+extern int radeon_bo_evict_vram(struct radeon_device *rdev,
+			bool allow_alloc_anyway);
 extern void radeon_bo_force_delete(struct radeon_device *rdev);
 extern int radeon_bo_init(struct radeon_device *rdev);
 extern void radeon_bo_fini(struct radeon_device *rdev);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index a907311..1644505 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1342,15 +1342,17 @@ int ttm_bo_create(struct ttm_bo_device *bdev,
 EXPORT_SYMBOL(ttm_bo_create);
 
 static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev,
-				   unsigned mem_type)
+			unsigned mem_type, bool allow_alloc_anyway)
 {
-	struct ttm_operation_ctx ctx = { false, false };
+	struct ttm_operation_ctx ttm_opt_ctx = { false, false };
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
 	struct ttm_bo_global *glob = bdev->glob;
 	struct dma_fence *fence;
 	int ret;
 	unsigned i;
 
+	if (allow_alloc_anyway)
+		ttm_opt_ctx.flags = TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY;
 	/*
 	 * Can't use standard list traversal since we're unlocking.
 	 */
@@ -1359,7 +1361,8 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev,
 	for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
 		while (!list_empty(&man->lru[i])) {
 			spin_unlock(&glob->lru_lock);
-			ret = ttm_mem_evict_first(bdev, mem_type, NULL, &ctx);
+			ret = ttm_mem_evict_first(bdev, mem_type, NULL,
+						  &ttm_opt_ctx);
 			if (ret)
 				return ret;
 			spin_lock(&glob->lru_lock);
@@ -1403,7 +1406,7 @@ int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
 
 	ret = 0;
 	if (mem_type > 0) {
-		ret = ttm_bo_force_list_clean(bdev, mem_type);
+		ret = ttm_bo_force_list_clean(bdev, mem_type, true);
 		if (ret) {
 			pr_err("Cleanup eviction failed\n");
 			return ret;
@@ -1419,7 +1422,8 @@ int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
 }
 EXPORT_SYMBOL(ttm_bo_clean_mm);
 
-int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type)
+int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type,
+			bool allow_allo_anyway)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem_type];
 
@@ -1433,7 +1437,7 @@ int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type)
 		return 0;
 	}
 
-	return ttm_bo_force_list_clean(bdev, mem_type);
+	return ttm_bo_force_list_clean(bdev, mem_type, allow_allo_anyway);
 }
 EXPORT_SYMBOL(ttm_bo_evict_mm);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 184340d..28f8e4f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -430,7 +430,7 @@ static int vmw_request_device(struct vmw_private *dev_priv)
 	if (dev_priv->cman)
 		vmw_cmdbuf_remove_pool(dev_priv->cman);
 	if (dev_priv->has_mob) {
-		(void) ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
+		(void) ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB, true);
 		vmw_otables_takedown(dev_priv);
 	}
 	if (dev_priv->cman)
@@ -463,7 +463,7 @@ static void vmw_release_device_early(struct vmw_private *dev_priv)
 		vmw_cmdbuf_remove_pool(dev_priv->cman);
 
 	if (dev_priv->has_mob) {
-		ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
+		ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB, true);
 		vmw_otables_takedown(dev_priv);
 	}
 }
@@ -1342,7 +1342,7 @@ void vmw_svga_disable(struct vmw_private *dev_priv)
 	if (dev_priv->bdev.man[TTM_PL_VRAM].use_type) {
 		dev_priv->bdev.man[TTM_PL_VRAM].use_type = false;
 		spin_unlock(&dev_priv->svga_lock);
-		if (ttm_bo_evict_mm(&dev_priv->bdev, TTM_PL_VRAM))
+		if (ttm_bo_evict_mm(&dev_priv->bdev, TTM_PL_VRAM, true))
 			DRM_ERROR("Failed evicting VRAM buffers.\n");
 		vmw_write(dev_priv, SVGA_REG_ENABLE,
 			  SVGA_REG_ENABLE_HIDE |
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index f7304c5..f97fb5f 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -636,6 +636,8 @@ int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type);
  *
  * @bdev: Pointer to a ttm_bo_device struct.
  * @mem_type: The memory type.
+ * @allow_alloc_anyway: if true allow ttm pages allocation always
+ * regardless of zone memory account limit
  *
  * Evicts all buffers on the lru list of the memory type.
  * This is normally part of a VT switch or an
@@ -649,7 +651,8 @@ int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type);
  * -ERESTARTSYS: The call was interrupted by a signal while waiting to
  * evict a buffer.
  */
-int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type);
+int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type,
+			bool allow_alloc_anyway);
 
 /**
  * ttm_kmap_obj_virtual
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-06  9:04 [PATCH 0/5] prevent OOM triggered by TTM Roger He
       [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
  2018-02-06  9:04 ` [PATCH 5/5] drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm Roger He
@ 2018-02-06  9:51 ` Christian König
  2018-02-06 10:07   ` He, Roger
  2018-02-07  6:43 ` Thomas Hellstrom
  3 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2018-02-06  9:51 UTC (permalink / raw)
  To: Roger He, amd-gfx, dri-devel

Nice work, but a few comments.

First of all you need to reorder the patches. Adding the exceptions to 
the restrictions should come first, then the restriction itself. 
Otherwise we might break a setup in between the patches and that is bad 
for bisecting.

Then make all values configurable, e.g. take a closer look at 
ttm_memory.c. Just add attributes directly under the memory_accounting 
directory (see ttm_mem_global_init).

Additional to that you can't put device specific information (the 
no_retry flag) into ttm_mem_global, that is driver unspecific and won't 
work like this.

Move the new call out of ttm_mem_global_reserve() and into 
ttm_page_alloc.c or ttm_page_alloc_dma.c (but keep it in ttm_memory.c). 
ttm_mem_global_reserve() is called for each page allocated and 
si_mem_available() is a bit to heavy for that.

Maybe name TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY something like 
_FORCE_ALLOCATION or _ALLOW_OOM.

And please also try if a criteria like (si_mem_available() + 
get_nr_swap_pages()) < limit works as well. This way we would have only 
a single new limit.

Regards,
Christian.

Am 06.02.2018 um 10:04 schrieb Roger He:
> currently ttm code has no any allocation limit. So it allows pages
> allocatation unlimited until OOM. Because if swap space is full
> of swapped pages and then system memory will be filled up with ttm
> pages. and then any memory allocation request will trigger OOM.
>
>
> the following patches is for prevent OOM triggered by TTM.
> the basic idea is when allocating TTM pages, check the free swap space
> firt. if it is less than the fixe limit, reject the allocation request.
> but there are two exceptions which should allow it regardless of zone
> memory account limit.
> a. page fault
>     for ttm_mem_global_reserve if serving for page fault routine,
>     because page fault routing already grabbed system memory so the
>     allowance of this exception is harmless. Otherwise, it will trigger
>      OOM killer.
> b. suspend
>     anyway, we should allow suspend success always.
>
>
> at last, if bdev.no_retry is false (by defaut), keep the original behavior
> no any change.
>
> Roger He (5):
>    drm/ttm: check if the free swap space is under limit 256MB
>    drm/ttm: keep original behavior except with flag no_retry
>    drm/ttm: use bit flag to replace allow_reserved_eviction in
>      ttm_operation_ctx
>    drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY
>    drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 10 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  8 +++--
>   drivers/gpu/drm/nouveau/nouveau_drm.c       |  2 +-
>   drivers/gpu/drm/qxl/qxl_object.c            |  4 +--
>   drivers/gpu/drm/radeon/radeon_device.c      |  6 ++--
>   drivers/gpu/drm/radeon/radeon_object.c      |  6 ++--
>   drivers/gpu/drm/radeon/radeon_object.h      |  3 +-
>   drivers/gpu/drm/ttm/ttm_bo.c                | 19 +++++++----
>   drivers/gpu/drm/ttm/ttm_bo_vm.c             |  6 ++--
>   drivers/gpu/drm/ttm/ttm_memory.c            | 51 ++++++++++++++++++++++++++---
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c    |  1 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c         |  6 ++--
>   include/drm/ttm/ttm_bo_api.h                | 14 ++++++--
>   include/drm/ttm/ttm_memory.h                |  6 ++++
>   18 files changed, 111 insertions(+), 43 deletions(-)
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-06  9:51 ` [PATCH 0/5] prevent OOM triggered by TTM Christian König
@ 2018-02-06 10:07   ` He, Roger
  0 siblings, 0 replies; 15+ messages in thread
From: He, Roger @ 2018-02-06 10:07 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx, dri-devel

	Move the new call out of ttm_mem_global_reserve() and into ttm_page_alloc.c or ttm_page_alloc_dma.c (but keep it in 	ttm_memory.c).  ttm_mem_global_reserve() is called for each page allocated and si_mem_available() is a bit to heavy for that.

Good idea! Agree with you completely, because initially I also concern that but no better way at that time.
Going to improve the patches. Thanks!

-----Original Message-----
From: Koenig, Christian 
Sent: Tuesday, February 06, 2018 5:52 PM
To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: thomas@shipmail.org
Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM

Nice work, but a few comments.

First of all you need to reorder the patches. Adding the exceptions to the restrictions should come first, then the restriction itself. 
Otherwise we might break a setup in between the patches and that is bad for bisecting.

Then make all values configurable, e.g. take a closer look at ttm_memory.c. Just add attributes directly under the memory_accounting directory (see ttm_mem_global_init).

Additional to that you can't put device specific information (the no_retry flag) into ttm_mem_global, that is driver unspecific and won't work like this.

Move the new call out of ttm_mem_global_reserve() and into ttm_page_alloc.c or ttm_page_alloc_dma.c (but keep it in ttm_memory.c). 
ttm_mem_global_reserve() is called for each page allocated and
si_mem_available() is a bit to heavy for that.

Maybe name TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY something like _FORCE_ALLOCATION or _ALLOW_OOM.

And please also try if a criteria like (si_mem_available() +
get_nr_swap_pages()) < limit works as well. This way we would have only a single new limit.

Regards,
Christian.

Am 06.02.2018 um 10:04 schrieb Roger He:
> currently ttm code has no any allocation limit. So it allows pages 
> allocatation unlimited until OOM. Because if swap space is full of 
> swapped pages and then system memory will be filled up with ttm pages. 
> and then any memory allocation request will trigger OOM.
>
>
> the following patches is for prevent OOM triggered by TTM.
> the basic idea is when allocating TTM pages, check the free swap space 
> firt. if it is less than the fixe limit, reject the allocation request.
> but there are two exceptions which should allow it regardless of zone 
> memory account limit.
> a. page fault
>     for ttm_mem_global_reserve if serving for page fault routine,
>     because page fault routing already grabbed system memory so the
>     allowance of this exception is harmless. Otherwise, it will trigger
>      OOM killer.
> b. suspend
>     anyway, we should allow suspend success always.
>
>
> at last, if bdev.no_retry is false (by defaut), keep the original 
> behavior no any change.
>
> Roger He (5):
>    drm/ttm: check if the free swap space is under limit 256MB
>    drm/ttm: keep original behavior except with flag no_retry
>    drm/ttm: use bit flag to replace allow_reserved_eviction in
>      ttm_operation_ctx
>    drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY
>    drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 10 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  8 +++--
>   drivers/gpu/drm/nouveau/nouveau_drm.c       |  2 +-
>   drivers/gpu/drm/qxl/qxl_object.c            |  4 +--
>   drivers/gpu/drm/radeon/radeon_device.c      |  6 ++--
>   drivers/gpu/drm/radeon/radeon_object.c      |  6 ++--
>   drivers/gpu/drm/radeon/radeon_object.h      |  3 +-
>   drivers/gpu/drm/ttm/ttm_bo.c                | 19 +++++++----
>   drivers/gpu/drm/ttm/ttm_bo_vm.c             |  6 ++--
>   drivers/gpu/drm/ttm/ttm_memory.c            | 51 ++++++++++++++++++++++++++---
>   drivers/gpu/drm/ttm/ttm_page_alloc_dma.c    |  1 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c         |  6 ++--
>   include/drm/ttm/ttm_bo_api.h                | 14 ++++++--
>   include/drm/ttm/ttm_memory.h                |  6 ++++
>   18 files changed, 111 insertions(+), 43 deletions(-)
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-06  9:04 [PATCH 0/5] prevent OOM triggered by TTM Roger He
                   ` (2 preceding siblings ...)
  2018-02-06  9:51 ` [PATCH 0/5] prevent OOM triggered by TTM Christian König
@ 2018-02-07  6:43 ` Thomas Hellstrom
  2018-02-07  8:25   ` He, Roger
  3 siblings, 1 reply; 15+ messages in thread
From: Thomas Hellstrom @ 2018-02-07  6:43 UTC (permalink / raw)
  To: Roger He, amd-gfx, dri-devel; +Cc: Christian.Koenig

Hi, Roger,

On 02/06/2018 10:04 AM, Roger He wrote:
> currently ttm code has no any allocation limit. So it allows pages
> allocatation unlimited until OOM. Because if swap space is full
> of swapped pages and then system memory will be filled up with ttm
> pages. and then any memory allocation request will trigger OOM.
>

I'm a bit curious, isn't this the way things are supposed to work on a 
linux system?
If all memory resources are used up, the OOM killer will kill the most 
memory hungry (perhaps rogue) process
rather than processes being nice and try to find out themselves whether 
allocations will succeed?
Why should TTM be different in that aspect? It would be good to know 
your reasoning WRT this?

Admittedly, graphics process OOM memory accounting doesn't work very 
well, due to not all BOs not being
CPU mapped, but it looks like there is recent work towards fixing this?

One thing I looked at at one point was to have TTM do the swapping 
itself instead of handing it off to the
shmem system. That way we could pre-allocate swap entries for all 
swappable (BO) memory, making sure that
we wouldn't run out of swap space when, for example, hibernating and 
that would also limit the pinned
non-swappable memory (from TTM driver kernel allocations for example) to 
half the system memory resources.

Thanks,
Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-07  6:43 ` Thomas Hellstrom
@ 2018-02-07  8:25   ` He, Roger
  2018-02-07  8:41     ` Christian König
       [not found]     ` <MWHPR1201MB0127DA7619E7AE8C95D8FE8DFDFC0-3iK1xFAIwjq9imrIu4W8xGrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
  0 siblings, 2 replies; 15+ messages in thread
From: He, Roger @ 2018-02-07  8:25 UTC (permalink / raw)
  To: Thomas Hellstrom, amd-gfx, dri-devel; +Cc: Koenig, Christian

	Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?

Now, in TTM struct ttm_bo_device it already has member no_retry to indicate your option.
If you prefer no OOM triggered by TTM, set it as true. The default is false to keep original behavior. 
AMD prefers return value of no memory rather than OOM for now.



	One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when, 

I prefer current mechanism of swap out. At the beginning the swapped pages stay in system memory by shmem until OS move to status with high memory pressure, that has an obvious advantage. For example, if the BO is swapped out into shmem, but not really be flushed into swap disk. When validate it and swap in it at this moment, the overhead is small compared to swap in from disk. In addition, No need swap space reservation for TTM pages when allocation since swap disk is shared not only for TTM exclusive.
So again we provide a flag no_retry in struct ttm_bo_device to let driver set according to its request.


Thanks
Roger(Hongbo.He)

-----Original Message-----
From: Thomas Hellstrom [mailto:thomas@shipmail.org] 
Sent: Wednesday, February 07, 2018 2:43 PM
To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM

Hi, Roger,

On 02/06/2018 10:04 AM, Roger He wrote:
> currently ttm code has no any allocation limit. So it allows pages 
> allocatation unlimited until OOM. Because if swap space is full of 
> swapped pages and then system memory will be filled up with ttm pages. 
> and then any memory allocation request will trigger OOM.
>

I'm a bit curious, isn't this the way things are supposed to work on a linux system?
If all memory resources are used up, the OOM killer will kill the most memory hungry (perhaps rogue) process rather than processes being nice and try to find out themselves whether allocations will succeed?
Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?

Admittedly, graphics process OOM memory accounting doesn't work very well, due to not all BOs not being CPU mapped, but it looks like there is recent work towards fixing this?

One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when, for example, hibernating and that would also limit the pinned non-swappable memory (from TTM driver kernel allocations for example) to half the system memory resources.

Thanks,
Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-07  8:25   ` He, Roger
@ 2018-02-07  8:41     ` Christian König
       [not found]       ` <d617ad27-095d-f17d-d310-89caf21d12ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
       [not found]     ` <MWHPR1201MB0127DA7619E7AE8C95D8FE8DFDFC0-3iK1xFAIwjq9imrIu4W8xGrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Christian König @ 2018-02-07  8:41 UTC (permalink / raw)
  To: He, Roger, Thomas Hellstrom, amd-gfx, dri-devel; +Cc: Koenig, Christian

> AMD prefers return value of no memory rather than OOM for now.
Yeah, but Thomas is right that the default for this feature should be 
that it is turned off.

That's why I said that we should make the limit configurable.

Regards,
Christian.

Am 07.02.2018 um 09:25 schrieb He, Roger:
> 	Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Now, in TTM struct ttm_bo_device it already has member no_retry to indicate your option.
> If you prefer no OOM triggered by TTM, set it as true. The default is false to keep original behavior.
> AMD prefers return value of no memory rather than OOM for now.
>
>
>
> 	One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when,
>
> I prefer current mechanism of swap out. At the beginning the swapped pages stay in system memory by shmem until OS move to status with high memory pressure, that has an obvious advantage. For example, if the BO is swapped out into shmem, but not really be flushed into swap disk. When validate it and swap in it at this moment, the overhead is small compared to swap in from disk. In addition, No need swap space reservation for TTM pages when allocation since swap disk is shared not only for TTM exclusive.
> So again we provide a flag no_retry in struct ttm_bo_device to let driver set according to its request.
>
>
> Thanks
> Roger(Hongbo.He)
>
> -----Original Message-----
> From: Thomas Hellstrom [mailto:thomas@shipmail.org]
> Sent: Wednesday, February 07, 2018 2:43 PM
> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM
>
> Hi, Roger,
>
> On 02/06/2018 10:04 AM, Roger He wrote:
>> currently ttm code has no any allocation limit. So it allows pages
>> allocatation unlimited until OOM. Because if swap space is full of
>> swapped pages and then system memory will be filled up with ttm pages.
>> and then any memory allocation request will trigger OOM.
>>
> I'm a bit curious, isn't this the way things are supposed to work on a linux system?
> If all memory resources are used up, the OOM killer will kill the most memory hungry (perhaps rogue) process rather than processes being nice and try to find out themselves whether allocations will succeed?
> Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Admittedly, graphics process OOM memory accounting doesn't work very well, due to not all BOs not being CPU mapped, but it looks like there is recent work towards fixing this?
>
> One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when, for example, hibernating and that would also limit the pinned non-swappable memory (from TTM driver kernel allocations for example) to half the system memory resources.
>
> Thanks,
> Thomas
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 0/5] prevent OOM triggered by TTM
       [not found]       ` <d617ad27-095d-f17d-d310-89caf21d12ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2018-02-07 10:19         ` He, Roger
  0 siblings, 0 replies; 15+ messages in thread
From: He, Roger @ 2018-02-07 10:19 UTC (permalink / raw)
  To: Koenig, Christian, Thomas Hellstrom,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Sure, will make it turn off as default and make the limit configurable.

Thanks
Roger(Hongbo.He)
-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: Wednesday, February 07, 2018 4:41 PM
To: He, Roger <Hongbo.He@amd.com>; Thomas Hellstrom <thomas@shipmail.org>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM

> AMD prefers return value of no memory rather than OOM for now.
Yeah, but Thomas is right that the default for this feature should be that it is turned off.

That's why I said that we should make the limit configurable.

Regards,
Christian.

Am 07.02.2018 um 09:25 schrieb He, Roger:
> 	Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Now, in TTM struct ttm_bo_device it already has member no_retry to indicate your option.
> If you prefer no OOM triggered by TTM, set it as true. The default is false to keep original behavior.
> AMD prefers return value of no memory rather than OOM for now.
>
>
>
> 	One thing I looked at at one point was to have TTM do the swapping 
> itself instead of handing it off to the shmem system. That way we 
> could pre-allocate swap entries for all swappable (BO) memory, making 
> sure that we wouldn't run out of swap space when,
>
> I prefer current mechanism of swap out. At the beginning the swapped pages stay in system memory by shmem until OS move to status with high memory pressure, that has an obvious advantage. For example, if the BO is swapped out into shmem, but not really be flushed into swap disk. When validate it and swap in it at this moment, the overhead is small compared to swap in from disk. In addition, No need swap space reservation for TTM pages when allocation since swap disk is shared not only for TTM exclusive.
> So again we provide a flag no_retry in struct ttm_bo_device to let driver set according to its request.
>
>
> Thanks
> Roger(Hongbo.He)
>
> -----Original Message-----
> From: Thomas Hellstrom [mailto:thomas@shipmail.org]
> Sent: Wednesday, February 07, 2018 2:43 PM
> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; 
> dri-devel@lists.freedesktop.org
> Cc: Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM
>
> Hi, Roger,
>
> On 02/06/2018 10:04 AM, Roger He wrote:
>> currently ttm code has no any allocation limit. So it allows pages 
>> allocatation unlimited until OOM. Because if swap space is full of 
>> swapped pages and then system memory will be filled up with ttm pages.
>> and then any memory allocation request will trigger OOM.
>>
> I'm a bit curious, isn't this the way things are supposed to work on a linux system?
> If all memory resources are used up, the OOM killer will kill the most memory hungry (perhaps rogue) process rather than processes being nice and try to find out themselves whether allocations will succeed?
> Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Admittedly, graphics process OOM memory accounting doesn't work very well, due to not all BOs not being CPU mapped, but it looks like there is recent work towards fixing this?
>
> One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when, for example, hibernating and that would also limit the pinned non-swappable memory (from TTM driver kernel allocations for example) to half the system memory resources.
>
> Thanks,
> Thomas
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
       [not found]     ` <MWHPR1201MB0127DA7619E7AE8C95D8FE8DFDFC0-3iK1xFAIwjq9imrIu4W8xGrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
@ 2018-02-07 13:17       ` Thomas Hellstrom
  2018-02-07 13:22         ` Christian König
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Hellstrom @ 2018-02-07 13:17 UTC (permalink / raw)
  To: He, Roger, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Koenig, Christian

Hi, Roger.

On 02/07/2018 09:25 AM, He, Roger wrote:
> 	Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Now, in TTM struct ttm_bo_device it already has member no_retry to indicate your option.
> If you prefer no OOM triggered by TTM, set it as true. The default is false to keep original behavior.
> AMD prefers return value of no memory rather than OOM for now.

Understood, but why is that? I mean just because TTM doesn't invoke the 
OOM killer, that doesn't mean that the process will, the next 
millisecond, page in a number of pages and invoke it? So this mechanism 
would be pretty susceptible to races?
> 	One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when,
>
> I prefer current mechanism of swap out. At the beginning the swapped pages stay in system memory by shmem until OS move to status with high memory pressure, that has an obvious advantage. For example, if the BO is swapped out into shmem, but not really be flushed into swap disk. When validate it and swap in it at this moment, the overhead is small compared to swap in from disk.

But that is true for a page handed off to the swap-cache as well. It 
won't be immediately flushed to disc, only when the swap cache is shrunk.

> In addition, No need swap space reservation for TTM pages when allocation since swap disk is shared not only for TTM exclusive.

That's true, but with VRAM, TTM overcommits swap space which may lead to 
ugly memory allocation failures at hibernate time.

> So again we provide a flag no_retry in struct ttm_bo_device to let driver set according to its request.

Thanks,
Thomas


>
>
> Thanks
> Roger(Hongbo.He)
>
> -----Original Message-----
> From: Thomas Hellstrom [mailto:thomas@shipmail.org]
> Sent: Wednesday, February 07, 2018 2:43 PM
> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM
>
> Hi, Roger,
>
> On 02/06/2018 10:04 AM, Roger He wrote:
>> currently ttm code has no any allocation limit. So it allows pages
>> allocatation unlimited until OOM. Because if swap space is full of
>> swapped pages and then system memory will be filled up with ttm pages.
>> and then any memory allocation request will trigger OOM.
>>
> I'm a bit curious, isn't this the way things are supposed to work on a linux system?
> If all memory resources are used up, the OOM killer will kill the most memory hungry (perhaps rogue) process rather than processes being nice and try to find out themselves whether allocations will succeed?
> Why should TTM be different in that aspect? It would be good to know your reasoning WRT this?
>
> Admittedly, graphics process OOM memory accounting doesn't work very well, due to not all BOs not being CPU mapped, but it looks like there is recent work towards fixing this?
>
> One thing I looked at at one point was to have TTM do the swapping itself instead of handing it off to the shmem system. That way we could pre-allocate swap entries for all swappable (BO) memory, making sure that we wouldn't run out of swap space when, for example, hibernating and that would also limit the pinned non-swappable memory (from TTM driver kernel allocations for example) to half the system memory resources.
>
> Thanks,
> Thomas
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
  2018-02-07 13:17       ` Thomas Hellstrom
@ 2018-02-07 13:22         ` Christian König
       [not found]           ` <4b46ea00-ddb4-a316-02b8-247bd3812a14-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Christian König @ 2018-02-07 13:22 UTC (permalink / raw)
  To: Thomas Hellstrom, He, Roger, amd-gfx, dri-devel

> Understood, but why is that?
Well because customers requested it :)

What we try to do here is having a parameter which says when less than x 
megabytes of memory are left then fail the allocation.

This is basically to prevent buggy applications which try to allocate as 
much memory as possible until they receive an -ENOMEM from running into 
the OOM killer.

> That's true, but with VRAM, TTM overcommits swap space which may lead 
> to ugly memory allocation failures at hibernate time.
Yeah, that is exactly the reason why I said that Roger should disable 
the limit during suspend swap out :)

Regards,
Christian.

Am 07.02.2018 um 14:17 schrieb Thomas Hellstrom:
> Hi, Roger.
>
> On 02/07/2018 09:25 AM, He, Roger wrote:
>>     Why should TTM be different in that aspect? It would be good to 
>> know your reasoning WRT this?
>>
>> Now, in TTM struct ttm_bo_device it already has member no_retry to 
>> indicate your option.
>> If you prefer no OOM triggered by TTM, set it as true. The default is 
>> false to keep original behavior.
>> AMD prefers return value of no memory rather than OOM for now.
>
> Understood, but why is that? I mean just because TTM doesn't invoke 
> the OOM killer, that doesn't mean that the process will, the next 
> millisecond, page in a number of pages and invoke it? So this 
> mechanism would be pretty susceptible to races?
>>     One thing I looked at at one point was to have TTM do the 
>> swapping itself instead of handing it off to the shmem system. That 
>> way we could pre-allocate swap entries for all swappable (BO) memory, 
>> making sure that we wouldn't run out of swap space when,
>>
>> I prefer current mechanism of swap out. At the beginning the swapped 
>> pages stay in system memory by shmem until OS move to status with 
>> high memory pressure, that has an obvious advantage. For example, if 
>> the BO is swapped out into shmem, but not really be flushed into swap 
>> disk. When validate it and swap in it at this moment, the overhead is 
>> small compared to swap in from disk.
>
> But that is true for a page handed off to the swap-cache as well. It 
> won't be immediately flushed to disc, only when the swap cache is shrunk.
>
>> In addition, No need swap space reservation for TTM pages when 
>> allocation since swap disk is shared not only for TTM exclusive.
>
> That's true, but with VRAM, TTM overcommits swap space which may lead 
> to ugly memory allocation failures at hibernate time.
>
>> So again we provide a flag no_retry in struct ttm_bo_device to let 
>> driver set according to its request.
>
> Thanks,
> Thomas
>
>
>>
>>
>> Thanks
>> Roger(Hongbo.He)
>>
>> -----Original Message-----
>> From: Thomas Hellstrom [mailto:thomas@shipmail.org]
>> Sent: Wednesday, February 07, 2018 2:43 PM
>> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; 
>> dri-devel@lists.freedesktop.org
>> Cc: Koenig, Christian <Christian.Koenig@amd.com>
>> Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM
>>
>> Hi, Roger,
>>
>> On 02/06/2018 10:04 AM, Roger He wrote:
>>> currently ttm code has no any allocation limit. So it allows pages
>>> allocatation unlimited until OOM. Because if swap space is full of
>>> swapped pages and then system memory will be filled up with ttm pages.
>>> and then any memory allocation request will trigger OOM.
>>>
>> I'm a bit curious, isn't this the way things are supposed to work on 
>> a linux system?
>> If all memory resources are used up, the OOM killer will kill the 
>> most memory hungry (perhaps rogue) process rather than processes 
>> being nice and try to find out themselves whether allocations will 
>> succeed?
>> Why should TTM be different in that aspect? It would be good to know 
>> your reasoning WRT this?
>>
>> Admittedly, graphics process OOM memory accounting doesn't work very 
>> well, due to not all BOs not being CPU mapped, but it looks like 
>> there is recent work towards fixing this?
>>
>> One thing I looked at at one point was to have TTM do the swapping 
>> itself instead of handing it off to the shmem system. That way we 
>> could pre-allocate swap entries for all swappable (BO) memory, making 
>> sure that we wouldn't run out of swap space when, for example, 
>> hibernating and that would also limit the pinned non-swappable memory 
>> (from TTM driver kernel allocations for example) to half the system 
>> memory resources.
>>
>> Thanks,
>> Thomas
>>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] prevent OOM triggered by TTM
       [not found]           ` <4b46ea00-ddb4-a316-02b8-247bd3812a14-5C7GfCeVMHo@public.gmane.org>
@ 2018-02-07 18:07             ` Thomas Hellstrom
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Hellstrom @ 2018-02-07 18:07 UTC (permalink / raw)
  To: Christian König, He, Roger,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Hi,

On 02/07/2018 02:22 PM, Christian König wrote:
>> Understood, but why is that?
> Well because customers requested it :)
>
> What we try to do here is having a parameter which says when less than 
> x megabytes of memory are left then fail the allocation.
>
> This is basically to prevent buggy applications which try to allocate 
> as much memory as possible until they receive an -ENOMEM from running 
> into the OOM killer.

OK. Understood.

>
>> That's true, but with VRAM, TTM overcommits swap space which may lead 
>> to ugly memory allocation failures at hibernate time.
> Yeah, that is exactly the reason why I said that Roger should disable 
> the limit during suspend swap out :)

Well that was really in the context of the swapping implementation 
rather in the context of this change so it was a little off-topic. Even 
if disabling this limit, TTM can overcommit. But looking at the swapping 
implementation is a different issue.

/Thomas





>
> Regards,
> Christian.
>
> Am 07.02.2018 um 14:17 schrieb Thomas Hellstrom:
>> Hi, Roger.
>>
>> On 02/07/2018 09:25 AM, He, Roger wrote:
>>>     Why should TTM be different in that aspect? It would be good to 
>>> know your reasoning WRT this?
>>>
>>> Now, in TTM struct ttm_bo_device it already has member no_retry to 
>>> indicate your option.
>>> If you prefer no OOM triggered by TTM, set it as true. The default 
>>> is false to keep original behavior.
>>> AMD prefers return value of no memory rather than OOM for now.
>>
>> Understood, but why is that? I mean just because TTM doesn't invoke 
>> the OOM killer, that doesn't mean that the process will, the next 
>> millisecond, page in a number of pages and invoke it? So this 
>> mechanism would be pretty susceptible to races?
>>>     One thing I looked at at one point was to have TTM do the 
>>> swapping itself instead of handing it off to the shmem system. That 
>>> way we could pre-allocate swap entries for all swappable (BO) 
>>> memory, making sure that we wouldn't run out of swap space when,
>>>
>>> I prefer current mechanism of swap out. At the beginning the swapped 
>>> pages stay in system memory by shmem until OS move to status with 
>>> high memory pressure, that has an obvious advantage. For example, if 
>>> the BO is swapped out into shmem, but not really be flushed into 
>>> swap disk. When validate it and swap in it at this moment, the 
>>> overhead is small compared to swap in from disk.
>>
>> But that is true for a page handed off to the swap-cache as well. It 
>> won't be immediately flushed to disc, only when the swap cache is 
>> shrunk.
>>
>>> In addition, No need swap space reservation for TTM pages when 
>>> allocation since swap disk is shared not only for TTM exclusive.
>>
>> That's true, but with VRAM, TTM overcommits swap space which may lead 
>> to ugly memory allocation failures at hibernate time.
>>
>>> So again we provide a flag no_retry in struct ttm_bo_device to let 
>>> driver set according to its request.
>>
>> Thanks,
>> Thomas
>>
>>
>>>
>>>
>>> Thanks
>>> Roger(Hongbo.He)
>>>
>>> -----Original Message-----
>>> From: Thomas Hellstrom [mailto:thomas@shipmail.org]
>>> Sent: Wednesday, February 07, 2018 2:43 PM
>>> To: He, Roger <Hongbo.He@amd.com>; amd-gfx@lists.freedesktop.org; 
>>> dri-devel@lists.freedesktop.org
>>> Cc: Koenig, Christian <Christian.Koenig@amd.com>
>>> Subject: Re: [PATCH 0/5] prevent OOM triggered by TTM
>>>
>>> Hi, Roger,
>>>
>>> On 02/06/2018 10:04 AM, Roger He wrote:
>>>> currently ttm code has no any allocation limit. So it allows pages
>>>> allocatation unlimited until OOM. Because if swap space is full of
>>>> swapped pages and then system memory will be filled up with ttm pages.
>>>> and then any memory allocation request will trigger OOM.
>>>>
>>> I'm a bit curious, isn't this the way things are supposed to work on 
>>> a linux system?
>>> If all memory resources are used up, the OOM killer will kill the 
>>> most memory hungry (perhaps rogue) process rather than processes 
>>> being nice and try to find out themselves whether allocations will 
>>> succeed?
>>> Why should TTM be different in that aspect? It would be good to know 
>>> your reasoning WRT this?
>>>
>>> Admittedly, graphics process OOM memory accounting doesn't work very 
>>> well, due to not all BOs not being CPU mapped, but it looks like 
>>> there is recent work towards fixing this?
>>>
>>> One thing I looked at at one point was to have TTM do the swapping 
>>> itself instead of handing it off to the shmem system. That way we 
>>> could pre-allocate swap entries for all swappable (BO) memory, 
>>> making sure that we wouldn't run out of swap space when, for 
>>> example, hibernating and that would also limit the pinned 
>>> non-swappable memory (from TTM driver kernel allocations for 
>>> example) to half the system memory resources.
>>>
>>> Thanks,
>>> Thomas
>>>
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-02-07 18:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-06  9:04 [PATCH 0/5] prevent OOM triggered by TTM Roger He
     [not found] ` <1517907874-21248-1-git-send-email-Hongbo.He-5C7GfCeVMHo@public.gmane.org>
2018-02-06  9:04   ` [PATCH 1/5] drm/ttm: check if the free swap space is under limit 256MB Roger He
2018-02-06  9:04   ` [PATCH 2/5] drm/ttm: keep original behavior except with flag no_retry Roger He
2018-02-06  9:04   ` [PATCH 3/5] drm/ttm: use bit flag to replace allow_reserved_eviction in ttm_operation_ctx Roger He
2018-02-06  9:04   ` [PATCH 4/5] drm/ttm: add bit flag TTM_OPT_FLAG_ALLOW_ALLOC_ANYWAY Roger He
2018-02-06  9:04 ` [PATCH 5/5] drm/ttm: add input parameter allow_allo_anyway for ttm_bo_evict_mm Roger He
2018-02-06  9:51 ` [PATCH 0/5] prevent OOM triggered by TTM Christian König
2018-02-06 10:07   ` He, Roger
2018-02-07  6:43 ` Thomas Hellstrom
2018-02-07  8:25   ` He, Roger
2018-02-07  8:41     ` Christian König
     [not found]       ` <d617ad27-095d-f17d-d310-89caf21d12ca-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-02-07 10:19         ` He, Roger
     [not found]     ` <MWHPR1201MB0127DA7619E7AE8C95D8FE8DFDFC0-3iK1xFAIwjq9imrIu4W8xGrFom/aUZj6nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
2018-02-07 13:17       ` Thomas Hellstrom
2018-02-07 13:22         ` Christian König
     [not found]           ` <4b46ea00-ddb4-a316-02b8-247bd3812a14-5C7GfCeVMHo@public.gmane.org>
2018-02-07 18:07             ` Thomas Hellstrom

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.