When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
Signed-off-by: Oded Gabbay oded.gabbay@amd.com --- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 7d4974b..69c5fe7 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -571,7 +571,7 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) break; }
- if (err < 0) + if ((err < 0) && (err != -EAGAIN)) dev_err(kfd_device, "ioctl error %ld for ioctl cmd 0x%x (#%d)\n", err, cmd, _IOC_NR(cmd));
From: Alexey Skidanov Alexey.Skidanov@amd.com
This patch adds the number of watch points to the node capabilities in the topology module
Reviewed-by: Oded Gabbay oded.gabbay@amd.com Signed-off-by: Alexey Skidanov Alexey.Skidanov@amd.com --- drivers/gpu/drm/amd/amdkfd/kfd_device.c | 1 + drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 1 + drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 13 +++++++++++++ 3 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c index 43884eb..436c31c 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c @@ -32,6 +32,7 @@ static const struct kfd_device_info kaveri_device_info = { .max_pasid_bits = 16, .ih_ring_entry_size = 4 * sizeof(uint32_t), + .num_of_watch_points = 4, .mqd_size_aligned = MQD_SIZE_ALIGNED };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index f9fb81e..ba2bba8 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -107,6 +107,7 @@ enum cache_policy { struct kfd_device_info { unsigned int max_pasid_bits; size_t ih_ring_entry_size; + uint8_t num_of_watch_points; uint16_t mqd_size_aligned; };
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c index b11792d..da34e1f 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c @@ -27,6 +27,7 @@ #include <linux/acpi.h> #include <linux/hash.h> #include <linux/cpufreq.h> +#include <linux/log2.h>
#include "kfd_priv.h" #include "kfd_crat.h" @@ -634,6 +635,7 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr, struct kfd_topology_device *dev; char public_name[KFD_TOPOLOGY_PUBLIC_NAME_SIZE]; uint32_t i; + uint32_t log_max_watch_addr;
/* Making sure that the buffer is an empty string */ buffer[0] = 0; @@ -708,6 +710,17 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr, dev->node_props.location_id);
if (dev->gpu) { + log_max_watch_addr = + __ilog2_u32(dev->gpu->device_info->num_of_watch_points); + + if (log_max_watch_addr) { + dev->node_props.capability |= + HSA_CAP_WATCH_POINTS_SUPPORTED; + dev->node_props.capability |= + (log_max_watch_addr << HSA_CAP_WATCH_POINTS_TOTALBITS_SHIFT) & + HSA_CAP_WATCH_POINTS_TOTALBITS_MASK; + } + sysfs_show_32bit_prop(buffer, "max_engine_clk_fcompute", kfd2kgd->get_max_engine_clock_in_mhz( dev->gpu->kgd));
Signed-off-by: Oded Gabbay oded.gabbay@amd.com --- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 69c5fe7..4083dbc 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -31,7 +31,6 @@ #include <uapi/linux/kfd_ioctl.h> #include <linux/time.h> #include <linux/mm.h> -#include <linux/uaccess.h> #include <uapi/asm-generic/mman-common.h> #include <asm/processor.h> #include "kfd_priv.h"
From: Alexey Skidanov Alexey.Skidanov@amd.com
This patch splits the current kfd_get_process_device_data() to two functions, one that specifically creates a pdd and another one which just do lookup.
This is done to enhance the readability and maintainability of the code.
Signed-off-by: Alexey Skidanov Alexey.Skidanov@amd.com Reviewed-by: Oded Gabbay oded.gabbay@amd.com --- .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 1 - drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c | 4 --- drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c | 13 ++++--- drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 +-- drivers/gpu/drm/amd/amdkfd/kfd_process.c | 40 +++++++++++++--------- .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 14 +++++--- 6 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c index f44d673..7b6df51 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c @@ -75,7 +75,6 @@ get_sh_mem_bases_nybble_64(struct kfd_process_device *pdd) nybble = (pdd->lds_base >> 60) & 0x0E;
return nybble; - }
static inline unsigned int get_sh_mem_bases_32(struct kfd_process_device *pdd) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c index b5791a5..1a9b355 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c @@ -137,10 +137,6 @@ int kfd_doorbell_mmap(struct kfd_process *process, struct vm_area_struct *vma) if (dev == NULL) return -EINVAL;
- /* Find if pdd exists for combination of process and gpu id */ - if (!kfd_get_process_device_data(dev, process, 0)) - return -EINVAL; - /* Calculate physical address of doorbell */ address = kfd_get_process_doorbells(dev, process);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c index e64aa99..5c91029 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c @@ -303,10 +303,11 @@ int kfd_init_apertures(struct kfd_process *process) while ((dev = kfd_topology_enum_kfd_devices(id)) != NULL && id < NUM_OF_SUPPORTED_GPUS) {
- pdd = kfd_get_process_device_data(dev, process, 1); - if (!pdd) - return -1; - + pdd = kfd_create_process_device_data(dev, process); + if (pdd == NULL) { + pr_err("Failed to create process device data\n"); + goto err; + } /* * For 64 bit process aperture will be statically reserved in * the x86_64 non canonical process address space @@ -349,6 +350,10 @@ int kfd_init_apertures(struct kfd_process *process) }
return 0; + +err: + mutex_unlock(&process->mutex); + return -1; }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index ba2bba8..a2e053c 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -473,8 +473,9 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev, struct kfd_process *p); void kfd_unbind_process_from_device(struct kfd_dev *dev, unsigned int pasid); struct kfd_process_device *kfd_get_process_device_data(struct kfd_dev *dev, - struct kfd_process *p, - int create_pdd); + struct kfd_process *p); +struct kfd_process_device *kfd_create_process_device_data(struct kfd_dev *dev, + struct kfd_process *p);
/* Process device data iterator */ struct kfd_process_device *kfd_get_first_process_device_data(struct kfd_process *p); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index 3c76ef0..a369c14 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -311,24 +311,29 @@ err_alloc_process: }
struct kfd_process_device *kfd_get_process_device_data(struct kfd_dev *dev, - struct kfd_process *p, - int create_pdd) + struct kfd_process *p) { struct kfd_process_device *pdd = NULL;
list_for_each_entry(pdd, &p->per_device_data, per_device_list) if (pdd->dev == dev) - return pdd; - - if (create_pdd) { - pdd = kzalloc(sizeof(*pdd), GFP_KERNEL); - if (pdd != NULL) { - pdd->dev = dev; - INIT_LIST_HEAD(&pdd->qpd.queues_list); - INIT_LIST_HEAD(&pdd->qpd.priv_queue_list); - pdd->qpd.dqm = dev->dqm; - list_add(&pdd->per_device_list, &p->per_device_data); - } + break; + + return pdd; +} + +struct kfd_process_device *kfd_create_process_device_data(struct kfd_dev *dev, + struct kfd_process *p) +{ + struct kfd_process_device *pdd = NULL; + + pdd = kzalloc(sizeof(*pdd), GFP_KERNEL); + if (pdd != NULL) { + pdd->dev = dev; + INIT_LIST_HEAD(&pdd->qpd.queues_list); + INIT_LIST_HEAD(&pdd->qpd.priv_queue_list); + pdd->qpd.dqm = dev->dqm; + list_add(&pdd->per_device_list, &p->per_device_data); }
return pdd; @@ -344,11 +349,14 @@ struct kfd_process_device *kfd_get_process_device_data(struct kfd_dev *dev, struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev, struct kfd_process *p) { - struct kfd_process_device *pdd = kfd_get_process_device_data(dev, p, 1); + struct kfd_process_device *pdd; int err;
- if (pdd == NULL) + pdd = kfd_get_process_device_data(dev, p); + if (!pdd) { + pr_err("Process device data doesn't exist\n"); return ERR_PTR(-ENOMEM); + }
if (pdd->bound) return pdd; @@ -384,7 +392,7 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev, unsigned int pasid)
pqm_uninit(&p->pqm);
- pdd = kfd_get_process_device_data(dev, p, 0); + pdd = kfd_get_process_device_data(dev, p);
/* * Just mark pdd as unbound, because we still need it to call 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 4752678..d12f9d3 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c @@ -167,8 +167,11 @@ int pqm_create_queue(struct process_queue_manager *pqm, q = NULL; kq = NULL;
- pdd = kfd_get_process_device_data(dev, pqm->process, 1); - BUG_ON(!pdd); + pdd = kfd_get_process_device_data(dev, pqm->process); + if (!pdd) { + pr_err("Process device data doesn't exist\n"); + return -1; + }
retval = find_available_queue_slot(pqm, qid); if (retval != 0) @@ -273,8 +276,11 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid) dev = pqn->q->device; BUG_ON(!dev);
- pdd = kfd_get_process_device_data(dev, pqm->process, 1); - BUG_ON(!pdd); + pdd = kfd_get_process_device_data(dev, pqm->process); + if (!pdd) { + pr_err("Process device data doesn't exist\n"); + return -1; + }
if (pqn->kq) { /* destroy kernel queue (DIQ) */
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Christian.
Signed-off-by: Oded Gabbay oded.gabbay@amd.com
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 7d4974b..69c5fe7 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -571,7 +571,7 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) break; }
- if (err < 0)
- if ((err < 0) && (err != -EAGAIN)) dev_err(kfd_device, "ioctl error %ld for ioctl cmd 0x%x (#%d)\n", err, cmd, _IOC_NR(cmd));
On 12/14/2014 04:10 PM, Christian König wrote:
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Christian.
Thanks, will fix and resend.
Oded
Signed-off-by: Oded Gabbay oded.gabbay@amd.com
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 7d4974b..69c5fe7 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -571,7 +571,7 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) break; }
- if (err < 0)
- if ((err < 0) && (err != -EAGAIN)) dev_err(kfd_device, "ioctl error %ld for ioctl cmd 0x%x (#%d)\n", err, cmd, _IOC_NR(cmd));
On Sun, Dec 14, 2014 at 03:10:17PM +0100, Christian König wrote:
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Please just reuse drmIoctl or at least copy it perfectly. We've had too many tears about ioctl restarting going badly wrong. Also make sure you never do a raw ioctl call anywhere for amdkfd. Adding Dave.
Thanks, Daniel
On 15 December 2014 at 17:59, Daniel Vetter daniel@ffwll.ch wrote:
On Sun, Dec 14, 2014 at 03:10:17PM +0100, Christian König wrote:
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Please just reuse drmIoctl or at least copy it perfectly. We've had too many tears about ioctl restarting going badly wrong. Also make sure you never do a raw ioctl call anywhere for amdkfd. Adding Dave.
Also please don't make a user triggerable printk.
If the user can throw crap at the ioctl and get msgs in dmesg, then its annoying as hell.
Copy the drm.debug stuff and code as well, and for userspace, yes do what Daniel says and use drmIoctl wrapper or something like that, though Daniel I believe one of the main consumers on i915 insists on opencoding his ioctls.
Dave.
On 12/15/2014 10:32 AM, Dave Airlie wrote:
On 15 December 2014 at 17:59, Daniel Vetter daniel@ffwll.ch wrote:
On Sun, Dec 14, 2014 at 03:10:17PM +0100, Christian König wrote:
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Please just reuse drmIoctl or at least copy it perfectly. We've had too many tears about ioctl restarting going badly wrong. Also make sure you never do a raw ioctl call anywhere for amdkfd. Adding Dave.
Also please don't make a user triggerable printk.
If the user can throw crap at the ioctl and get msgs in dmesg, then its annoying as hell.
Copy the drm.debug stuff and code as well, and for userspace, yes do what Daniel says and use drmIoctl wrapper or something like that, though Daniel I believe one of the main consumers on i915 insists on opencoding his ioctls.
Dave.
Dave & Daniel,
Thanks for the guidance & comments. I will take a look at drmioctl & drm.debug stuff and copy/reuse what's relevant. After that, I'll resubmit this patch, and probably this will generate some more patches for me to submit for review.
Oded
On 12/15/2014 10:32 AM, Dave Airlie wrote:
On 15 December 2014 at 17:59, Daniel Vetter daniel@ffwll.ch wrote:
On Sun, Dec 14, 2014 at 03:10:17PM +0100, Christian König wrote:
Am 14.12.2014 um 14:35 schrieb Oded Gabbay:
When an ioctl function returns -EAGAIN, don't print error in kfd_ioctl()
You most likely want to handle -ERESTARTSYS the same way.
Please just reuse drmIoctl or at least copy it perfectly. We've had too many tears about ioctl restarting going badly wrong. Also make sure you never do a raw ioctl call anywhere for amdkfd. Adding Dave.
Also please don't make a user triggerable printk.
If the user can throw crap at the ioctl and get msgs in dmesg, then its annoying as hell.
Copy the drm.debug stuff and code as well, and for userspace, yes do what Daniel says and use drmIoctl wrapper or something like that, though Daniel I believe one of the main consumers on i915 insists on opencoding his ioctls.
Dave.
Hi Dave, Daniel
I just sent a patch-set that copies the drm_ioctl() handling to kfd_ioctl(), as you requested. All error prints have been converted to debug prints. This is the first part and I'm now going to change the userspace as well.
Oded
dri-devel@lists.freedesktop.org
-
Christian König -
Daniel Vetter -
Dave Airlie -
Oded Gabbay