From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Matthew Brost <matthew.brost@intel.com>
Cc: Stuart Summers <stuart.summers@intel.com>,
Vivi@dut025-tglu.fm.intel.com, "Chang,
Bruce" <yu.bruce.chang@intel.com>,
intel-xe@lists.freedesktop.org
Subject: Re: [Intel-xe] [PATCH] drm/xe: fix pvc unload issue
Date: Tue, 4 Apr 2023 12:29:00 -0400 [thread overview]
Message-ID: <ZCxQTG1+z698Pac+@intel.com> (raw)
In-Reply-To: <ZCudEneYy6bOCqCa@DUT025-TGLU.fm.intel.com>
On Tue, Apr 04, 2023 at 03:44:18AM +0000, Matthew Brost wrote:
> On Mon, Apr 03, 2023 at 10:20:31PM +0000, Chang, Bruce wrote:
> > Currently, unload pvc driver will generate a null dereference
> > and the call stack is as below.
> >
> > [ 4850.618000] Call Trace:
> > [ 4850.620740] <TASK>
> > [ 4850.623134] ttm_bo_cleanup_memtype_use+0x3f/0x50 [ttm]
> > [ 4850.628661] ttm_bo_release+0x154/0x2c0 [ttm]
> > [ 4850.633317] ? drm_buddy_fini+0x62/0x80 [drm_buddy]
> > [ 4850.638487] ? __kmem_cache_free+0x27d/0x2c0
> > [ 4850.643054] ttm_bo_put+0x38/0x60 [ttm]
> > [ 4850.647190] xe_gem_object_free+0x1f/0x30 [xe]
> > [ 4850.651945] drm_gem_object_free+0x1e/0x30 [drm]
> > [ 4850.656904] ggtt_fini_noalloc+0x9d/0xe0 [xe]
> > [ 4850.661574] drm_managed_release+0xb5/0x150 [drm]
> > [ 4850.666617] drm_dev_release+0x30/0x50 [drm]
> > [ 4850.671209] devm_drm_dev_init_release+0x3c/0x60 [drm]
> >
> > There are a couple issues, but the main one is due to TTM has only
> > one TTM_PL_TT region, but since pvc has 2 tiles and tries to setup
> > 1 TTM_PL_TT each tile. The second will overwrite the first one.
> >
> > During unload time, the first tile will reset the TTM_PL_TT manger
> > and when the second tile is trying to free Bo and it will generate
> > the null reference since the TTM manage is already got reset to 0.
> >
> > The fix is to use one global TTM_PL_TT manager.
> >
> > v2: make gtt mgr global and change the name to sys_mgr
> >
> > Cc: Stuart Summers <stuart.summers@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com>
>
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
pushed to drm-xe-next. Thanks for the patch and review.
>
> > ---
> > drivers/gpu/drm/xe/Makefile | 2 +-
> > drivers/gpu/drm/xe/xe_device.c | 2 +
> > drivers/gpu/drm/xe/xe_device.h | 1 +
> > drivers/gpu/drm/xe/xe_device_types.h | 2 +
> > drivers/gpu/drm/xe/xe_gt.c | 18 -----
> > drivers/gpu/drm/xe/xe_gt_types.h | 2 -
> > drivers/gpu/drm/xe/xe_ttm_gtt_mgr.h | 16 -----
> > drivers/gpu/drm/xe/xe_ttm_gtt_mgr_types.h | 18 -----
> > .../xe/{xe_ttm_gtt_mgr.c => xe_ttm_sys_mgr.c} | 67 +++++++------------
> > 9 files changed, 31 insertions(+), 97 deletions(-)
> > delete mode 100644 drivers/gpu/drm/xe/xe_ttm_gtt_mgr.h
> > delete mode 100644 drivers/gpu/drm/xe/xe_ttm_gtt_mgr_types.h
> > rename drivers/gpu/drm/xe/{xe_ttm_gtt_mgr.c => xe_ttm_sys_mgr.c} (52%)
> >
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index 669c7b6f552c..7e19a15af3dc 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -83,7 +83,7 @@ xe-y += xe_bb.o \
> > xe_step.o \
> > xe_sync.o \
> > xe_trace.o \
> > - xe_ttm_gtt_mgr.o \
> > + xe_ttm_sys_mgr.o \
> > xe_ttm_stolen_mgr.o \
> > xe_ttm_vram_mgr.o \
> > xe_tuning.o \
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index a79f934e3d2d..4fb9aff27686 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -279,6 +279,8 @@ int xe_device_probe(struct xe_device *xe)
> > if (err)
> > goto err_irq_shutdown;
> >
> > + xe_ttm_sys_mgr_init(xe);
> > +
> > for_each_gt(gt, xe, id) {
> > err = xe_gt_init_noalloc(gt);
> > if (err)
> > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> > index d277f8985f7b..d9d1b09a8e38 100644
> > --- a/drivers/gpu/drm/xe/xe_device.h
> > +++ b/drivers/gpu/drm/xe/xe_device.h
> > @@ -116,4 +116,5 @@ static inline bool xe_device_has_flat_ccs(struct xe_device *xe)
> > }
> >
> > u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size);
> > +int xe_ttm_sys_mgr_init(struct xe_device *xe);
> > #endif
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > index 88f863edc41c..6fb53a54913f 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -195,6 +195,8 @@ struct xe_device {
> > /** @mapping: pointer to VRAM mappable space */
> > void *__iomem mapping;
> > } vram;
> > + /** @sys_mgr: system TTM manager */
> > + struct ttm_resource_manager sys_mgr;
> > } mem;
> >
> > /** @usm: unified memory state */
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index fd7a5b43ba3e..a6ea138a7877 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -36,7 +36,6 @@
> > #include "xe_ring_ops.h"
> > #include "xe_sa.h"
> > #include "xe_sched_job.h"
> > -#include "xe_ttm_gtt_mgr.h"
> > #include "xe_ttm_vram_mgr.h"
> > #include "xe_tuning.h"
> > #include "xe_uc.h"
> > @@ -77,16 +76,11 @@ int xe_gt_alloc(struct xe_device *xe, struct xe_gt *gt)
> > if (!gt->mem.vram_mgr)
> > return -ENOMEM;
> >
> > - gt->mem.gtt_mgr = drmm_kzalloc(drm, sizeof(*gt->mem.gtt_mgr),
> > - GFP_KERNEL);
> > - if (!gt->mem.gtt_mgr)
> > - return -ENOMEM;
> > } else {
> > struct xe_gt *full_gt = xe_find_full_gt(gt);
> >
> > gt->mem.ggtt = full_gt->mem.ggtt;
> > gt->mem.vram_mgr = full_gt->mem.vram_mgr;
> > - gt->mem.gtt_mgr = full_gt->mem.gtt_mgr;
> > }
> >
> > gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0);
> > @@ -98,26 +92,14 @@ static int gt_ttm_mgr_init(struct xe_gt *gt)
> > {
> > struct xe_device *xe = gt_to_xe(gt);
> > int err;
> > - struct sysinfo si;
> > - u64 gtt_size;
> > -
> > - si_meminfo(&si);
> > - gtt_size = (u64)si.totalram * si.mem_unit * 3/4;
> >
> > if (gt->mem.vram.size) {
> > err = xe_ttm_vram_mgr_init(gt, gt->mem.vram_mgr);
> > if (err)
> > return err;
> > - gtt_size = min(max((XE_DEFAULT_GTT_SIZE_MB << 20),
> > - (u64)gt->mem.vram.size),
> > - gtt_size);
> > xe->info.mem_region_mask |= BIT(gt->info.vram_id) << 1;
> > }
> >
> > - err = xe_ttm_gtt_mgr_init(gt, gt->mem.gtt_mgr, gtt_size);
> > - if (err)
> > - return err;
> > -
> > return 0;
> > }
> >
> > diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> > index 8f29aba455e0..9d3117fad2e4 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_types.h
> > +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> > @@ -162,8 +162,6 @@ struct xe_gt {
> > } vram;
> > /** @vram_mgr: VRAM TTM manager */
> > struct xe_ttm_vram_mgr *vram_mgr;
> > - /** @gtt_mr: GTT TTM manager */
> > - struct xe_ttm_gtt_mgr *gtt_mgr;
> > /** @ggtt: Global graphics translation table */
> > struct xe_ggtt *ggtt;
> > } mem;
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr.h b/drivers/gpu/drm/xe/xe_ttm_gtt_mgr.h
> > deleted file mode 100644
> > index d1d57cb9c2b8..000000000000
> > --- a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr.h
> > +++ /dev/null
> > @@ -1,16 +0,0 @@
> > -/* SPDX-License-Identifier: MIT */
> > -/*
> > - * Copyright © 2022 Intel Corporation
> > - */
> > -
> > -#ifndef _XE_TTGM_GTT_MGR_H_
> > -#define _XE_TTGM_GTT_MGR_H_
> > -
> > -#include "xe_ttm_gtt_mgr_types.h"
> > -
> > -struct xe_gt;
> > -
> > -int xe_ttm_gtt_mgr_init(struct xe_gt *gt, struct xe_ttm_gtt_mgr *mgr,
> > - u64 gtt_size);
> > -
> > -#endif
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr_types.h b/drivers/gpu/drm/xe/xe_ttm_gtt_mgr_types.h
> > deleted file mode 100644
> > index c66737488326..000000000000
> > --- a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr_types.h
> > +++ /dev/null
> > @@ -1,18 +0,0 @@
> > -/* SPDX-License-Identifier: MIT */
> > -/*
> > - * Copyright © 2022 Intel Corporation
> > - */
> > -
> > -#ifndef _XE_TTM_GTT_MGR_TYPES_H_
> > -#define _XE_TTM_GTT_MGR_TYPES_H_
> > -
> > -#include <drm/ttm/ttm_device.h>
> > -
> > -struct xe_gt;
> > -
> > -struct xe_ttm_gtt_mgr {
> > - struct xe_gt *gt;
> > - struct ttm_resource_manager manager;
> > -};
> > -
> > -#endif
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr.c b/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
> > similarity index 52%
> > rename from drivers/gpu/drm/xe/xe_ttm_gtt_mgr.c
> > rename to drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
> > index 8075781070f2..cf5f4f73d4dc 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_gtt_mgr.c
> > +++ b/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
> > @@ -12,31 +12,24 @@
> >
> > #include "xe_bo.h"
> > #include "xe_gt.h"
> > -#include "xe_ttm_gtt_mgr.h"
> >
> > -struct xe_ttm_gtt_node {
> > +struct xe_ttm_sys_node {
> > struct ttm_buffer_object *tbo;
> > struct ttm_range_mgr_node base;
> > };
> >
> > -static inline struct xe_ttm_gtt_mgr *
> > -to_gtt_mgr(struct ttm_resource_manager *man)
> > +static inline struct xe_ttm_sys_node *
> > +to_xe_ttm_sys_node(struct ttm_resource *res)
> > {
> > - return container_of(man, struct xe_ttm_gtt_mgr, manager);
> > + return container_of(res, struct xe_ttm_sys_node, base.base);
> > }
> >
> > -static inline struct xe_ttm_gtt_node *
> > -to_xe_ttm_gtt_node(struct ttm_resource *res)
> > -{
> > - return container_of(res, struct xe_ttm_gtt_node, base.base);
> > -}
> > -
> > -static int xe_ttm_gtt_mgr_new(struct ttm_resource_manager *man,
> > +static int xe_ttm_sys_mgr_new(struct ttm_resource_manager *man,
> > struct ttm_buffer_object *tbo,
> > const struct ttm_place *place,
> > struct ttm_resource **res)
> > {
> > - struct xe_ttm_gtt_node *node;
> > + struct xe_ttm_sys_node *node;
> > int r;
> >
> > node = kzalloc(struct_size(node, base.mm_nodes, 1), GFP_KERNEL);
> > @@ -66,32 +59,31 @@ static int xe_ttm_gtt_mgr_new(struct ttm_resource_manager *man,
> > return r;
> > }
> >
> > -static void xe_ttm_gtt_mgr_del(struct ttm_resource_manager *man,
> > +static void xe_ttm_sys_mgr_del(struct ttm_resource_manager *man,
> > struct ttm_resource *res)
> > {
> > - struct xe_ttm_gtt_node *node = to_xe_ttm_gtt_node(res);
> > + struct xe_ttm_sys_node *node = to_xe_ttm_sys_node(res);
> >
> > ttm_resource_fini(man, res);
> > kfree(node);
> > }
> >
> > -static void xe_ttm_gtt_mgr_debug(struct ttm_resource_manager *man,
> > +static void xe_ttm_sys_mgr_debug(struct ttm_resource_manager *man,
> > struct drm_printer *printer)
> > {
> >
> > }
> >
> > -static const struct ttm_resource_manager_func xe_ttm_gtt_mgr_func = {
> > - .alloc = xe_ttm_gtt_mgr_new,
> > - .free = xe_ttm_gtt_mgr_del,
> > - .debug = xe_ttm_gtt_mgr_debug
> > +static const struct ttm_resource_manager_func xe_ttm_sys_mgr_func = {
> > + .alloc = xe_ttm_sys_mgr_new,
> > + .free = xe_ttm_sys_mgr_del,
> > + .debug = xe_ttm_sys_mgr_debug
> > };
> >
> > -static void ttm_gtt_mgr_fini(struct drm_device *drm, void *arg)
> > +static void ttm_sys_mgr_fini(struct drm_device *drm, void *arg)
> > {
> > - struct xe_ttm_gtt_mgr *mgr = arg;
> > - struct xe_device *xe = gt_to_xe(mgr->gt);
> > - struct ttm_resource_manager *man = &mgr->manager;
> > + struct xe_device *xe = (struct xe_device *)arg;
> > + struct ttm_resource_manager *man = &xe->mem.sys_mgr;
> > int err;
> >
> > ttm_resource_manager_set_used(man, false);
> > @@ -104,27 +96,18 @@ static void ttm_gtt_mgr_fini(struct drm_device *drm, void *arg)
> > ttm_set_driver_manager(&xe->ttm, XE_PL_TT, NULL);
> > }
> >
> > -int xe_ttm_gtt_mgr_init(struct xe_gt *gt, struct xe_ttm_gtt_mgr *mgr,
> > - u64 gtt_size)
> > +int xe_ttm_sys_mgr_init(struct xe_device *xe)
> > {
> > - struct xe_device *xe = gt_to_xe(gt);
> > - struct ttm_resource_manager *man = &mgr->manager;
> > - int err;
> > -
> > - XE_BUG_ON(xe_gt_is_media_type(gt));
> > + struct ttm_resource_manager *man = &xe->mem.sys_mgr;
> > + struct sysinfo si;
> > + u64 gtt_size;
> >
> > - mgr->gt = gt;
> > + si_meminfo(&si);
> > + gtt_size = (u64)si.totalram * si.mem_unit * 3/4;
> > man->use_tt = true;
> > - man->func = &xe_ttm_gtt_mgr_func;
> > -
> > + man->func = &xe_ttm_sys_mgr_func;
> > ttm_resource_manager_init(man, &xe->ttm, gtt_size >> PAGE_SHIFT);
> > -
> > - ttm_set_driver_manager(&xe->ttm, XE_PL_TT, &mgr->manager);
> > + ttm_set_driver_manager(&xe->ttm, XE_PL_TT, man);
> > ttm_resource_manager_set_used(man, true);
> > -
> > - err = drmm_add_action_or_reset(&xe->drm, ttm_gtt_mgr_fini, mgr);
> > - if (err)
> > - return err;
> > -
> > - return 0;
> > + return drmm_add_action_or_reset(&xe->drm, ttm_sys_mgr_fini, xe);
> > }
> > --
> > 2.25.1
> >
next prev parent reply other threads:[~2023-04-04 16:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-03 22:20 [Intel-xe] [PATCH] drm/xe: fix pvc unload issue Chang, Bruce
2023-04-03 22:31 ` [Intel-xe] ✓ CI.Patch_applied: success for drm/xe: fix pvc unload issue (rev2) Patchwork
2023-04-03 22:32 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
2023-04-03 22:36 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-04-03 22:57 ` [Intel-xe] ○ CI.BAT: info " Patchwork
2023-04-04 3:44 ` [Intel-xe] [PATCH] drm/xe: fix pvc unload issue Matthew Brost
2023-04-04 16:29 ` Rodrigo Vivi [this message]
2023-04-04 18:48 ` Lucas De Marchi
2023-04-04 19:45 ` Rodrigo Vivi
2023-04-04 20:22 ` Lucas De Marchi
-- strict thread matches above, loose matches on Subject: below --
2023-03-30 15:40 Chang, Bruce
2023-03-31 5:40 ` Matthew Auld
2023-03-31 22:55 ` Chang, Yu bruce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZCxQTG1+z698Pac+@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=Vivi@dut025-tglu.fm.intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.brost@intel.com \
--cc=stuart.summers@intel.com \
--cc=yu.bruce.chang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.