| Message ID | 20220329230105.601666-4-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Userspace allocated GPU addresses | expand |
On Wed, 30 Mar 2022 at 02:00, Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > Prep for a following patch. While we are at it, convert a few remaining > WARN_ON()s to GEM_WARN_ON(). Well... GEM_WARN_ON doesn't really look like a 'while we are at it'. It might be better to split it into a separate commit. > > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/msm_gem.c | 2 +- > drivers/gpu/drm/msm/msm_gem.h | 1 + > drivers/gpu/drm/msm/msm_gem_vma.c | 15 ++++++++++----- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index a4f61972667b..f96d1dc72021 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -938,7 +938,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m, > name, comm ? ":" : "", comm ? comm : "", > vma->aspace, vma->iova, > vma->mapped ? "mapped" : "unmapped", > - vma->inuse); > + msm_gem_vma_inuse(vma)); > kfree(comm); > } > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > index 947ff7d9b471..1b7f0f0b88bf 100644 > --- a/drivers/gpu/drm/msm/msm_gem.h > +++ b/drivers/gpu/drm/msm/msm_gem.h > @@ -61,6 +61,7 @@ struct msm_gem_vma { > int msm_gem_init_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma, int npages, > u64 range_start, u64 range_end); > +bool msm_gem_vma_inuse(struct msm_gem_vma *vma); > void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma); > void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c > index f914ddbaea89..dc2ae097805e 100644 > --- a/drivers/gpu/drm/msm/msm_gem_vma.c > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c > @@ -37,6 +37,11 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace) > return aspace; > } > > +bool msm_gem_vma_inuse(struct msm_gem_vma *vma) > +{ > + return !!vma->inuse; > +} It almost asks to be a static inline. The patch 04 makes it complex, so it's probably not that important. > + > /* Actually unmap memory for the vma */ > void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma) > @@ -44,7 +49,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > unsigned size = vma->node.size << PAGE_SHIFT; > > /* Print a message if we try to purge a vma in use */ > - if (WARN_ON(vma->inuse > 0)) > + if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) > return; > > /* Don't do anything if the memory isn't mapped */ > @@ -61,7 +66,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma) > { > - if (!WARN_ON(!vma->iova)) > + if (!GEM_WARN_ON(!vma->iova)) > vma->inuse--; > } > > @@ -73,7 +78,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, > unsigned size = npages << PAGE_SHIFT; > int ret = 0; > > - if (WARN_ON(!vma->iova)) > + if (GEM_WARN_ON(!vma->iova)) > return -EINVAL; > > /* Increase the usage counter */ > @@ -100,7 +105,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, > void msm_gem_close_vma(struct msm_gem_address_space *aspace, > struct msm_gem_vma *vma) > { > - if (WARN_ON(vma->inuse > 0 || vma->mapped)) > + if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped)) > return; > > spin_lock(&aspace->lock); > @@ -120,7 +125,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, > { > int ret; > > - if (WARN_ON(vma->iova)) > + if (GEM_WARN_ON(vma->iova)) > return -EBUSY; > > spin_lock(&aspace->lock); > -- > 2.35.1 >
On Wed, Mar 30, 2022 at 4:32 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Wed, 30 Mar 2022 at 02:00, Rob Clark <robdclark@gmail.com> wrote: > > > > From: Rob Clark <robdclark@chromium.org> > > > > Prep for a following patch. While we are at it, convert a few remaining > > WARN_ON()s to GEM_WARN_ON(). > > Well... GEM_WARN_ON doesn't really look like a 'while we are at it'. > It might be better to split it into a separate commit. it was a bit lazy.. I'll split it out > > > > Signed-off-by: Rob Clark <robdclark@chromium.org> > > --- > > drivers/gpu/drm/msm/msm_gem.c | 2 +- > > drivers/gpu/drm/msm/msm_gem.h | 1 + > > drivers/gpu/drm/msm/msm_gem_vma.c | 15 ++++++++++----- > > 3 files changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > > index a4f61972667b..f96d1dc72021 100644 > > --- a/drivers/gpu/drm/msm/msm_gem.c > > +++ b/drivers/gpu/drm/msm/msm_gem.c > > @@ -938,7 +938,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m, > > name, comm ? ":" : "", comm ? comm : "", > > vma->aspace, vma->iova, > > vma->mapped ? "mapped" : "unmapped", > > - vma->inuse); > > + msm_gem_vma_inuse(vma)); > > kfree(comm); > > } > > > > diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h > > index 947ff7d9b471..1b7f0f0b88bf 100644 > > --- a/drivers/gpu/drm/msm/msm_gem.h > > +++ b/drivers/gpu/drm/msm/msm_gem.h > > @@ -61,6 +61,7 @@ struct msm_gem_vma { > > int msm_gem_init_vma(struct msm_gem_address_space *aspace, > > struct msm_gem_vma *vma, int npages, > > u64 range_start, u64 range_end); > > +bool msm_gem_vma_inuse(struct msm_gem_vma *vma); > > void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > > struct msm_gem_vma *vma); > > void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, > > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c > > index f914ddbaea89..dc2ae097805e 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_vma.c > > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c > > @@ -37,6 +37,11 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace) > > return aspace; > > } > > > > +bool msm_gem_vma_inuse(struct msm_gem_vma *vma) > > +{ > > + return !!vma->inuse; > > +} > > It almost asks to be a static inline. The patch 04 makes it complex, > so it's probably not that important. yeah, that is the reason I didn't make it static inline BR, -R > > + > > /* Actually unmap memory for the vma */ > > void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > > struct msm_gem_vma *vma) > > @@ -44,7 +49,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > > unsigned size = vma->node.size << PAGE_SHIFT; > > > > /* Print a message if we try to purge a vma in use */ > > - if (WARN_ON(vma->inuse > 0)) > > + if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) > > return; > > > > /* Don't do anything if the memory isn't mapped */ > > @@ -61,7 +66,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, > > void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, > > struct msm_gem_vma *vma) > > { > > - if (!WARN_ON(!vma->iova)) > > + if (!GEM_WARN_ON(!vma->iova)) > > vma->inuse--; > > } > > > > @@ -73,7 +78,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, > > unsigned size = npages << PAGE_SHIFT; > > int ret = 0; > > > > - if (WARN_ON(!vma->iova)) > > + if (GEM_WARN_ON(!vma->iova)) > > return -EINVAL; > > > > /* Increase the usage counter */ > > @@ -100,7 +105,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, > > void msm_gem_close_vma(struct msm_gem_address_space *aspace, > > struct msm_gem_vma *vma) > > { > > - if (WARN_ON(vma->inuse > 0 || vma->mapped)) > > + if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped)) > > return; > > > > spin_lock(&aspace->lock); > > @@ -120,7 +125,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, > > { > > int ret; > > > > - if (WARN_ON(vma->iova)) > > + if (GEM_WARN_ON(vma->iova)) > > return -EBUSY; > > > > spin_lock(&aspace->lock); > > -- > > 2.35.1 > > > > > -- > With best wishes > Dmitry
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index a4f61972667b..f96d1dc72021 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -938,7 +938,7 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m, name, comm ? ":" : "", comm ? comm : "", vma->aspace, vma->iova, vma->mapped ? "mapped" : "unmapped", - vma->inuse); + msm_gem_vma_inuse(vma)); kfree(comm); } diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 947ff7d9b471..1b7f0f0b88bf 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -61,6 +61,7 @@ struct msm_gem_vma { int msm_gem_init_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma, int npages, u64 range_start, u64 range_end); +bool msm_gem_vma_inuse(struct msm_gem_vma *vma); void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma); void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index f914ddbaea89..dc2ae097805e 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -37,6 +37,11 @@ msm_gem_address_space_get(struct msm_gem_address_space *aspace) return aspace; } +bool msm_gem_vma_inuse(struct msm_gem_vma *vma) +{ + return !!vma->inuse; +} + /* Actually unmap memory for the vma */ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) @@ -44,7 +49,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, unsigned size = vma->node.size << PAGE_SHIFT; /* Print a message if we try to purge a vma in use */ - if (WARN_ON(vma->inuse > 0)) + if (GEM_WARN_ON(msm_gem_vma_inuse(vma))) return; /* Don't do anything if the memory isn't mapped */ @@ -61,7 +66,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace, void msm_gem_unmap_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (!WARN_ON(!vma->iova)) + if (!GEM_WARN_ON(!vma->iova)) vma->inuse--; } @@ -73,7 +78,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, unsigned size = npages << PAGE_SHIFT; int ret = 0; - if (WARN_ON(!vma->iova)) + if (GEM_WARN_ON(!vma->iova)) return -EINVAL; /* Increase the usage counter */ @@ -100,7 +105,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace, void msm_gem_close_vma(struct msm_gem_address_space *aspace, struct msm_gem_vma *vma) { - if (WARN_ON(vma->inuse > 0 || vma->mapped)) + if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped)) return; spin_lock(&aspace->lock); @@ -120,7 +125,7 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace, { int ret; - if (WARN_ON(vma->iova)) + if (GEM_WARN_ON(vma->iova)) return -EBUSY; spin_lock(&aspace->lock);