drm/amdkfd: Remove vla
Subsystems (3)
PATCH 1/1 drm/amdkfd: Remove vla
There's an ongoing effort to remove VLAs[1] from the kernel to eventually turn on -Wvla. The single VLA usage in the amdkfd driver is actually constant across all current platforms. Switch to a constant size array instead.
[1] https://lkml.org/lkml/2018/3/7/621
--- drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +5 -3
@@ -139,10 +139,12 @@ static void interrupt_wq(struct work_struct *work) { struct kfd_dev *dev = container_of(work, struct kfd_dev, interrupt_work); +uint32_t ih_ring_entry[4 ]; -uint32_t ih_ring_entry[DIV_ROUND_UP ( - dev->device_info->ih_ring_entry_size, -sizeof (uint32_t ))]; +if (dev->device_info->ih_ring_entry_size > (4 *sizeof (uint32_t ))) { +dev_err (kfd_chardev (),"Ring entry too small\n" ); +return ; + }while (dequeue_ih_ring_entry (dev, ih_ring_entry)) dev->device_info->event_interrupt_class->interrupt_wq(dev,
Signature · 6 lines
2.14.3 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Christian König
maintainerAm 09.04.2018 um 23:06 schrieb Laura Abbott:
Quoted 3 lines
There's an ongoing effort to remove VLAs[1] from the kernel to eventually turn on -Wvla. The single VLA usage in the amdkfd driver is actually constant across all current platforms.
Actually that isn't correct.
Could be that we haven't upstreamed KFD support for them, but Vega10 have a different interrupt ring entry size and so would cause the error message here.
Quoted 2 lines
Switch to a constant size array
instead.
I would say to just make make the array bigger.
Regards,
Christian.
Quoted 28 lines
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Laura Abbott <[email protected]>--- drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +5
-3@@ -139,10 +139,12 @@ static void interrupt_wq(struct work_struct *work) { struct kfd_dev *dev = container_of(work, struct kfd_dev, interrupt_work); +uint32_t ih_ring_entry[4 ]; -uint32_t ih_ring_entry[DIV_ROUND_UP ( - dev->device_info->ih_ring_entry_size, -sizeof (uint32_t ))]; +if (dev->device_info->ih_ring_entry_size > (4 *sizeof (uint32_t ))) { +dev_err (kfd_chardev (),"Ring entry too small\n" ); +return ; + }while (dequeue_ih_ring_entry (dev, ih_ring_entry)) dev->device_info->event_interrupt_class->interrupt_wq(dev,
Felix Kuehling
maintainerThanks Christian for catching that. I'm working on a patch series to upstream Vega10 support, about 95% done. It will add this ASIC info for Vega10:
static const struct kfd_device_info vega10_device_info = {
.asic_family = CHIP_VEGA10,
.max_pasid_bits = 16,
.max_no_of_hqd = 24,
.doorbell_size = 8,
.ih_ring_entry_size = 8 * sizeof(uint32_t), /* !!! IH ring entry size is bigger on Vega10 !!! */ .event_interrupt_class = &event_interrupt_class_v9,
.num_of_watch_points = 4,
.mqd_size_aligned = MQD_SIZE_ALIGNED,
.supports_cwsr = true,
.needs_iommu_device = false,
.needs_pci_atomics = false,
};
If you change it to uint32_t ih_ring_entry[8] and update the check, it should be reasonably future proof.
Regards,
Felix
Quoted 45 lines · Christian König
Am 09.04.2018 um 23:06 schrieb Laura Abbott:
Quoted 4 lines
There's an ongoing effort to remove VLAs[1] from the kernel to eventually
turn on -Wvla. The single VLA usage in the amdkfd driver is actually constant across all current platforms.Actually that isn't correct.
Could be that we haven't upstreamed KFD support for them, but Vega10 have a different interrupt ring entry size and so would cause the error message here.
Quoted 2 lines
Switch to a constant size array
instead.I would say to just make make the array bigger.
Regards,
Christian.Quoted 25 lines
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Laura Abbott <[email protected]>---
drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c| 8 +++++---1 file changed, 5 insertions(+), 3 deletions(-)drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +1
-0@@ -139,10 +139,12 @@ static void interrupt_wq(struct work_struct *work) { struct kfd_dev *dev = container_of(work, struct kfd_dev, interrupt_work); + uint32_t ih_ring_entry[4];-
uint32_tih_ring_entry[DIV_ROUND_UP(
-dev->device_info->ih_ring_entry_size, -sizeof(uint32_t))];
+ if (dev->device_info->ih_ring_entry_size> (4 *sizeof(uint32_t))) {
+dev_err(kfd_chardev(), "Ring entry too small\n"); + return;
Quoted 8 lines
Quoted 3 lines
+ }
while (dequeue_ih_ring_entry(dev, ih_ring_entry)) dev->device_info->event_interrupt_class->interrupt_wq(dev,_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Quoted 9 lines · Christian König
Am 09.04.2018 um 23:06 schrieb Laura Abbott:
Quoted 3 lines
There's an ongoing effort to remove VLAs[1] from the kernel to eventually turn on -Wvla. The single VLA usage in the amdkfd driver is actually constant across all current platforms.
Actually that isn't correct.
Could be that we haven't upstreamed KFD support for them, but Vega10 have a different interrupt ring entry size and so would cause the error message here.
Quoted 8 lines
Quoted 2 lines
Switch to a constant size array
instead.I would say to just make make the array bigger.
Regards,
Christian.
What array size would accommodate future chips?
Quoted 18 lines
Quoted 18 lines
[1] https://lkml.org/lkml/2018/3/7/621
Signed-off-by Laura Abbott <[email protected]>---
drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c| 8 +++++---1 file changed, 5 insertions(+), 3 deletions(-)drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +1
-1@@ -139,10 +139,12 @@ static void interrupt_wq(struct work_struct *work) { struct kfd_dev *dev = container_of(work, struct kfd_dev, interrupt_work); + uint32_t ih_ring_entry[4]; - uint32_t ih_ring_entry[DIV_ROUND_UP(
Quoted 9 lines
drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c static void interrupt_wq(struct work_struct *work) · line 150
Quoted 8 lines
drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +4
-2- dev->device_info->ih_ring_entry_size, - sizeof(uint32_t))]; + if (dev->device_info->ih_ring_entry_size > (4 * sizeof(uint32_t))) { + dev_err(kfd_chardev(), "Ring entry too small\n"); + return; + } while (dequeue_ih_ring_entry(dev, ih_ring_entry)) dev->device_info->event_interrupt_class->interrupt_wq(dev,
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx