Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3024 discussions

Re: [PATCH v7 1/4] drm/panthor: Introduce BO labeling

by Steven Price

On 14/04/2025 20:43, Adrián Larumbe wrote: > Hi Steven, > > On 14.04.2025 10:50, Steven Price wrote: >> Hi Adrián, >> >> On 11/04/2025 16:03, Adrián Larumbe wrote: >>> Add a new character string Panthor BO field, and a function that allows >>> setting it from within the driver. >>> >>> Driver takes care of freeing the string when it's replaced or no longer >>> needed at object destruction time, but allocating it is the responsibility >>> of callers. >>> >>> Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> >>> Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> >>> --- >>> drivers/gpu/drm/panthor/panthor_gem.c | 39 +++++++++++++++++++++++++++ >>> drivers/gpu/drm/panthor/panthor_gem.h | 17 ++++++++++++ >>> 2 files changed, 56 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c >>> index 8244a4e6c2a2..af0ac17f357f 100644 >>> --- a/drivers/gpu/drm/panthor/panthor_gem.c >>> +++ b/drivers/gpu/drm/panthor/panthor_gem.c >>> @@ -2,6 +2,7 @@ >>> /* Copyright 2019 Linaro, Ltd, Rob Herring <robh(a)kernel.org> */ >>> /* Copyright 2023 Collabora ltd. */ >>> >>> +#include <linux/cleanup.h> >>> #include <linux/dma-buf.h> >>> #include <linux/dma-mapping.h> >>> #include <linux/err.h> >>> @@ -18,6 +19,14 @@ static void panthor_gem_free_object(struct drm_gem_object *obj) >>> struct panthor_gem_object *bo = to_panthor_bo(obj); >>> struct drm_gem_object *vm_root_gem = bo->exclusive_vm_root_gem; >>> >>> + /* >>> + * Label might have been allocated with kstrdup_const(), >>> + * we need to take that into account when freeing the memory >>> + */ >>> + kfree_const(bo->label.str); >>> + >>> + mutex_destroy(&bo->label.lock); >>> + >>> drm_gem_free_mmap_offset(&bo->base.base); >>> mutex_destroy(&bo->gpuva_list_lock); >>> drm_gem_shmem_free(&bo->base); >>> @@ -196,6 +205,7 @@ struct drm_gem_object *panthor_gem_create_object(struct drm_device *ddev, size_t >>> obj->base.map_wc = !ptdev->coherent; >>> mutex_init(&obj->gpuva_list_lock); >>> drm_gem_gpuva_set_lock(&obj->base.base, &obj->gpuva_list_lock); >>> + mutex_init(&obj->label.lock); >>> >>> return &obj->base.base; >>> } >>> @@ -247,3 +257,32 @@ panthor_gem_create_with_handle(struct drm_file *file, >>> >>> return ret; >>> } >>> + >>> +void >>> +panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label) >>> +{ >>> + struct panthor_gem_object *bo = to_panthor_bo(obj); >>> + const char *old_label; >>> + >>> + scoped_guard(mutex, &bo->label.lock) { >>> + old_label = bo->label.str; >>> + bo->label.str = label; >>> + } >>> + >>> + kfree(old_label); >> >> Shouldn't this be kfree_const()? I suspect as things stand we can't >> trigger this bug but it's inconsistent. > > This could only be called either from the set_label() ioctl, in which case > old_label could be NULL or a pointer to a string obtained from strdup(); or from > panthor_gem_kernel_bo_set_label(). In the latter case, it could only ever be > NULL, since we don't reassign kernel BO labels, so it'd be safe too. Yeah I thought it probably doesn't cause problems now, but it's a foot gun for the future. > However I do agree that it's not consistent, and then in the future perhaps > relabelling kernel BO's might be justified, so I'll change it to kfree_const(). Thanks! >>> +} >>> + >>> +void >>> +panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label) >>> +{ >>> + const char *str; >>> + >>> + str = kstrdup_const(label, GFP_KERNEL); >>> + if (!str) { >> >> In the next patch you permit user space to clear the label >> (args->size==0) which means label==NULL and AFAICT kstrdup_const() will >> return NULL in this case triggering this warning. > > Kernel and UM-exposed BO's should never experience cross-labelling, so in theory > this scenario ought to be impossible. The only way panthor_gem_kernel_bo_set_label() > might take NULL in the 'label' argument is that someone called panthor_kernel_bo_create() > with NULL for its name 'argument'. You're absolutely correct - I somehow got confused between the kernel and user paths. It's the user path above which needs to handle NULL (and does). > I think as a defensive check, I could do something as follows > > void > panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label) > { > const char *str; > > /* We should never attempt labelling a UM-exposed GEM object */ > if (drm_WARN_ON(bo->obj->dev, &bo->obj->handle_count > 0)) > return; > > if (!label) > return; > > [...] > } I'm happy for you to do nothing here - that was my mistake getting the two functions muddled. Sorry for the noise. I'm equally happy for the defensive checks above. Steve >> Thanks, >> Steve >> >>> + /* Failing to allocate memory for a label isn't a fatal condition */ >>> + drm_warn(bo->obj->dev, "Not enough memory to allocate BO label"); >>> + return; >>> + } >>> + >>> + panthor_gem_bo_set_label(bo->obj, str); >>> +} >>> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h >>> index 1a363bb814f4..af0d77338860 100644 >>> --- a/drivers/gpu/drm/panthor/panthor_gem.h >>> +++ b/drivers/gpu/drm/panthor/panthor_gem.h >>> @@ -46,6 +46,20 @@ struct panthor_gem_object { >>> >>> /** @flags: Combination of drm_panthor_bo_flags flags. */ >>> u32 flags; >>> + >>> + /** >>> + * @label: BO tagging fields. The label can be assigned within the >>> + * driver itself or through a specific IOCTL. >>> + */ >>> + struct { >>> + /** >>> + * @label.str: Pointer to NULL-terminated string, >>> + */ >>> + const char *str; >>> + >>> + /** @lock.str: Protects access to the @label.str field. */ >>> + struct mutex lock; >>> + } label; >>> }; >>> >>> /** >>> @@ -91,6 +105,9 @@ panthor_gem_create_with_handle(struct drm_file *file, >>> struct panthor_vm *exclusive_vm, >>> u64 *size, u32 flags, uint32_t *handle); >>> >>> +void panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label); >>> +void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label); >>> + >>> static inline u64 >>> panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo) >>> { > > Adrian Larumbe

4 months, 4 weeks

[PATCH v3] drm/gem: Internally test import_attach for imported objects

by Thomas Zimmermann

Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 9b71f7a9f3f8..a3133a08267c 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -588,8 +588,7 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP -- 2.49.0

4 months, 4 weeks

Re: [PATCH] dma-buf: heaps: Set allocation orders for larger page sizes

by T.J. Mercier

On Tue, Apr 15, 2025 at 10:20 AM Juan Yescas <jyescas(a)google.com> wrote: > > This change sets the allocation orders for the different page sizes > (4k, 16k, 64k) based on PAGE_SHIFT. Before this change, the orders > for large page sizes were calculated incorrectly, this caused system > heap to allocate from 2% to 4% more memory on 16KiB page size kernels. > > This change was tested on 4k/16k page size kernels. > > Signed-off-by: Juan Yescas <jyescas(a)google.com> I think "dma-buf: system_heap:" would be better for the subject since this is specific to the system heap. Would you mind cleaning up the extra space on line 321 too? @@ -318,7 +318,7 @@ static struct page *alloc_largest_available(unsigned long size, int i; for (i = 0; i < NUM_ORDERS; i++) { - if (size < (PAGE_SIZE << orders[i])) + if (size < (PAGE_SIZE << orders[i])) With that, Reviewed-by: T.J. Mercier <tjmercier(a)google.com> Fixes: d963ab0f15fb ("dma-buf: system_heap: Allocate higher order pages if available") is also probably a good idea. > --- > drivers/dma-buf/heaps/system_heap.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c > index 26d5dc89ea16..54674c02dcb4 100644 > --- a/drivers/dma-buf/heaps/system_heap.c > +++ b/drivers/dma-buf/heaps/system_heap.c > @@ -50,8 +50,15 @@ static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP}; > * to match with the sizes often found in IOMMUs. Using order 4 pages instead > * of order 0 pages can significantly improve the performance of many IOMMUs > * by reducing TLB pressure and time spent updating page tables. > + * > + * Note: When the order is 0, the minimum allocation is PAGE_SIZE. The possible > + * page sizes for ARM devices could be 4K, 16K and 64K. > */ > -static const unsigned int orders[] = {8, 4, 0}; > +#define ORDER_1M (20 - PAGE_SHIFT) > +#define ORDER_64K (16 - PAGE_SHIFT) > +#define ORDER_FOR_PAGE_SIZE (0) > +static const unsigned int orders[] = {ORDER_1M, ORDER_64K, ORDER_FOR_PAGE_SIZE}; > + > #define NUM_ORDERS ARRAY_SIZE(orders) > > static struct sg_table *dup_sg_table(struct sg_table *table) > -- > 2.49.0.604.gff1f9ca942-goog >

4 months, 4 weeks

Re: [PATCH] dma-buf: heaps: Set allocation orders for larger page sizes

by John Stultz

4 months, 4 weeks

Re: [PATCH v2] drm/gem: Internally test import_attach for imported objects

by Thomas Zimmermann

Hi Am 15.04.25 um 16:19 schrieb Boris Brezillon: > On Tue, 15 Apr 2025 16:02:20 +0200 > Thomas Zimmermann <tzimmermann(a)suse.de> wrote: > >> Test struct drm_gem_object.import_attach.dmabuf to detect imported >> objects. Warn if the stored state is inconsistent. >> >> During object clenaup, the dma_buf field might be NULL. Testing it in >> an object's free callback then incorrectly does a cleanup as for native >> objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that >> clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). >> >> v2: >> - use import_attach.dmabuf instead of dma_buf (Christian) >> >> Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> >> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") >> Reported-by: Andy Yan <andyshrk(a)163.com> >> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… >> Tested-by: Andy Yan <andyshrk(a)163.com> >> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> >> Cc: Anusha Srivatsa <asrivats(a)redhat.com> >> Cc: Christian König <christian.koenig(a)amd.com> >> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> >> Cc: Maxime Ripard <mripard(a)kernel.org> >> Cc: David Airlie <airlied(a)gmail.com> >> Cc: Simona Vetter <simona(a)ffwll.ch> >> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> >> Cc: "Christian König" <christian.koenig(a)amd.com> >> Cc: dri-devel(a)lists.freedesktop.org >> Cc: linux-media(a)vger.kernel.org >> Cc: linaro-mm-sig(a)lists.linaro.org >> --- >> include/drm/drm_gem.h | 26 ++++++++++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h >> index 9b71f7a9f3f8..464b9c7feec0 100644 >> --- a/include/drm/drm_gem.h >> +++ b/include/drm/drm_gem.h >> @@ -579,6 +579,21 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje >> return (obj->handle_count > 1) || obj->dma_buf; >> } >> >> +/** >> + * drm_gem_owns_dma_buf() - Tests if GEM object backs a DMA-buffer object >> + * @obj: the GEM object >> + * @obj: the DMA buffer >> + * >> + * Returns: >> + * True if the DMA buffer refers to the GEM object's buffer. >> + */ >> +static inline bool drm_gem_owns_dma_buf(const struct drm_gem_object *obj, >> + const struct dma_buf *dma_buf) >> +{ >> + /* The dma-buf's priv field points to the original GEM object. */ >> + return dma_buf->priv == obj; >> +} >> + >> /** >> * drm_gem_is_imported() - Tests if GEM object's buffer has been imported >> * @obj: the GEM object >> @@ -588,8 +603,15 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje >> */ >> static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) >> { >> - /* The dma-buf's priv field points to the original GEM object. */ >> - return obj->dma_buf && (obj->dma_buf->priv != obj); >> + const struct dma_buf *dma_buf = NULL; >> + >> + if (!obj->import_attach) >> + return false; >> + >> + dma_buf = obj->import_attach->dmabuf; >> + >> + /* Warn if we somehow reimported our own buffer. */ >> + return !drm_WARN_ON_ONCE(obj->dev, !dma_buf || drm_gem_owns_dma_buf(obj, dma_buf)); > I'm honestly not sure I see the point of checking > obj->import_attach->dmabuf. If obj->import_attach != NULL, we're sure > it's a foreign buffer already, otherwise we would get the original GEM > object which has ->import_attach=NULL. So why not go for a simple > > return obj->import_attach != NULL; > > check, and extend the check when you get to implement imports without > import attachments (not sure what those would look like BTW). I have no strong opinion. I just found it confusing to rely on import_attach when the dma_buf is what we originally imported. Best regards Thomas -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)

4 months, 4 weeks

[PATCH] drm/gem: Internally test import_attach for imported objects

by Thomas Zimmermann

Test struct drm_gem_object.import_attach to detect imported objects during cleanup. At that point, the imported DMA buffer might have already been released and the dma_buf field is NULL. The object's free callback then does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually clear the dma_buf field in drm_gem_object_exported_dma_buf_free(). Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- include/drm/drm_gem.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 9b71f7a9f3f8..f09b8afcf86d 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -589,7 +589,13 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return (obj->dma_buf && (obj->dma_buf->priv != obj)) || + /* + * TODO: During object release, the dma-buf might already + * be gone. For now keep testing import_attach, but + * this should be removed at some point. + */ + obj->import_attach; } #ifdef CONFIG_LOCKDEP -- 2.49.0

4 months, 4 weeks

[PATCH v2] drm/gem: Internally test import_attach for imported objects

by Thomas Zimmermann

Test struct drm_gem_object.import_attach.dmabuf to detect imported objects. Warn if the stored state is inconsistent. During object clenaup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- include/drm/drm_gem.h | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 9b71f7a9f3f8..464b9c7feec0 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -579,6 +579,21 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje return (obj->handle_count > 1) || obj->dma_buf; } +/** + * drm_gem_owns_dma_buf() - Tests if GEM object backs a DMA-buffer object + * @obj: the GEM object + * @obj: the DMA buffer + * + * Returns: + * True if the DMA buffer refers to the GEM object's buffer. + */ +static inline bool drm_gem_owns_dma_buf(const struct drm_gem_object *obj, + const struct dma_buf *dma_buf) +{ + /* The dma-buf's priv field points to the original GEM object. */ + return dma_buf->priv == obj; +} + /** * drm_gem_is_imported() - Tests if GEM object's buffer has been imported * @obj: the GEM object @@ -588,8 +603,15 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + const struct dma_buf *dma_buf = NULL; + + if (!obj->import_attach) + return false; + + dma_buf = obj->import_attach->dmabuf; + + /* Warn if we somehow reimported our own buffer. */ + return !drm_WARN_ON_ONCE(obj->dev, !dma_buf || drm_gem_owns_dma_buf(obj, dma_buf)); } #ifdef CONFIG_LOCKDEP -- 2.49.0

