Re: [PATCH v0.9.1 3/6] sched/umcg: implement UMCG syscalls

From: Peter Zijlstra <peterz@infradead.org>
To: Peter Oskolkov <posk@posk.io>
Cc: Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-api@vger.kernel.org, Paul Turner <pjt@google.com>,
	Ben Segall <bsegall@google.com>, Peter Oskolkov <posk@google.com>,
	Andrei Vagin <avagin@google.com>, Jann Horn <jannh@google.com>,
	Thierry Delisle <tdelisle@uwaterloo.ca>
Subject: Re: [PATCH v0.9.1 3/6] sched/umcg: implement UMCG syscalls
Date: Fri, 26 Nov 2021 23:16:43 +0100	[thread overview]
Message-ID: <YaFcyzq0WpnXu+2R@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <YaEUts3RbOLyvAjl@hirez.programming.kicks-ass.net>

On Fri, Nov 26, 2021 at 06:09:10PM +0100, Peter Zijlstra wrote:

> @@ -155,8 +159,7 @@ static unsigned long exit_to_user_mode_l
>  	 * Before returning to user space ensure that all pending work
>  	 * items have been completed.
>  	 */
> -	while (ti_work & EXIT_TO_USER_MODE_WORK) {
> -
> +	do {
>  		local_irq_enable_exit_to_user(ti_work);
>  
>  		if (ti_work & _TIF_NEED_RESCHED)
> @@ -168,6 +171,10 @@ static unsigned long exit_to_user_mode_l
>  		if (ti_work & _TIF_PATCH_PENDING)
>  			klp_update_patch_state(current);
>  
> +		/* must be before handle_signal_work(); terminates on sigpending */
> +		if (ti_work & _TIF_UMCG)
> +			umcg_notify_resume(regs);
> +
>  		if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
>  			handle_signal_work(regs, ti_work);
>  
> @@ -188,7 +195,7 @@ static unsigned long exit_to_user_mode_l
>  		tick_nohz_user_enter_prepare();
>  
>  		ti_work = READ_ONCE(current_thread_info()->flags);
> -	}
> +	} while (ti_work & EXIT_TO_USER_MODE_WORK);
>  
>  	/* Return the latest work state for arch_exit_to_user_mode() */
>  	return ti_work;
> @@ -203,7 +210,7 @@ static void exit_to_user_mode_prepare(st
>  	/* Flush pending rcuog wakeup before the last need_resched() check */
>  	tick_nohz_user_enter_prepare();
>  
> -	if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
> +	if (unlikely(ti_work & (EXIT_TO_USER_MODE_WORK | _TIF_UMCG)))
>  		ti_work = exit_to_user_mode_loop(regs, ti_work);
>  
>  	arch_exit_to_user_mode_prepare(regs, ti_work);

Thomas, since you're looking at this. I'm not quite sure I got this
right. The intent is that when _TIF_UMCG is set (and it is never cleared
until the task unregisters) it is called at least once.

The thinking is that if umcg_wait() gets interrupted, we'll drop out,
handle the signal and then resume the wait, which can obviously happen
any number of times.

It's just that I'm never quite sure where signal crud happens; I'm
assuming handle_signal_work() simply mucks about with regs (sets sp and
ip etc.. to the signal stack) and drops out of kernel mode, and on
re-entry we do this whole merry cycle once again. But I never actually
dug that deep.