On Tue, 2026-06-30 at 12:04 -0400, Shahyan Soltani wrote:
The num_fences, count, i, and j variables in dma_fence_dedup_array() and __dma_fence_unwrap_merge() have inconsistent integer types, mixing both unsigned int and int.
Use type size_t consistently for these instead, and update the return type of dma_fence_dedup_array() accordingly.
Signed-off-by: Shahyan Soltani shahyan.soltani@amd.com Suggested-by: Philipp Stanner phasta@mailbox.org
Thx for fixing this, cool work
Reviewed-by: Philipp Stanner phasta@kernel.org
The rest of the subsystems (dma_resv_reserve_fences, drm_exec, drm_gpuvm, xe, nouveau, etc) uses "unsigned int" for num_fences, for example the amdgpu caller in amdgpu_userq_fence.c.
You mention that because you can't / won't change them?
My suggestion actually has been to go for `unsigned int`. Christian opinioned that it should be size_t. Shouldn't be a big deal, though, my issue was just the possibility for negative numbers.
Christian, would it be a bit better to be consistent with the parties Shayan mentions?
P.
drivers/dma-buf/dma-fence-unwrap.c | 8 ++++---- include/linux/dma-fence-unwrap.h | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index 53bb40e70b27..65e87d263c3a 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -93,9 +93,9 @@ static int fence_cmp(const void *_a, const void *_b) * * Return: Number of unique fences remaining in the array. */ -int dma_fence_dedup_array(struct dma_fence **fences, int num_fences) +size_t dma_fence_dedup_array(struct dma_fence **fences, size_t num_fences) {
- int i, j;
- size_t i, j;
sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL); @@ -115,14 +115,14 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences) EXPORT_SYMBOL_GPL(dma_fence_dedup_array); /* Implementation for the dma_fence_merge() marco, don't use directly */ -struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, +struct dma_fence *__dma_fence_unwrap_merge(size_t num_fences, struct dma_fence **fences, struct dma_fence_unwrap *iter) { struct dma_fence *tmp, *unsignaled = NULL, **array; struct dma_fence_array *result; ktime_t timestamp;
- int i, count;
- size_t i, count;
count = 0; timestamp = ns_to_ktime(0); diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h index 62df222fe0f1..7bfacdf79de2 100644 --- a/include/linux/dma-fence-unwrap.h +++ b/include/linux/dma-fence-unwrap.h @@ -8,6 +8,8 @@ #ifndef __LINUX_DMA_FENCE_UNWRAP_H #define __LINUX_DMA_FENCE_UNWRAP_H +#include <linux/types.h>
struct dma_fence; /** @@ -48,11 +50,11 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor); for (fence = dma_fence_unwrap_first(head, cursor); fence; \ fence = dma_fence_unwrap_next(cursor)) -struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, +struct dma_fence *__dma_fence_unwrap_merge(size_t num_fences, struct dma_fence **fences, struct dma_fence_unwrap *cursors); -int dma_fence_dedup_array(struct dma_fence **array, int num_fences); +size_t dma_fence_dedup_array(struct dma_fence **array, size_t num_fences); /** * dma_fence_unwrap_merge - unwrap and merge fences
On 7/1/26 08:52, Philipp Stanner wrote:
On Tue, 2026-06-30 at 12:04 -0400, Shahyan Soltani wrote:
The num_fences, count, i, and j variables in dma_fence_dedup_array() and __dma_fence_unwrap_merge() have inconsistent integer types, mixing both unsigned int and int.
Use type size_t consistently for these instead, and update the return type of dma_fence_dedup_array() accordingly.
Signed-off-by: Shahyan Soltani shahyan.soltani@amd.com Suggested-by: Philipp Stanner phasta@mailbox.org
Thx for fixing this, cool work
Reviewed-by: Philipp Stanner phasta@kernel.org
The rest of the subsystems (dma_resv_reserve_fences, drm_exec, drm_gpuvm, xe, nouveau, etc) uses "unsigned int" for num_fences, for example the amdgpu caller in amdgpu_userq_fence.c.
Good point.
You mention that because you can't / won't change them?
My suggestion actually has been to go for `unsigned int`. Christian opinioned that it should be size_t. Shouldn't be a big deal, though, my issue was just the possibility for negative numbers.
Yeah, unsigned int would probably work as well. But ARRAY_SIZE() returns size_t if I'm not completely mistaken and I have seen size_t being used elsewhere as well.
Christian, would it be a bit better to be consistent with the parties Shayan mentions?
For the amdgpu use cases I don't care much because we limit num_fences to 16bit anyway and don't allow larger allocations.
The drm_exec and drm_gpuvm should definitely be fixed as well. Not sure about XE or Nouveau,
It should just not be a signed type anywhere, cause that can indeed mean trouble.
Regards, Christian.
P.
drivers/dma-buf/dma-fence-unwrap.c | 8 ++++---- include/linux/dma-fence-unwrap.h | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index 53bb40e70b27..65e87d263c3a 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -93,9 +93,9 @@ static int fence_cmp(const void *_a, const void *_b) * * Return: Number of unique fences remaining in the array. */ -int dma_fence_dedup_array(struct dma_fence **fences, int num_fences) +size_t dma_fence_dedup_array(struct dma_fence **fences, size_t num_fences) {
- int i, j;
- size_t i, j;
sort(fences, num_fences, sizeof(*fences), fence_cmp, NULL); @@ -115,14 +115,14 @@ int dma_fence_dedup_array(struct dma_fence **fences, int num_fences) EXPORT_SYMBOL_GPL(dma_fence_dedup_array); /* Implementation for the dma_fence_merge() marco, don't use directly */ -struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, +struct dma_fence *__dma_fence_unwrap_merge(size_t num_fences, struct dma_fence **fences, struct dma_fence_unwrap *iter) { struct dma_fence *tmp, *unsignaled = NULL, **array; struct dma_fence_array *result; ktime_t timestamp;
- int i, count;
- size_t i, count;
count = 0; timestamp = ns_to_ktime(0); diff --git a/include/linux/dma-fence-unwrap.h b/include/linux/dma-fence-unwrap.h index 62df222fe0f1..7bfacdf79de2 100644 --- a/include/linux/dma-fence-unwrap.h +++ b/include/linux/dma-fence-unwrap.h @@ -8,6 +8,8 @@ #ifndef __LINUX_DMA_FENCE_UNWRAP_H #define __LINUX_DMA_FENCE_UNWRAP_H +#include <linux/types.h>
struct dma_fence; /** @@ -48,11 +50,11 @@ struct dma_fence *dma_fence_unwrap_next(struct dma_fence_unwrap *cursor); for (fence = dma_fence_unwrap_first(head, cursor); fence; \ fence = dma_fence_unwrap_next(cursor)) -struct dma_fence *__dma_fence_unwrap_merge(unsigned int num_fences, +struct dma_fence *__dma_fence_unwrap_merge(size_t num_fences, struct dma_fence **fences, struct dma_fence_unwrap *cursors); -int dma_fence_dedup_array(struct dma_fence **array, int num_fences); +size_t dma_fence_dedup_array(struct dma_fence **array, size_t num_fences); /** * dma_fence_unwrap_merge - unwrap and merge fences
linaro-mm-sig@lists.linaro.org
-
Christian König -
Philipp Stanner