4 months, 4 weeks

Re: [PATCH] drm/gem: Internally test import_attach for imported objects

by Christian König

Am 15.04.25 um 15:10 schrieb Simona Vetter: >> This is for devices who only want to do a vmap of the buffer, isn't it? > ... it's for the vmap only case, where you might not even have a struct > device. Or definitely not a reasonable one, like maybe a faux_bus device > or some device on a bus that really doesn't do dma (e.g. spi or i2c), and > where hence dma_buf_map_attachment is just something you never ever want > to do. Even in that case I would still suggest to at least create an attachment to let the exporter know that somebody is doing something with it's buffer. That is also important for move notification since you can't do those without an attachment. BTW: What is keeping a vmap alive after dropping the reservation lock? There is no pinning whatsoever as far as I can see. > I think we might want to transform obj->import_attach into a union or > tagged pointer or something like that, which can cover both cases. And > maybe a drm_gem_bo_imported_dma_buf() helper that gives you the dma_buf no > matter what if it's imported, or NULL if it's allocated on that > drm_device? Yeah, I had the same idea before as well. Just didn't know if that was something worth looking into. Regards, Christian. > > Cheers, Sima

4 months, 4 weeks

Re: [PATCH v2 2/2] drm/nouveau: nouveau_fence: Standardize list iterations

