Re: [PATCH] drm/amdkfd: Fix CRIU restore op due to doorbell offset

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

 



On 2022-09-07 14:36, Rajneesh Bhardwaj wrote:
Recently introduced change to allocate doorbells only when the first
queue is created or mapped for CPU / GPU access, did not consider
Checkpoint Restore scenario completely. This fix allows the CRIU restore
operation by extedning the doorbell optimization to CRIU restore
scenario.

typo: extending

A few more nit-picks inline.



Fixes: 'commit 15bcfbc55b57 ("drm/amdkfd: Allocate doorbells only when needed")'

Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@xxxxxxx>
---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c               | 8 ++++++++
  drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c              | 4 +++-
  drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 9 +++++++++
  3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 84da1a9ce37c..c476993e3927 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2153,6 +2153,13 @@ static int criu_restore_devices(struct kfd_process *p,
  			ret = PTR_ERR(pdd);
  			goto exit;
  		}
+
+		if (!pdd->doorbell_index &&
+			kfd_alloc_process_doorbells(pdd->dev, &pdd->doorbell_index) < 0) {
+			pr_err("Failed to allocate process doorbells\n");
+			ret = -ENOMEM;
+			goto err_alloc_doorbells;
+	}

Incorrect indentation. The closing brace should be indented one more tab. And the if condition would be more readable if the second line was aligned under the !pdd->doorbell_index.

You don't need a new err_alloc_doorbells label. Just goto exit.


  	}
/*
@@ -2161,6 +2168,7 @@ static int criu_restore_devices(struct kfd_process *p,
  	 */
  	*priv_offset += args->num_devices * sizeof(*device_privs);
+err_alloc_doorbells:
  exit:
  	kfree(device_buckets);
  	return ret;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
index b33798f89ef0..7690514c4eb3 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c
@@ -157,8 +157,10 @@ int kfd_doorbell_mmap(struct kfd_dev *dev, struct kfd_process *process,
/* Calculate physical address of doorbell */
  	address = kfd_get_process_doorbells(pdd);
-	if (!address)
+	if (!address) {
+		pr_err("Failed to  get physical address of process doorbell\n");

Please use the same error message as above for consistency. Or better yet, move the error printing into kfd_alloc_process_doorbells, so you don't have to duplicate the same error message in multiple places.


  		return -ENOMEM;
+	}
  	vma->vm_flags |= VM_IO | VM_DONTCOPY | VM_DONTEXPAND | VM_NORESERVE |
  				VM_DONTDUMP | VM_PFNMAP;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 6e3e7f54381b..9f05f64c5af8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -857,6 +857,14 @@ int kfd_criu_restore_queue(struct kfd_process *p,
  		ret = -EINVAL;
  		goto exit;
  	}
+
+	if (!pdd->doorbell_index &&
+	    kfd_alloc_process_doorbells(pdd->dev, &pdd->doorbell_index) < 0) {
+		pr_err("Failed to alloc process doorbells\n");

Same as above.


+		ret = -ENOMEM;
+		goto err_alloc_doorbells;

You don't need a new err_alloc_doorbells label. Just goto exit.


+	}
+
  	/* data stored in this order: mqd, ctl_stack */
  	mqd = q_extra_data;
  	ctl_stack = mqd + q_data->mqd_size;
@@ -876,6 +884,7 @@ int kfd_criu_restore_queue(struct kfd_process *p,
  	if (q_data->gws)
  		ret = pqm_set_gws(&p->pqm, q_data->q_id, pdd->dev->gws);
+err_alloc_doorbells:
  exit:
  	if (ret)
  		pr_err("Failed to restore queue (%d)\n", ret);



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux