Re: [PATCH V7 08/14] powerpc/vas: Take reference to PID and mm for user space windows

[Haren Myneni <haren@linux.ibm.com>]

On Tue, 2020-03-17 at 15:09 +1100, Michael Ellerman wrote:
> Haren Myneni <haren@linux.ibm.com> writes:
> > Process close windows after its requests are completed. In multi-thread
> > applications, child can open a window but release FD will not be called
> > upon its exit. Parent thread will be closing it later upon its exit.
> 
> What if the parent exits first?

Thanks for the review.

If the parent exists, child thread will close the window when it exits.
So we should not get the case where window is still open. 


> 
> > The parent can also send NX requests with this window and NX can
> > generate page faults. After kernel handles the page fault, send
> > signal to process by using PID if CSB address is invalid. Parent
> > thread will not receive signal since its PID is different from the one
> > saved in vas_window. So use tgid in case if the task for the pid saved
> > in window is not running and send signal to its parent.
> >
> > To prevent reusing the pid until the window closed, take reference to
> > pid and task mm.
> 
> That text is all very dense. Can you please flesh it out and reword it
> to clearly spell out what's going on in much more detail.

Sure, will update the commit description. 
> 
> 
> > diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
> > index a45d81d..7587258 100644
> > --- a/arch/powerpc/platforms/powernv/vas-window.c
> > +++ b/arch/powerpc/platforms/powernv/vas-window.c
> > @@ -1266,8 +1300,17 @@ int vas_win_close(struct vas_window *window)
> >  	poll_window_castout(window);
> >  
> >  	/* if send window, drop reference to matching receive window */
> > -	if (window->tx_win)
> > +	if (window->tx_win) {
> > +		if (window->user_win) {
> > +			/* Drop references to pid and mm */
> > +			put_pid(window->pid);
> > +			if (window->mm) {
> > +				mmdrop(window->mm);
> > +				mm_context_remove_copro(window->mm);
> 
> That seems backward. Once you drop the reference the mm can be freed
> can't it?

Yes, will change.

> 
> > +			}
> > +		}
> >  		put_rx_win(window->rxwin);
> > +	}
> >  
> >  	vas_window_free(window);
> 
> cheers