by Christian König

Am 15.04.25 um 14:19 schrieb Philipp Stanner: > nouveau_fence.c iterates over lists in a non-canonical way. Since the > operations done are just basic for-each-loops, they should be written in > the standard form. > > Use for_each_safe() instead of the custom loop iterations. > > Signed-off-by: Philipp Stanner <phasta(a)kernel.org> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/gpu/drm/nouveau/nouveau_fence.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c > index 6ded8c2b6d3b..60d961b43488 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c > @@ -84,11 +84,12 @@ void > nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error) > { > struct nouveau_fence *fence; > + struct list_head *pos, *tmp; > unsigned long flags; > > spin_lock_irqsave(&fctx->lock, flags); > - while (!list_empty(&fctx->pending)) { > - fence = list_entry(fctx->pending.next, typeof(*fence), head); > + list_for_each_safe(pos, tmp, &fctx->pending) { > + fence = list_entry(pos, struct nouveau_fence, head); > > if (error && !dma_fence_is_signaled_locked(&fence->base)) > dma_fence_set_error(&fence->base, error); > @@ -131,11 +132,12 @@ static int > nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx) > { > struct nouveau_fence *fence; > + struct list_head *pos, *tmp; > int drop = 0; > u32 seq = fctx->read(chan); > > - while (!list_empty(&fctx->pending)) { > - fence = list_entry(fctx->pending.next, typeof(*fence), head); > + list_for_each_safe(pos, tmp, &fctx->pending) { > + fence = list_entry(pos, struct nouveau_fence, head); > > if ((int)(seq - fence->base.seqno) < 0) > break;

4 months, 4 weeks

Re: [PATCH v7 2/4] drm/panthor: Add driver IOCTL for setting BO labels

by Daniel Stone

Hi, On Fri, 11 Apr 2025 at 16:05, Adrián Larumbe <adrian.larumbe(a)collabora.com> wrote: > +#define PANTHOR_BO_LABEL_MAXLEN PAGE_SIZE PAGE_SIZE can change between kernel builds with a config setting. If the thinking here is '4KiB is big enough' (which I agree with), then just define it to 4096. Cheers, Daniel

4 months, 4 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig