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

[Christoph Hellwig <hch@infradead.org>]

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