| Message ID | 20230320144356.803762-2-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm+PM+icc: Make job_run() reclaim-safe | expand |
Am 20.03.23 um 15:43 schrieb Rob Clark: > From: Rob Clark <robdclark@chromium.org> > > Avoid allocating memory in job_run() by pre-allocating the hw_fence. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/msm_fence.c | 12 +++++++++--- > drivers/gpu/drm/msm/msm_fence.h | 3 ++- > drivers/gpu/drm/msm/msm_gem_submit.c | 7 +++++++ > drivers/gpu/drm/msm/msm_ringbuffer.c | 2 +- > 4 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c > index 56641408ea74..bab3d84f1686 100644 > --- a/drivers/gpu/drm/msm/msm_fence.c > +++ b/drivers/gpu/drm/msm/msm_fence.c > @@ -99,7 +99,7 @@ static const struct dma_fence_ops msm_fence_ops = { > }; > > struct dma_fence * > -msm_fence_alloc(struct msm_fence_context *fctx) > +msm_fence_alloc(void) > { > struct msm_fence *f; > > @@ -107,10 +107,16 @@ msm_fence_alloc(struct msm_fence_context *fctx) > if (!f) > return ERR_PTR(-ENOMEM); > > + return &f->base; > +} > + > +void > +msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx) > +{ > + struct msm_fence *f = to_msm_fence(fence); > + > f->fctx = fctx; > > dma_fence_init(&f->base, &msm_fence_ops, &fctx->spinlock, > fctx->context, ++fctx->last_fence); > - > - return &f->base; > } > diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h > index 7f1798c54cd1..f913fa22d8fe 100644 > --- a/drivers/gpu/drm/msm/msm_fence.h > +++ b/drivers/gpu/drm/msm/msm_fence.h > @@ -61,7 +61,8 @@ void msm_fence_context_free(struct msm_fence_context *fctx); > bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence); > void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence); > > -struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx); > +struct dma_fence * msm_fence_alloc(void); > +void msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx); > > static inline bool > fence_before(uint32_t a, uint32_t b) > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > index be4bf77103cd..2570c018b0cb 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -41,6 +41,13 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, > if (!submit) > return ERR_PTR(-ENOMEM); > > + submit->hw_fence = msm_fence_alloc(); > + if (IS_ERR(submit->hw_fence)) { > + ret = PTR_ERR(submit->hw_fence); > + kfree(submit); > + return ERR_PTR(ret); > + } > + > ret = drm_sched_job_init(&submit->base, queue->entity, queue); > if (ret) { > kfree(submit); You probably need some error handling here or otherwise leak submit->hw_fence. Apart from that looks good to me. Christian. > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c > index 57a8e9564540..a62b45e5a8c3 100644 > --- a/drivers/gpu/drm/msm/msm_ringbuffer.c > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c > @@ -18,7 +18,7 @@ static struct dma_fence *msm_job_run(struct drm_sched_job *job) > struct msm_gpu *gpu = submit->gpu; > int i; > > - submit->hw_fence = msm_fence_alloc(fctx); > + msm_fence_init(submit->hw_fence, fctx); > > for (i = 0; i < submit->nr_bos; i++) { > struct drm_gem_object *obj = &submit->bos[i].obj->base;
On Mon, Mar 20, 2023 at 9:52 AM Christian König <christian.koenig@amd.com> wrote: > > > > Am 20.03.23 um 15:43 schrieb Rob Clark: > > From: Rob Clark <robdclark@chromium.org> > > > > Avoid allocating memory in job_run() by pre-allocating the hw_fence. > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/msm/msm_fence.c | 12 +++++++++--- > > drivers/gpu/drm/msm/msm_fence.h | 3 ++- > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 +++++++ > > drivers/gpu/drm/msm/msm_ringbuffer.c | 2 +- > > 4 files changed, 19 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c > > index 56641408ea74..bab3d84f1686 100644 > > --- a/drivers/gpu/drm/msm/msm_fence.c > > +++ b/drivers/gpu/drm/msm/msm_fence.c > > @@ -99,7 +99,7 @@ static const struct dma_fence_ops msm_fence_ops = { > > }; > > > > struct dma_fence * > > -msm_fence_alloc(struct msm_fence_context *fctx) > > +msm_fence_alloc(void) > > { > > struct msm_fence *f; > > > > @@ -107,10 +107,16 @@ msm_fence_alloc(struct msm_fence_context *fctx) > > if (!f) > > return ERR_PTR(-ENOMEM); > > > > + return &f->base; > > +} > > + > > +void > > +msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx) > > +{ > > + struct msm_fence *f = to_msm_fence(fence); > > + > > f->fctx = fctx; > > > > dma_fence_init(&f->base, &msm_fence_ops, &fctx->spinlock, > > fctx->context, ++fctx->last_fence); > > - > > - return &f->base; > > } > > diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h > > index 7f1798c54cd1..f913fa22d8fe 100644 > > --- a/drivers/gpu/drm/msm/msm_fence.h > > +++ b/drivers/gpu/drm/msm/msm_fence.h > > @@ -61,7 +61,8 @@ void msm_fence_context_free(struct msm_fence_context *fctx); > > bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence); > > void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence); > > > > -struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx); > > +struct dma_fence * msm_fence_alloc(void); > > +void msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx); > > > > static inline bool > > fence_before(uint32_t a, uint32_t b) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > > index be4bf77103cd..2570c018b0cb 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -41,6 +41,13 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, > > if (!submit) > > return ERR_PTR(-ENOMEM); > > > > + submit->hw_fence = msm_fence_alloc(); > > + if (IS_ERR(submit->hw_fence)) { > > + ret = PTR_ERR(submit->hw_fence); > > + kfree(submit); > > + return ERR_PTR(ret); > > + } > > + > > ret = drm_sched_job_init(&submit->base, queue->entity, queue); > > if (ret) { > > kfree(submit); > > You probably need some error handling here or otherwise leak > submit->hw_fence. ah, right.. thx BR, -R > Apart from that looks good to me. > > Christian. > > > diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c > > index 57a8e9564540..a62b45e5a8c3 100644 > > --- a/drivers/gpu/drm/msm/msm_ringbuffer.c > > +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c > > @@ -18,7 +18,7 @@ static struct dma_fence *msm_job_run(struct drm_sched_job *job) > > struct msm_gpu *gpu = submit->gpu; > > int i; > > > > - submit->hw_fence = msm_fence_alloc(fctx); > > + msm_fence_init(submit->hw_fence, fctx); > > > > for (i = 0; i < submit->nr_bos; i++) { > > struct drm_gem_object *obj = &submit->bos[i].obj->base; >
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c index 56641408ea74..bab3d84f1686 100644 --- a/drivers/gpu/drm/msm/msm_fence.c +++ b/drivers/gpu/drm/msm/msm_fence.c @@ -99,7 +99,7 @@ static const struct dma_fence_ops msm_fence_ops = { }; struct dma_fence * -msm_fence_alloc(struct msm_fence_context *fctx) +msm_fence_alloc(void) { struct msm_fence *f; @@ -107,10 +107,16 @@ msm_fence_alloc(struct msm_fence_context *fctx) if (!f) return ERR_PTR(-ENOMEM); + return &f->base; +} + +void +msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx) +{ + struct msm_fence *f = to_msm_fence(fence); + f->fctx = fctx; dma_fence_init(&f->base, &msm_fence_ops, &fctx->spinlock, fctx->context, ++fctx->last_fence); - - return &f->base; } diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h index 7f1798c54cd1..f913fa22d8fe 100644 --- a/drivers/gpu/drm/msm/msm_fence.h +++ b/drivers/gpu/drm/msm/msm_fence.h @@ -61,7 +61,8 @@ void msm_fence_context_free(struct msm_fence_context *fctx); bool msm_fence_completed(struct msm_fence_context *fctx, uint32_t fence); void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence); -struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx); +struct dma_fence * msm_fence_alloc(void); +void msm_fence_init(struct dma_fence *fence, struct msm_fence_context *fctx); static inline bool fence_before(uint32_t a, uint32_t b) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index be4bf77103cd..2570c018b0cb 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -41,6 +41,13 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev, if (!submit) return ERR_PTR(-ENOMEM); + submit->hw_fence = msm_fence_alloc(); + if (IS_ERR(submit->hw_fence)) { + ret = PTR_ERR(submit->hw_fence); + kfree(submit); + return ERR_PTR(ret); + } + ret = drm_sched_job_init(&submit->base, queue->entity, queue); if (ret) { kfree(submit); diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c index 57a8e9564540..a62b45e5a8c3 100644 --- a/drivers/gpu/drm/msm/msm_ringbuffer.c +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c @@ -18,7 +18,7 @@ static struct dma_fence *msm_job_run(struct drm_sched_job *job) struct msm_gpu *gpu = submit->gpu; int i; - submit->hw_fence = msm_fence_alloc(fctx); + msm_fence_init(submit->hw_fence, fctx); for (i = 0; i < submit->nr_bos; i++) { struct drm_gem_object *obj = &submit->bos[i].obj->base;