HI Dan,
I'll comment inline with [KR] for the most part.
Most of these come from the fact that "failure" here is not critical. If we can't make the procfs, we just comment about it in dmesg and carry on. If we fail to make the procfs structure here, we just report and carry on. The rest of the kernel can function without it, so failure isn't critical. But I should make this clear in the comments. There is no real error handling required, but we can clarify that with comments. I'll try to address this in a coming patch.
Kent
-----Original Message-----
From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Sent: Monday, June 24, 2019 8:27 AM
To: Russell, Kent <Kent.Russell@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: [bug report] drm/amdkfd: Add procfs-style information for KFD processes
Hello Kent Russell,
The patch de9f26bbd384: "drm/amdkfd: Add procfs-style information for KFD processes" from Jun 13, 2019, leads to the following static checker warning:
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c:297 kfd_create_process()
error: 'process' dereferencing possible ERR_PTR()
drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_process.c
284 */
285 mutex_lock(&kfd_processes_mutex);
286
287 /* A prior open of /dev/kfd could have already created the process. */
288 process = find_process(thread);
289 if (process) {
290 pr_debug("Process already found\n");
291 } else {
292 process = create_process(thread, filep);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This returns error pointers.
[KR] This was the original code. Not saying it's right though, I'll see if we can address it.
293
294 if (!procfs.kobj)
^^^^^^^^^^^ This is a global. Can we check it earlier?
[KR] The big thing for this is that if the kobj doesn't exist, we can skip the rest. We've basically got 2 things here and I combined them into 1, getting the process and making the procfs entry. I should make comments to clarify.
295 goto out;
296
297 process->kobj = kfd_alloc_struct(process->kobj);
298 if (!process->kobj) {
299 pr_warn("Creating procfs kobject failed");
300 goto out;
We return success on this path.
301 }
302 ret = kobject_init_and_add(process->kobj, &procfs_type,
303 procfs.kobj, "%d",
304 (int)process->lead_thread->pid);
305 if (ret) {
306 pr_warn("Creating procfs pid directory failed");
307 goto out;
No error handling. Basically whenever there is a goto out the error handling is suspect. It's better to pick a name which says what the error label does...
308 }
309
310 process->attr_pasid.name = "pasid";
311 process->attr_pasid.mode = KFD_SYSFS_FILE_MODE;
312 sysfs_attr_init(&process->attr_pasid);
313 ret = sysfs_create_file(process->kobj, &process->attr_pasid);
314 if (ret)
315 pr_warn("Creating pasid for pid %d failed",
316 (int)process->lead_thread->pid);
Error handling and error code missing.
317 }
318 out:
319 mutex_unlock(&kfd_processes_mutex);
320
321 return process;
322 }
regards,
dan carpenter
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
