Re: [PATCH V3 2/5] accel/amdxdna: Replace idr api with xarray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/13/2024 17:29, Lizhi Hou wrote:
Switch mailbox message id and hardware context id management over from
the idr api to the xarray api.

Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
  drivers/accel/amdxdna/TODO              |  1 -
  drivers/accel/amdxdna/aie2_ctx.c        |  5 ++-
  drivers/accel/amdxdna/aie2_message.c    |  5 ++-
  drivers/accel/amdxdna/aie2_pci.c        |  6 +--
  drivers/accel/amdxdna/amdxdna_ctx.c     | 47 +++++++++------------
  drivers/accel/amdxdna/amdxdna_mailbox.c | 56 ++++++++++---------------
  drivers/accel/amdxdna/amdxdna_pci_drv.c |  4 +-
  drivers/accel/amdxdna/amdxdna_pci_drv.h |  8 +++-
  8 files changed, 60 insertions(+), 72 deletions(-)

diff --git a/drivers/accel/amdxdna/TODO b/drivers/accel/amdxdna/TODO
index a130259f5f70..de4e1dbc8868 100644
--- a/drivers/accel/amdxdna/TODO
+++ b/drivers/accel/amdxdna/TODO
@@ -1,4 +1,3 @@
-- Replace idr with xa
  - Add import and export BO support
  - Add debugfs support
  - Add debug BO support
diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index b5282555bbf2..7218f751144c 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -11,6 +11,7 @@
  #include <drm/drm_syncobj.h>
  #include <linux/hmm.h>
  #include <linux/types.h>
+#include <linux/xarray.h>
  #include <trace/events/amdxdna.h>
