From: Andres Rodriguez <andres.rodriguez@xxxxxxx> Replace our implementation of a lockless ring buffer with the standard linux kernel kfifo. We shouldn't maintain our own version of a standard data structure. Signed-off-by: Andres Rodriguez <andres.rodriguez at amd.com> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com> --- drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 78 ++++++++++-------------------- drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 6 +-- 2 files changed, 27 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c index 70b3a99c..ffbb91a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c @@ -42,25 +42,24 @@ #include <linux/slab.h> #include <linux/device.h> +#include <linux/kfifo.h> #include "kfd_priv.h" -#define KFD_INTERRUPT_RING_SIZE 1024 +#define KFD_IH_NUM_ENTRIES 1024 static void interrupt_wq(struct work_struct *); int kfd_interrupt_init(struct kfd_dev *kfd) { - void *interrupt_ring = kmalloc_array(KFD_INTERRUPT_RING_SIZE, - kfd->device_info->ih_ring_entry_size, - GFP_KERNEL); - if (!interrupt_ring) - return -ENOMEM; - - kfd->interrupt_ring = interrupt_ring; - kfd->interrupt_ring_size = - KFD_INTERRUPT_RING_SIZE * kfd->device_info->ih_ring_entry_size; - atomic_set(&kfd->interrupt_ring_wptr, 0); - atomic_set(&kfd->interrupt_ring_rptr, 0); + int r; + + r = kfifo_alloc(&kfd->ih_fifo, + KFD_IH_NUM_ENTRIES * kfd->device_info->ih_ring_entry_size, + GFP_KERNEL); + if (r) { + dev_err(kfd_chardev(), "Failed to allocate IH fifo\n"); + return r; + } spin_lock_init(&kfd->interrupt_lock); @@ -98,68 +97,41 @@ void kfd_interrupt_exit(struct kfd_dev *kfd) */ flush_scheduled_work(); - kfree(kfd->interrupt_ring); + kfifo_free(&kfd->ih_fifo); } /* - * This assumes that it can't be called concurrently with itself - * but only with dequeue_ih_ring_entry. + * Assumption: single reader/writer. This function is not re-entrant */ bool enqueue_ih_ring_entry(struct kfd_dev *kfd, const void *ih_ring_entry) { - unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr); - unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr); + int count; - if ((rptr - wptr) % kfd->interrupt_ring_size == - kfd->device_info->ih_ring_entry_size) { - /* This is very bad, the system is likely to hang. */ + count = kfifo_in(&kfd->ih_fifo, ih_ring_entry, + kfd->device_info->ih_ring_entry_size); + if (count != kfd->device_info->ih_ring_entry_size) { dev_err_ratelimited(kfd_chardev(), - "Interrupt ring overflow, dropping interrupt.\n"); + "Interrupt ring overflow, dropping interrupt %d\n", + count); return false; } - memcpy(kfd->interrupt_ring + wptr, ih_ring_entry, - kfd->device_info->ih_ring_entry_size); - - wptr = (wptr + kfd->device_info->ih_ring_entry_size) % - kfd->interrupt_ring_size; - smp_wmb(); /* Ensure memcpy'd data is visible before wptr update. */ - atomic_set(&kfd->interrupt_ring_wptr, wptr); - return true; } /* - * This assumes that it can't be called concurrently with itself - * but only with enqueue_ih_ring_entry. + * Assumption: single reader/writer. This function is not re-entrant */ static bool dequeue_ih_ring_entry(struct kfd_dev *kfd, void *ih_ring_entry) { - /* - * Assume that wait queues have an implicit barrier, i.e. anything that - * happened in the ISR before it queued work is visible. - */ - - unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr); - unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr); + int count; - if (rptr == wptr) - return false; - - memcpy(ih_ring_entry, kfd->interrupt_ring + rptr, - kfd->device_info->ih_ring_entry_size); - - rptr = (rptr + kfd->device_info->ih_ring_entry_size) % - kfd->interrupt_ring_size; + count = kfifo_out(&kfd->ih_fifo, ih_ring_entry, + kfd->device_info->ih_ring_entry_size); - /* - * Ensure the rptr write update is not visible until - * memcpy has finished reading. - */ - smp_mb(); - atomic_set(&kfd->interrupt_ring_rptr, rptr); + WARN_ON(count && count != kfd->device_info->ih_ring_entry_size); - return true; + return count == kfd->device_info->ih_ring_entry_size; } static void interrupt_wq(struct work_struct *work) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index ebae8e1..e8d6c0e 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -32,6 +32,7 @@ #include <linux/spinlock.h> #include <linux/kfd_ioctl.h> #include <linux/idr.h> +#include <linux/kfifo.h> #include <kgd_kfd_interface.h> #include "amd_shared.h" @@ -182,10 +183,7 @@ struct kfd_dev { unsigned int gtt_sa_num_of_chunks; /* Interrupts */ - void *interrupt_ring; - size_t interrupt_ring_size; - atomic_t interrupt_ring_rptr; - atomic_t interrupt_ring_wptr; + struct kfifo ih_fifo; struct work_struct interrupt_work; spinlock_t interrupt_lock; -- 2.7.4