On 08/02/2024 18:13, Erick Archer wrote:
The "struct i915_syncmap" uses a dynamically sized set of trailing elements. It can use an "u32" array or a "struct i915_syncmap *" array. So, use the preferred way in the kernel declaring flexible arrays [1]. Because there are two possibilities for the trailing arrays, it is necessary to declare a union and use the DECLARE_FLEX_ARRAY macro. The comment can be removed as the union is now clear enough. Also, avoid the open-coded arithmetic in the memory allocator functions [2] using the "struct_size" macro. Moreover, refactor the "__sync_seqno" and "__sync_child" functions due to now it is possible to use the union members added to the structure. This way, it is also possible to avoid the open-coded arithmetic in pointers. Link: https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays [1] Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [2] Signed-off-by: Erick Archer <erick.archer@xxxxxxx>
Looks good to me too so I've pushed it to drm-intel-gt-next, thanks! Regards, Tvrtko
--- drivers/gpu/drm/i915/i915_syncmap.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_syncmap.c b/drivers/gpu/drm/i915/i915_syncmap.c index 60404dbb2e9f..df6437c37373 100644 --- a/drivers/gpu/drm/i915/i915_syncmap.c +++ b/drivers/gpu/drm/i915/i915_syncmap.c @@ -75,13 +75,10 @@ struct i915_syncmap { unsigned int height; unsigned int bitmap; struct i915_syncmap *parent; - /* - * Following this header is an array of either seqno or child pointers: - * union { - * u32 seqno[KSYNCMAP]; - * struct i915_syncmap *child[KSYNCMAP]; - * }; - */ + union { + DECLARE_FLEX_ARRAY(u32, seqno); + DECLARE_FLEX_ARRAY(struct i915_syncmap *, child); + }; }; /** @@ -99,13 +96,13 @@ void i915_syncmap_init(struct i915_syncmap **root) static inline u32 *__sync_seqno(struct i915_syncmap *p) { GEM_BUG_ON(p->height); - return (u32 *)(p + 1); + return p->seqno; } static inline struct i915_syncmap **__sync_child(struct i915_syncmap *p) { GEM_BUG_ON(!p->height); - return (struct i915_syncmap **)(p + 1); + return p->child; } static inline unsigned int @@ -200,7 +197,7 @@ __sync_alloc_leaf(struct i915_syncmap *parent, u64 id) { struct i915_syncmap *p; - p = kmalloc(sizeof(*p) + KSYNCMAP * sizeof(u32), GFP_KERNEL); + p = kmalloc(struct_size(p, seqno, KSYNCMAP), GFP_KERNEL); if (unlikely(!p)) return NULL; @@ -282,7 +279,7 @@ static noinline int __sync_set(struct i915_syncmap **root, u64 id, u32 seqno) unsigned int above; /* Insert a join above the current layer */ - next = kzalloc(sizeof(*next) + KSYNCMAP * sizeof(next), + next = kzalloc(struct_size(next, child, KSYNCMAP), GFP_KERNEL); if (unlikely(!next)) return -ENOMEM; -- 2.25.1