| Message ID | 1412834579-24703-1-git-send-email-michel@daenzer.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 09.10.2014 um 08:02 schrieb Michel Dänzer: > From: Michel Dänzer <michel.daenzer@amd.com> > > The radeon driver uses placement range restrictions for several reasons, > in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > during a page fault. > > Without this change, TTM could evict other BOs while trying to satisfy > the requested placement, even if the evicted BOs were outside of the > requested placement range. Doing so didn't free up any space in the > requested placement range, so the (potentially high) eviction cost was > incurred for no benefit. > > Nominating for stable because radeon driver changes in 3.17 made this > much more noticeable than before. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > Cc: stable@vger.kernel.org > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 8f5cec6..407fa2d 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -709,6 +709,7 @@ out: > > static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > uint32_t mem_type, > + const struct ttm_place *place, > bool interruptible, > bool no_wait_gpu) > { > @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > list_for_each_entry(bo, &man->lru, lru) { > ret = __ttm_bo_reserve(bo, false, true, false, NULL); > - if (!ret) > + if (!ret) { > + if (place && (place->fpfn || place->lpfn)) { > + /* Don't evict this BO if it's outside of the > + * requested placement range > + */ > + if (place->fpfn >= (bo->mem.start + bo->mem.size) || > + (place->lpfn && place->lpfn <= bo->mem.start)) { > + __ttm_bo_unreserve(bo); > + ret = -EBUSY; > + continue; > + } > + } > + > break; > + } > } > > if (ret) { > @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, > return ret; > if (mem->mm_node) > break; > - ret = ttm_mem_evict_first(bdev, mem_type, > + ret = ttm_mem_evict_first(bdev, mem_type, place, > interruptible, no_wait_gpu); > if (unlikely(ret != 0)) > return ret; > @@ -1233,7 +1247,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > while (!list_empty(&man->lru)) { > spin_unlock(&glob->lru_lock); > - ret = ttm_mem_evict_first(bdev, mem_type, false, false); > + ret = ttm_mem_evict_first(bdev, mem_type, NULL, false, false); > if (ret) { > if (allow_errors) { > return ret;
On 2014-10-09 07:02, Michel Dänzer wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > The radeon driver uses placement range restrictions for several > reasons, > in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > during a page fault. > > Without this change, TTM could evict other BOs while trying to satisfy > the requested placement, even if the evicted BOs were outside of the > requested placement range. Doing so didn't free up any space in the > requested placement range, so the (potentially high) eviction cost was > incurred for no benefit. > > Nominating for stable because radeon driver changes in 3.17 made this > much more noticeable than before. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > Cc: stable@vger.kernel.org > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c > b/drivers/gpu/drm/ttm/ttm_bo.c > index 8f5cec6..407fa2d 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -709,6 +709,7 @@ out: > > static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > uint32_t mem_type, > + const struct ttm_place *place, > bool interruptible, > bool no_wait_gpu) > { > @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct > ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > list_for_each_entry(bo, &man->lru, lru) { > ret = __ttm_bo_reserve(bo, false, true, false, NULL); > - if (!ret) > + if (!ret) { > + if (place && (place->fpfn || place->lpfn)) { > + /* Don't evict this BO if it's outside of the > + * requested placement range > + */ > + if (place->fpfn >= (bo->mem.start + bo->mem.size) || > + (place->lpfn && place->lpfn <= bo->mem.start)) { > + __ttm_bo_unreserve(bo); > + ret = -EBUSY; > + continue; > + } > + } > + > break; > + } > } > > if (ret) { > @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct > ttm_buffer_object *bo, > return ret; > if (mem->mm_node) > break; > - ret = ttm_mem_evict_first(bdev, mem_type, > + ret = ttm_mem_evict_first(bdev, mem_type, place, > interruptible, no_wait_gpu); > if (unlikely(ret != 0)) > return ret; > @@ -1233,7 +1247,7 @@ static int ttm_bo_force_list_clean(struct > ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > while (!list_empty(&man->lru)) { > spin_unlock(&glob->lru_lock); > - ret = ttm_mem_evict_first(bdev, mem_type, false, false); > + ret = ttm_mem_evict_first(bdev, mem_type, NULL, false, false); > if (ret) { > if (allow_errors) { > return ret; I believe you need to "s/place/placement/" over this patch.
On 09.10.2014 19:22, Alan Swanson wrote: > On 2014-10-09 07:02, Michel Dänzer wrote: >> From: Michel Dänzer <michel.daenzer@amd.com> >> >> The radeon driver uses placement range restrictions for several reasons, >> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. >> during a page fault. >> >> Without this change, TTM could evict other BOs while trying to satisfy >> the requested placement, even if the evicted BOs were outside of the >> requested placement range. Doing so didn't free up any space in the >> requested placement range, so the (potentially high) eviction cost was >> incurred for no benefit. >> >> Nominating for stable because radeon driver changes in 3.17 made this >> much more noticeable than before. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 >> Cc: stable@vger.kernel.org >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- >> 1 file changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index 8f5cec6..407fa2d 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -709,6 +709,7 @@ out: >> >> static int ttm_mem_evict_first(struct ttm_bo_device *bdev, >> uint32_t mem_type, >> + const struct ttm_place *place, >> bool interruptible, >> bool no_wait_gpu) >> { >> @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct >> ttm_bo_device *bdev, >> spin_lock(&glob->lru_lock); >> list_for_each_entry(bo, &man->lru, lru) { >> ret = __ttm_bo_reserve(bo, false, true, false, NULL); >> - if (!ret) >> + if (!ret) { >> + if (place && (place->fpfn || place->lpfn)) { >> + /* Don't evict this BO if it's outside of the >> + * requested placement range >> + */ >> + if (place->fpfn >= (bo->mem.start + bo->mem.size) || >> + (place->lpfn && place->lpfn <= bo->mem.start)) { >> + __ttm_bo_unreserve(bo); >> + ret = -EBUSY; >> + continue; >> + } >> + } >> + >> break; >> + } >> } >> >> if (ret) { >> @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct >> ttm_buffer_object *bo, >> return ret; >> if (mem->mm_node) >> break; >> - ret = ttm_mem_evict_first(bdev, mem_type, >> + ret = ttm_mem_evict_first(bdev, mem_type, place, >> interruptible, no_wait_gpu); >> if (unlikely(ret != 0)) >> return ret; [...] > I believe you need to "s/place/placement/" over this patch. The fpfn and lpfn members were moved from struct ttm_placement to a new struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. If you mean something else, please elaborate.
On Fri, 2014-10-10 at 12:20 +0900, Michel Dänzer wrote: > On 09.10.2014 19:22, Alan Swanson wrote: > > On 2014-10-09 07:02, Michel Dänzer wrote: > >> From: Michel Dänzer <michel.daenzer@amd.com> > >> > >> The radeon driver uses placement range restrictions for several reasons, > >> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > >> during a page fault. > >> > >> Without this change, TTM could evict other BOs while trying to satisfy > >> the requested placement, even if the evicted BOs were outside of the > >> requested placement range. Doing so didn't free up any space in the > >> requested placement range, so the (potentially high) eviction cost was > >> incurred for no benefit. > >> > >> Nominating for stable because radeon driver changes in 3.17 made this > >> much more noticeable than before. > >> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > >> Cc: stable@vger.kernel.org > >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > >> --- > >> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > >> 1 file changed, 17 insertions(+), 3 deletions(-) > >> > [...] > > > I believe you need to "s/place/placement/" over this patch. > > The fpfn and lpfn members were moved from struct ttm_placement to a new > struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. > > If you mean something else, please elaborate. This patch failed to build on 3.17.0 so wouldn't be a candidate for stable unless the currently drm-next only ttm_place patch also goes to stable (else replace ttm_place with ttm_placements in the patch for stable)?
On 10.10.2014 17:51, Alan Swanson wrote: > On Fri, 2014-10-10 at 12:20 +0900, Michel Dänzer wrote: >> On 09.10.2014 19:22, Alan Swanson wrote: >>> On 2014-10-09 07:02, Michel Dänzer wrote: >>>> From: Michel Dänzer <michel.daenzer@amd.com> >>>> >>>> The radeon driver uses placement range restrictions for several reasons, >>>> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. >>>> during a page fault. >>>> >>>> Without this change, TTM could evict other BOs while trying to satisfy >>>> the requested placement, even if the evicted BOs were outside of the >>>> requested placement range. Doing so didn't free up any space in the >>>> requested placement range, so the (potentially high) eviction cost was >>>> incurred for no benefit. >>>> >>>> Nominating for stable because radeon driver changes in 3.17 made this >>>> much more noticeable than before. >>>> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >>>> --- >>>> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- >>>> 1 file changed, 17 insertions(+), 3 deletions(-) >>>> >> [...] >> >>> I believe you need to "s/place/placement/" over this patch. >> >> The fpfn and lpfn members were moved from struct ttm_placement to a new >> struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. >> >> If you mean something else, please elaborate. > > This patch failed to build on 3.17.0 so wouldn't be a candidate for > stable unless the currently drm-next only ttm_place patch also goes to > stable (else replace ttm_place with ttm_placements in the patch for > stable)? Right, I guess I should drop the Cc: stable then and submit a manual backport of it to the stable list once it has landed in Linus' tree.
On Fri, Oct 10, 2014 at 05:59:19PM +0900, Michel Dänzer wrote: > On 10.10.2014 17:51, Alan Swanson wrote: > >On Fri, 2014-10-10 at 12:20 +0900, Michel Dänzer wrote: > >>On 09.10.2014 19:22, Alan Swanson wrote: > >>>On 2014-10-09 07:02, Michel Dänzer wrote: > >>>>From: Michel Dänzer <michel.daenzer@amd.com> > >>>> > >>>>The radeon driver uses placement range restrictions for several reasons, > >>>>in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > >>>>during a page fault. > >>>> > >>>>Without this change, TTM could evict other BOs while trying to satisfy > >>>>the requested placement, even if the evicted BOs were outside of the > >>>>requested placement range. Doing so didn't free up any space in the > >>>>requested placement range, so the (potentially high) eviction cost was > >>>>incurred for no benefit. > >>>> > >>>>Nominating for stable because radeon driver changes in 3.17 made this > >>>>much more noticeable than before. > >>>> > >>>>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > >>>>Cc: stable@vger.kernel.org > >>>>Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > >>>>--- > >>>> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > >>>> 1 file changed, 17 insertions(+), 3 deletions(-) > >>>> > >>[...] > >> > >>>I believe you need to "s/place/placement/" over this patch. > >> > >>The fpfn and lpfn members were moved from struct ttm_placement to a new > >>struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. > >> > >>If you mean something else, please elaborate. > > > >This patch failed to build on 3.17.0 so wouldn't be a candidate for > >stable unless the currently drm-next only ttm_place patch also goes to > >stable (else replace ttm_place with ttm_placements in the patch for > >stable)? > > Right, I guess I should drop the Cc: stable then and submit a manual > backport of it to the stable list once it has landed in Linus' tree. I've thought it's ok to cc: stable a patch - Greg's scripts will send you a mail as a nice reminder if the patch fails to apply. At least we regularly pull this stunt with i915 patches. Cc'ing Greg for clarification. -Daniel
On Sat, Oct 11, 2014 at 08:31:49PM +0200, Daniel Vetter wrote: > On Fri, Oct 10, 2014 at 05:59:19PM +0900, Michel Dänzer wrote: > > On 10.10.2014 17:51, Alan Swanson wrote: > > >On Fri, 2014-10-10 at 12:20 +0900, Michel Dänzer wrote: > > >>On 09.10.2014 19:22, Alan Swanson wrote: > > >>>On 2014-10-09 07:02, Michel Dänzer wrote: > > >>>>From: Michel Dänzer <michel.daenzer@amd.com> > > >>>> > > >>>>The radeon driver uses placement range restrictions for several reasons, > > >>>>in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > > >>>>during a page fault. > > >>>> > > >>>>Without this change, TTM could evict other BOs while trying to satisfy > > >>>>the requested placement, even if the evicted BOs were outside of the > > >>>>requested placement range. Doing so didn't free up any space in the > > >>>>requested placement range, so the (potentially high) eviction cost was > > >>>>incurred for no benefit. > > >>>> > > >>>>Nominating for stable because radeon driver changes in 3.17 made this > > >>>>much more noticeable than before. > > >>>> > > >>>>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > > >>>>Cc: stable@vger.kernel.org > > >>>>Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > > >>>>--- > > >>>> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > > >>>> 1 file changed, 17 insertions(+), 3 deletions(-) > > >>>> > > >>[...] > > >> > > >>>I believe you need to "s/place/placement/" over this patch. > > >> > > >>The fpfn and lpfn members were moved from struct ttm_placement to a new > > >>struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. > > >> > > >>If you mean something else, please elaborate. > > > > > >This patch failed to build on 3.17.0 so wouldn't be a candidate for > > >stable unless the currently drm-next only ttm_place patch also goes to > > >stable (else replace ttm_place with ttm_placements in the patch for > > >stable)? > > > > Right, I guess I should drop the Cc: stable then and submit a manual > > backport of it to the stable list once it has landed in Linus' tree. > > I've thought it's ok to cc: stable a patch - Greg's scripts will send you > a mail as a nice reminder if the patch fails to apply. At least we > regularly pull this stunt with i915 patches. Cc'ing Greg for > clarification. Yup, that's fine to do, it's what the scripts are for :)
On Thu, Oct 9, 2014 at 2:02 AM, Michel Dänzer <michel@daenzer.net> wrote: > From: Michel Dänzer <michel.daenzer@amd.com> > > The radeon driver uses placement range restrictions for several reasons, > in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. > during a page fault. > > Without this change, TTM could evict other BOs while trying to satisfy > the requested placement, even if the evicted BOs were outside of the > requested placement range. Doing so didn't free up any space in the > requested placement range, so the (potentially high) eviction cost was > incurred for no benefit. > > Nominating for stable because radeon driver changes in 3.17 made this > much more noticeable than before. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 > Cc: stable@vger.kernel.org > Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Thomas, do you want to pull this through the ttm tree or can I take it through radeon? Alex > --- > drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 8f5cec6..407fa2d 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -709,6 +709,7 @@ out: > > static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > uint32_t mem_type, > + const struct ttm_place *place, > bool interruptible, > bool no_wait_gpu) > { > @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > list_for_each_entry(bo, &man->lru, lru) { > ret = __ttm_bo_reserve(bo, false, true, false, NULL); > - if (!ret) > + if (!ret) { > + if (place && (place->fpfn || place->lpfn)) { > + /* Don't evict this BO if it's outside of the > + * requested placement range > + */ > + if (place->fpfn >= (bo->mem.start + bo->mem.size) || > + (place->lpfn && place->lpfn <= bo->mem.start)) { > + __ttm_bo_unreserve(bo); > + ret = -EBUSY; > + continue; > + } > + } > + > break; > + } > } > > if (ret) { > @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, > return ret; > if (mem->mm_node) > break; > - ret = ttm_mem_evict_first(bdev, mem_type, > + ret = ttm_mem_evict_first(bdev, mem_type, place, > interruptible, no_wait_gpu); > if (unlikely(ret != 0)) > return ret; > @@ -1233,7 +1247,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev, > spin_lock(&glob->lru_lock); > while (!list_empty(&man->lru)) { > spin_unlock(&glob->lru_lock); > - ret = ttm_mem_evict_first(bdev, mem_type, false, false); > + ret = ttm_mem_evict_first(bdev, mem_type, NULL, false, false); > if (ret) { > if (allow_errors) { > return ret; > -- > 2.1.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex, Please pull these patches through Radeon, Thanks, Thomas On 10/13/2014 08:22 PM, Alex Deucher wrote: > On Thu, Oct 9, 2014 at 2:02 AM, Michel Dänzer <michel@daenzer.net> wrote: >> From: Michel Dänzer <michel.daenzer@amd.com> >> >> The radeon driver uses placement range restrictions for several reasons, >> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. >> during a page fault. >> >> Without this change, TTM could evict other BOs while trying to satisfy >> the requested placement, even if the evicted BOs were outside of the >> requested placement range. Doing so didn't free up any space in the >> requested placement range, so the (potentially high) eviction cost was >> incurred for no benefit. >> >> Nominating for stable because radeon driver changes in 3.17 made this >> much more noticeable than before. >> >> Bugzilla: https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D84662&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=fUnmlW1Tlnje4oNF3duZPPLcUcRvxyQZ%2BA%2Fyl5YOGBc%3D%0A&s=53d70e00aa98306fc71f222fe9aa14b3ff9cf30dab9a060633a2341526cb09cb >> Cc: stable@vger.kernel.org >> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > > Thomas, do you want to pull this through the ttm tree or can I take it > through radeon? > > Alex > >> --- >> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- >> 1 file changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index 8f5cec6..407fa2d 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -709,6 +709,7 @@ out: >> >> static int ttm_mem_evict_first(struct ttm_bo_device *bdev, >> uint32_t mem_type, >> + const struct ttm_place *place, >> bool interruptible, >> bool no_wait_gpu) >> { >> @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, >> spin_lock(&glob->lru_lock); >> list_for_each_entry(bo, &man->lru, lru) { >> ret = __ttm_bo_reserve(bo, false, true, false, NULL); >> - if (!ret) >> + if (!ret) { >> + if (place && (place->fpfn || place->lpfn)) { >> + /* Don't evict this BO if it's outside of the >> + * requested placement range >> + */ >> + if (place->fpfn >= (bo->mem.start + bo->mem.size) || >> + (place->lpfn && place->lpfn <= bo->mem.start)) { >> + __ttm_bo_unreserve(bo); >> + ret = -EBUSY; >> + continue; >> + } >> + } >> + >> break; >> + } >> } >> >> if (ret) { >> @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, >> return ret; >> if (mem->mm_node) >> break; >> - ret = ttm_mem_evict_first(bdev, mem_type, >> + ret = ttm_mem_evict_first(bdev, mem_type, place, >> interruptible, no_wait_gpu); >> if (unlikely(ret != 0)) >> return ret; >> @@ -1233,7 +1247,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev, >> spin_lock(&glob->lru_lock); >> while (!list_empty(&man->lru)) { >> spin_unlock(&glob->lru_lock); >> - ret = ttm_mem_evict_first(bdev, mem_type, false, false); >> + ret = ttm_mem_evict_first(bdev, mem_type, NULL, false, false); >> if (ret) { >> if (allow_errors) { >> return ret; >> -- >> 2.1.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/dri-devel&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=fUnmlW1Tlnje4oNF3duZPPLcUcRvxyQZ%2BA%2Fyl5YOGBc%3D%0A&s=3c28f2dc220442e12a62cd069fd203cefbe4b157be43987cebd908d2084e32e9
On 12.10.2014 05:24, Greg KH wrote: > On Sat, Oct 11, 2014 at 08:31:49PM +0200, Daniel Vetter wrote: >> On Fri, Oct 10, 2014 at 05:59:19PM +0900, Michel Dänzer wrote: >>> On 10.10.2014 17:51, Alan Swanson wrote: >>>> On Fri, 2014-10-10 at 12:20 +0900, Michel Dänzer wrote: >>>>> On 09.10.2014 19:22, Alan Swanson wrote: >>>>>> On 2014-10-09 07:02, Michel Dänzer wrote: >>>>>>> From: Michel Dänzer <michel.daenzer@amd.com> >>>>>>> >>>>>>> The radeon driver uses placement range restrictions for several reasons, >>>>>>> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g. >>>>>>> during a page fault. >>>>>>> >>>>>>> Without this change, TTM could evict other BOs while trying to satisfy >>>>>>> the requested placement, even if the evicted BOs were outside of the >>>>>>> requested placement range. Doing so didn't free up any space in the >>>>>>> requested placement range, so the (potentially high) eviction cost was >>>>>>> incurred for no benefit. >>>>>>> >>>>>>> Nominating for stable because radeon driver changes in 3.17 made this >>>>>>> much more noticeable than before. >>>>>>> >>>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662 >>>>>>> Cc: stable@vger.kernel.org >>>>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++--- >>>>>>> 1 file changed, 17 insertions(+), 3 deletions(-) >>>>>>> >>>>> [...] >>>>> >>>>>> I believe you need to "s/place/placement/" over this patch. >>>>> >>>>> The fpfn and lpfn members were moved from struct ttm_placement to a new >>>>> struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65. >>>>> >>>>> If you mean something else, please elaborate. >>>> >>>> This patch failed to build on 3.17.0 so wouldn't be a candidate for >>>> stable unless the currently drm-next only ttm_place patch also goes to >>>> stable (else replace ttm_place with ttm_placements in the patch for >>>> stable)? >>> >>> Right, I guess I should drop the Cc: stable then and submit a manual >>> backport of it to the stable list once it has landed in Linus' tree. >> >> I've thought it's ok to cc: stable a patch - Greg's scripts will send you >> a mail as a nice reminder if the patch fails to apply. At least we >> regularly pull this stunt with i915 patches. Cc'ing Greg for >> clarification. > > Yup, that's fine to do, it's what the scripts are for :) Okay, thanks guys for the clarification.
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 8f5cec6..407fa2d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -709,6 +709,7 @@ out: static int ttm_mem_evict_first(struct ttm_bo_device *bdev, uint32_t mem_type, + const struct ttm_place *place, bool interruptible, bool no_wait_gpu) { @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, spin_lock(&glob->lru_lock); list_for_each_entry(bo, &man->lru, lru) { ret = __ttm_bo_reserve(bo, false, true, false, NULL); - if (!ret) + if (!ret) { + if (place && (place->fpfn || place->lpfn)) { + /* Don't evict this BO if it's outside of the + * requested placement range + */ + if (place->fpfn >= (bo->mem.start + bo->mem.size) || + (place->lpfn && place->lpfn <= bo->mem.start)) { + __ttm_bo_unreserve(bo); + ret = -EBUSY; + continue; + } + } + break; + } } if (ret) { @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, return ret; if (mem->mm_node) break; - ret = ttm_mem_evict_first(bdev, mem_type, + ret = ttm_mem_evict_first(bdev, mem_type, place, interruptible, no_wait_gpu); if (unlikely(ret != 0)) return ret; @@ -1233,7 +1247,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev, spin_lock(&glob->lru_lock); while (!list_empty(&man->lru)) { spin_unlock(&glob->lru_lock); - ret = ttm_mem_evict_first(bdev, mem_type, false, false); + ret = ttm_mem_evict_first(bdev, mem_type, NULL, false, false); if (ret) { if (allow_errors) { return ret;