On Tue, Dec 5, 2017 at 9:14 PM, Felix Kuehling <felix.kuehling at amd.com> wrote: > On 2017-12-05 03:43 AM, Oded Gabbay wrote: >> On Tue, Nov 28, 2017 at 1:29 AM, Felix Kuehling <Felix.Kuehling at amd.com> wrote: >>> Increment the kfd_process.lead_thread's reference counter to make >>> it safe to dereference. This is needed for getting a safe reference >>> to the process' mm_struct. >> I don't object to this patch, but I thought we don't dereference the >> process' mm_struct... > > Correct. Instead we get the mm by calling get_task_mm(p->lead_thread). > But for that we need to properly hold a reference to the task. Otherwise > p->lead_thread may be pointing to a task that doesn't exist any more. I > missed that part when I first ported that change. > > Regards, > Felix Yes, agreed (I missed that too). This patch is: Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com> > >> >> From kfd_priv.h: >> >> /* >> * Opaque pointer to mm_struct. We don't hold a reference to >> * it so it should never be dereferenced from here. This is >> * only used for looking up processes by their mm. >> */ >> void *mm; >> >> >>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com> >>> --- >>> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >>> index 99c18ee..660d8bc 100644 >>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c >>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >>> @@ -24,6 +24,7 @@ >>> #include <linux/log2.h> >>> #include <linux/sched.h> >>> #include <linux/sched/mm.h> >>> +#include <linux/sched/task.h> >>> #include <linux/slab.h> >>> #include <linux/amd-iommu.h> >>> #include <linux/notifier.h> >>> @@ -191,6 +192,8 @@ static void kfd_process_wq_release(struct work_struct *work) >>> >>> mutex_destroy(&p->mutex); >>> >>> + put_task_struct(p->lead_thread); >>> + >>> kfree(p); >>> >>> kfree(work); >>> @@ -342,6 +345,7 @@ static struct kfd_process *create_process(const struct task_struct *thread) >>> (uintptr_t)process->mm); >>> >>> process->lead_thread = thread->group_leader; >>> + get_task_struct(process->lead_thread); >>> >>> INIT_LIST_HEAD(&process->per_device_data); >>> >>> -- >>> 2.7.4 >>> >