#include "aie2_msg_priv.h"
@@ -90,11 +91,11 @@ void aie2_restart_ctx(struct amdxdna_client *client)
  {
  	struct amdxdna_dev *xdna = client->xdna;
  	struct amdxdna_hwctx *hwctx;
-	int next = 0;
+	unsigned long hwctx_id;
drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
  	mutex_lock(&client->hwctx_lock);
-	idr_for_each_entry_continue(&client->hwctx_idr, hwctx, next) {
+	amdxdna_for_each_hwctx(client, hwctx_id, hwctx) {
  		if (hwctx->status != HWCTX_STAT_STOP)
  			continue;
diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
index c01a1d957b56..c90f5e2fc115 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -14,6 +14,7 @@
  #include <linux/errno.h>
  #include <linux/pci.h>
  #include <linux/types.h>
+#include <linux/xarray.h>
#include "aie2_msg_priv.h"
  #include "aie2_pci.h"
@@ -315,10 +316,10 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
  	struct amdxdna_dev *xdna = ndev->xdna;
  	struct amdxdna_client *client;
  	struct amdxdna_hwctx *hwctx;
+	unsigned long hwctx_id;
  	dma_addr_t dma_addr;
  	u32 aie_bitmap = 0;
  	u8 *buff_addr;
-	int next = 0;
  	int ret, idx;
buff_addr = dma_alloc_noncoherent(xdna->ddev.dev, size, &dma_addr,
@@ -329,7 +330,7 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
  	/* Go through each hardware context and mark the AIE columns that are active */
  	list_for_each_entry(client, &xdna->client_list, node) {
  		idx = srcu_read_lock(&client->hwctx_srcu);
-		idr_for_each_entry_continue(&client->hwctx_idr, hwctx, next)
+		amdxdna_for_each_hwctx(client, hwctx_id, hwctx)
  			aie_bitmap |= amdxdna_hwctx_col_map(hwctx);
  		srcu_read_unlock(&client->hwctx_srcu, idx);
  	}
diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index ebf68e3f8341..25c4215cc456 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -15,6 +15,7 @@
  #include <linux/iommu.h>
  #include <linux/iopoll.h>
  #include <linux/pci.h>
+#include <linux/xarray.h>
#include "aie2_msg_priv.h"
  #include "aie2_pci.h"
@@ -691,11 +692,11 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
  	struct amdxdna_drm_query_hwctx *tmp;
  	struct amdxdna_client *tmp_client;
  	struct amdxdna_hwctx *hwctx;
+	unsigned long hwctx_id;
  	bool overflow = false;
  	u32 req_bytes = 0;
  	u32 hw_i = 0;
  	int ret = 0;
-	int next;
  	int idx;
drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
@@ -707,8 +708,7 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
  	buf = u64_to_user_ptr(args->buffer);
  	list_for_each_entry(tmp_client, &xdna->client_list, node) {
  		idx = srcu_read_lock(&tmp_client->hwctx_srcu);
-		next = 0;
-		idr_for_each_entry_continue(&tmp_client->hwctx_idr, hwctx, next) {
+		amdxdna_for_each_hwctx(tmp_client, hwctx_id, hwctx) {
  			req_bytes += sizeof(*tmp);
  			if (args->buffer_size < req_bytes) {
  				/* Continue iterating to get the required size */
diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
index 5478b631b73f..324f35c43f6c 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.c
+++ b/drivers/accel/amdxdna/amdxdna_ctx.c
@@ -11,6 +11,7 @@
  #include <drm/drm_gem_shmem_helper.h>
  #include <drm/drm_print.h>
  #include <drm/gpu_scheduler.h>
+#include <linux/xarray.h>
  #include <trace/events/amdxdna.h>
#include "amdxdna_ctx.h"
@@ -63,11 +64,11 @@ void amdxdna_hwctx_suspend(struct amdxdna_client *client)
  {
  	struct amdxdna_dev *xdna = client->xdna;
  	struct amdxdna_hwctx *hwctx;
-	int next = 0;
+	unsigned long hwctx_id;
drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
  	mutex_lock(&client->hwctx_lock);
-	idr_for_each_entry_continue(&client->hwctx_idr, hwctx, next)
+	amdxdna_for_each_hwctx(client, hwctx_id, hwctx)
  		xdna->dev_info->ops->hwctx_suspend(hwctx);
  	mutex_unlock(&client->hwctx_lock);
  }
@@ -76,11 +77,11 @@ void amdxdna_hwctx_resume(struct amdxdna_client *client)
  {
  	struct amdxdna_dev *xdna = client->xdna;
  	struct amdxdna_hwctx *hwctx;
-	int next = 0;
+	unsigned long hwctx_id;
drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
  	mutex_lock(&client->hwctx_lock);
-	idr_for_each_entry_continue(&client->hwctx_idr, hwctx, next)
+	amdxdna_for_each_hwctx(client, hwctx_id, hwctx)
  		xdna->dev_info->ops->hwctx_resume(hwctx);
  	mutex_unlock(&client->hwctx_lock);
  }
@@ -149,13 +150,13 @@ int amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo)
  void amdxdna_hwctx_remove_all(struct amdxdna_client *client)
  {
  	struct amdxdna_hwctx *hwctx;
-	int next = 0;
+	unsigned long hwctx_id;
mutex_lock(&client->hwctx_lock);
-	idr_for_each_entry_continue(&client->hwctx_idr, hwctx, next) {
+	amdxdna_for_each_hwctx(client, hwctx_id, hwctx) {
  		XDNA_DBG(client->xdna, "PID %d close HW context %d",
  			 client->pid, hwctx->id);
-		idr_remove(&client->hwctx_idr, hwctx->id);
+		xa_erase(&client->hwctx_xa, hwctx->id);
  		mutex_unlock(&client->hwctx_lock);
  		amdxdna_hwctx_destroy_rcu(hwctx, &client->hwctx_srcu);
  		mutex_lock(&client->hwctx_lock);
@@ -194,15 +195,13 @@ int amdxdna_drm_create_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
  	hwctx->num_tiles = args->num_tiles;
  	hwctx->mem_size = args->mem_size;
  	hwctx->max_opc = args->max_opc;
-	mutex_lock(&client->hwctx_lock);
-	ret = idr_alloc_cyclic(&client->hwctx_idr, hwctx, 0, MAX_HWCTX_ID, GFP_KERNEL);
+	ret = xa_alloc_cyclic(&client->hwctx_xa, &hwctx->id, hwctx,
+			      XA_LIMIT(AMDXDNA_INVALID_CTX_HANDLE + 1, MAX_HWCTX_ID),
+			      &client->next_hwctxid, GFP_KERNEL);
  	if (ret < 0) {
-		mutex_unlock(&client->hwctx_lock);
  		XDNA_ERR(xdna, "Allocate hwctx ID failed, ret %d", ret);
  		goto free_hwctx;
  	}
-	hwctx->id = ret;
-	mutex_unlock(&client->hwctx_lock);
hwctx->name = kasprintf(GFP_KERNEL, "hwctx.%d.%d", client->pid, hwctx->id);
  	if (!hwctx->name) {
@@ -228,9 +227,7 @@ int amdxdna_drm_create_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
  free_name:
  	kfree(hwctx->name);
  rm_id:
-	mutex_lock(&client->hwctx_lock);
-	idr_remove(&client->hwctx_idr, hwctx->id);
-	mutex_unlock(&client->hwctx_lock);
+	xa_erase(&client->hwctx_xa, hwctx->id);
  free_hwctx:
  	kfree(hwctx);
  exit:
@@ -249,24 +246,18 @@ int amdxdna_drm_destroy_hwctx_ioctl(struct drm_device *dev, void *data, struct d
  	if (!drm_dev_enter(dev, &idx))
  		return -ENODEV;
- /*
-	 * Use hwctx_lock to achieve exclusion with other hwctx writers,
-	 * SRCU to synchronize with exec/wait command ioctls.
-	 *
-	 * The pushed jobs are handled by DRM scheduler during destroy.
-	 */
-	mutex_lock(&client->hwctx_lock);
-	hwctx = idr_find(&client->hwctx_idr, args->handle);
+	hwctx = xa_erase(&client->hwctx_xa, args->handle);
  	if (!hwctx) {
-		mutex_unlock(&client->hwctx_lock);
  		ret = -EINVAL;
  		XDNA_DBG(xdna, "PID %d HW context %d not exist",
  			 client->pid, args->handle);
  		goto out;
  	}
-	idr_remove(&client->hwctx_idr, hwctx->id);
-	mutex_unlock(&client->hwctx_lock);
+ /*
+	 * The pushed jobs are handled by DRM scheduler during destroy.
+	 * SRCU to synchronize with exec command ioctls.
+	 */
  	amdxdna_hwctx_destroy_rcu(hwctx, &client->hwctx_srcu);
XDNA_DBG(xdna, "PID %d destroyed HW context %d", client->pid, args->handle);
@@ -324,7 +315,7 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
mutex_lock(&xdna->dev_lock);
  	idx = srcu_read_lock(&client->hwctx_srcu);
-	hwctx = idr_find(&client->hwctx_idr, args->handle);
+	hwctx = xa_load(&client->hwctx_xa, args->handle);
  	if (!hwctx) {
  		XDNA_DBG(xdna, "PID %d failed to get hwctx %d", client->pid, args->handle);
  		ret = -EINVAL;
@@ -436,7 +427,7 @@ int amdxdna_cmd_submit(struct amdxdna_client *client,
  	}
idx = srcu_read_lock(&client->hwctx_srcu);
-	hwctx = idr_find(&client->hwctx_idr, hwctx_hdl);
+	hwctx = xa_load(&client->hwctx_xa, hwctx_hdl);
  	if (!hwctx) {
  		XDNA_DBG(xdna, "PID %d failed to get hwctx %d",
  			 client->pid, hwctx_hdl);
diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c
index 378d985222cd..1afc8079e3d1 100644
--- a/drivers/accel/amdxdna/amdxdna_mailbox.c
+++ b/drivers/accel/amdxdna/amdxdna_mailbox.c
@@ -8,6 +8,7 @@
  #include <linux/bitfield.h>
  #include <linux/interrupt.h>
  #include <linux/iopoll.h>
+#include <linux/xarray.h>
#define CREATE_TRACE_POINTS
  #include <trace/events/amdxdna.h>
@@ -55,8 +56,8 @@ struct mailbox_channel {
  	struct xdna_mailbox_chann_res	res[CHAN_RES_NUM];
  	int				msix_irq;
  	u32				iohub_int_addr;
-	struct idr			chan_idr;
-	spinlock_t			chan_idr_lock; /* protect chan_idr */
+	struct xarray			chan_xa;
+	u32				next_msgid;
  	u32				x2i_tail;
/* Received msg related fields */
@@ -165,19 +166,17 @@ static inline int mailbox_validate_msgid(int msg_id)
static int mailbox_acquire_msgid(struct mailbox_channel *mb_chann, struct mailbox_msg *mb_msg)
  {
-	unsigned long flags;
-	int msg_id;
+	u32 msg_id;
+	int ret;
- spin_lock_irqsave(&mb_chann->chan_idr_lock, flags);
-	msg_id = idr_alloc_cyclic(&mb_chann->chan_idr, mb_msg, 0,
-				  MAX_MSG_ID_ENTRIES, GFP_NOWAIT);
-	spin_unlock_irqrestore(&mb_chann->chan_idr_lock, flags);
-	if (msg_id < 0)
-		return msg_id;
+	ret = xa_alloc_cyclic_irq(&mb_chann->chan_xa, &msg_id, mb_msg,
+				  XA_LIMIT(0, MAX_MSG_ID_ENTRIES - 1),
+				  &mb_chann->next_msgid, GFP_NOWAIT);
+	if (ret < 0)
+		return ret;
/*
-	 * The IDR becomes less efficient when dealing with larger IDs.
-	 * Thus, add MAGIC_VAL to the higher bits.
+	 * Add MAGIC_VAL to the higher bits.
  	 */
  	msg_id |= MAGIC_VAL;
  	return msg_id;
@@ -185,25 +184,17 @@ static int mailbox_acquire_msgid(struct mailbox_channel *mb_chann, struct mailbo
static void mailbox_release_msgid(struct mailbox_channel *mb_chann, int msg_id)
  {
-	unsigned long flags;
-
  	msg_id &= ~MAGIC_VAL_MASK;
-	spin_lock_irqsave(&mb_chann->chan_idr_lock, flags);
-	idr_remove(&mb_chann->chan_idr, msg_id);
-	spin_unlock_irqrestore(&mb_chann->chan_idr_lock, flags);
+	xa_erase_irq(&mb_chann->chan_xa, msg_id);
  }
-static int mailbox_release_msg(int id, void *p, void *data)
+static void mailbox_release_msg(struct mailbox_channel *mb_chann,
+				struct mailbox_msg *mb_msg)
  {
-	struct mailbox_channel *mb_chann = data;
-	struct mailbox_msg *mb_msg = p;
-
  	MB_DBG(mb_chann, "msg_id 0x%x msg opcode 0x%x",
  	       mb_msg->pkg.header.id, mb_msg->pkg.header.opcode);
  	mb_msg->notify_cb(mb_msg->handle, NULL, 0);
  	kfree(mb_msg);
-
-	return 0;
  }
static int
@@ -255,7 +246,6 @@ mailbox_get_resp(struct mailbox_channel *mb_chann, struct xdna_msg_header *heade
  		 void *data)
  {
  	struct mailbox_msg *mb_msg;
-	unsigned long flags;
  	int msg_id;
  	int ret;
@@ -266,15 +256,11 @@ mailbox_get_resp(struct mailbox_channel *mb_chann, struct xdna_msg_header *heade
  	}
msg_id &= ~MAGIC_VAL_MASK;
-	spin_lock_irqsave(&mb_chann->chan_idr_lock, flags);
-	mb_msg = idr_find(&mb_chann->chan_idr, msg_id);
+	mb_msg = xa_erase_irq(&mb_chann->chan_xa, msg_id);
  	if (!mb_msg) {
  		MB_ERR(mb_chann, "Cannot find msg 0x%x", msg_id);
-		spin_unlock_irqrestore(&mb_chann->chan_idr_lock, flags);
  		return -EINVAL;
  	}
-	idr_remove(&mb_chann->chan_idr, msg_id);
-	spin_unlock_irqrestore(&mb_chann->chan_idr_lock, flags);
MB_DBG(mb_chann, "opcode 0x%x size %d id 0x%x",
  	       header->opcode, header->total_size, header->id);
@@ -498,8 +484,7 @@ xdna_mailbox_create_channel(struct mailbox *mb,
  	memcpy(&mb_chann->res[CHAN_RES_X2I], x2i, sizeof(*x2i));
  	memcpy(&mb_chann->res[CHAN_RES_I2X], i2x, sizeof(*i2x));
- spin_lock_init(&mb_chann->chan_idr_lock);
-	idr_init(&mb_chann->chan_idr);
+	xa_init_flags(&mb_chann->chan_xa, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
  	mb_chann->x2i_tail = mailbox_get_tailptr(mb_chann, CHAN_RES_X2I);
  	mb_chann->i2x_head = mailbox_get_headptr(mb_chann, CHAN_RES_I2X);
@@ -531,13 +516,18 @@ xdna_mailbox_create_channel(struct mailbox *mb, int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann)
  {
+	struct mailbox_msg *mb_msg;
+	unsigned long msg_id;
+
  	MB_DBG(mb_chann, "IRQ disabled and RX work cancelled");
  	free_irq(mb_chann->msix_irq, mb_chann);
  	destroy_workqueue(mb_chann->work_q);
  	/* We can clean up and release resources */
- idr_for_each(&mb_chann->chan_idr, mailbox_release_msg, mb_chann);
-	idr_destroy(&mb_chann->chan_idr);
+	xa_for_each(&mb_chann->chan_xa, msg_id, mb_msg)
+		mailbox_release_msg(mb_chann, mb_msg);
+
+	xa_destroy(&mb_chann->chan_xa);
MB_DBG(mb_chann, "Mailbox channel destroyed, irq: %d", mb_chann->msix_irq);
  	kfree(mb_chann);
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index 0ba2af987837..c00ca314d033 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -78,7 +78,7 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
  	}
  	mutex_init(&client->hwctx_lock);
  	init_srcu_struct(&client->hwctx_srcu);
-	idr_init_base(&client->hwctx_idr, AMDXDNA_INVALID_CTX_HANDLE + 1);
+	xa_init_flags(&client->hwctx_xa, XA_FLAGS_ALLOC);
  	mutex_init(&client->mm_lock);
mutex_lock(&xdna->dev_lock);
@@ -109,7 +109,7 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp)
XDNA_DBG(xdna, "closing pid %d", client->pid); - idr_destroy(&client->hwctx_idr);
+	xa_destroy(&client->hwctx_xa);
  	cleanup_srcu_struct(&client->hwctx_srcu);
  	mutex_destroy(&client->hwctx_lock);
  	mutex_destroy(&client->mm_lock);
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h
index 0c2e31c05d3e..b62b63a66098 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
@@ -6,6 +6,8 @@
  #ifndef _AMDXDNA_PCI_DRV_H_
  #define _AMDXDNA_PCI_DRV_H_
+#include <linux/xarray.h>
+
  #define XDNA_INFO(xdna, fmt, args...)	drm_info(&(xdna)->ddev, fmt, ##args)
  #define XDNA_WARN(xdna, fmt, args...)	drm_warn(&(xdna)->ddev, "%s: "fmt, __func__, ##args)
  #define XDNA_ERR(xdna, fmt, args...)	drm_err(&(xdna)->ddev, "%s: "fmt, __func__, ##args)
@@ -100,7 +102,8 @@ struct amdxdna_client {
  	struct mutex			hwctx_lock; /* protect hwctx */
  	/* do NOT wait this srcu when hwctx_lock is held */
  	struct srcu_struct		hwctx_srcu;
-	struct idr			hwctx_idr;
+	struct xarray			hwctx_xa;
+	u32				next_hwctxid;
  	struct amdxdna_dev		*xdna;
  	struct drm_file			*filp;
@@ -111,6 +114,9 @@ struct amdxdna_client {
  	int				pasid;
  };
+#define amdxdna_for_each_hwctx(client, hwctx_id, entry) \
+	xa_for_each(&(client)->hwctx_xa, hwctx_id, entry)
+
  /* Add device info below */
  extern const struct amdxdna_dev_info dev_npu1_info;
  extern const struct amdxdna_dev_info dev_npu2_info;




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux