Re: [PATCH V2 07/13] powerpc/vas: Take reference to PID and mm for user space windows

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

 



On Thu, 2019-12-12 at 05:02 -0800, Christoph Hellwig wrote:
> > +	if (txwin->user_win) {
> > +		/*
> > +		 * Window opened by child thread may not be closed when
> > +		 * it exits. So take reference to its pid and release it
> > +		 * when the window is free by parent thread.
> > +		 * Acquire a reference to the task's pid to make sure
> > +		 * pid will not be re-used.
> > +		 */
> > +		txwin->pid = get_task_pid(current, PIDTYPE_PID);
> > +		/*
> > +		 * Acquire a reference to the task's mm.
> > +		 */
> > +		txwin->mm = get_task_mm(current);
> > +
> > +		if (txwin->mm) {
> > +			mmput(txwin->mm);
> > +			mmgrab(txwin->mm);
> 
> Doesn't the mmgrab need to come before the mmput?
> 
> > +			mm_context_add_copro(txwin->mm);
> > +		} else {
> > +			put_pid(txwin->pid);
> > +			pr_err("VAS: pid(%d): mm_struct is not found\n",
> > +					current->pid);
> > +			rc = -EPERM;
> > +			goto free_window;
> > +		}
> 
> Also the code is much easier to follow if you handle the error
> first and avoid the else:
> 
> 		txwin->mm = get_task_mm(current);
> 		if (!txwin->mm) {
> 			put_pid(txwin->pid);
> 			pr_err("VAS: pid(%d): mm_struct is not found\n",
> 					current->pid);
> 			rc = -EPERM;
> 			goto free_window;
> 		}
> 		mmgrab(txwin->mm);
> 		mmput(txwin->mm);
> 
> Also don't you need to take a reference to the struct pid for the
> tgid as well?

Process opens window and closed it upon its exit. We do not have to take
pid reference in this case. But in multithread applications, child
thread can open window, can exit without closing it. Parent thread can
use this window and expects to close it later. So in this case pid
reference for child thread is needed. Where as parent thread is the one
who is closing the window later, do not to have to take its pid
reference.

Basically we are taking pid reference to the process who opens the
window to cover multithread applications.

Thanks for your review comments. I will post V3 patches.





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux