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 > > 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 >>
