From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S932503AbcHIVdv (ORCPT <rfc822;w@1wt.eu>);
	Tue, 9 Aug 2016 17:33:51 -0400
Received: from bombadil.infradead.org ([198.137.202.9]:33405 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932382AbcHIVds (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 9 Aug 2016 17:33:48 -0400
Date: Tue, 9 Aug 2016 23:33:14 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        Russell King <linux@arm.linux.org.uk>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-api <linux-api@vger.kernel.org>, Paul Turner <pjt@google.com>,
        Andrew Hunter <ahh@google.com>, Andy Lutomirski <luto@amacapital.net>,
        Andi Kleen <andi@firstfloor.org>, Dave Watson <davejwatson@fb.com>,
        Chris Lameter <cl@linux.com>, Ben Maurer <bmaurer@fb.com>,
        rostedt <rostedt@goodmis.org>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Michael Kerrisk <mtk.manpages@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>
Subject: Re: [RFC PATCH v7 1/7] Restartable sequences system call
Message-ID: <20160809213314.GK30192@twins.programming.kicks-ass.net>
References: <1469135662-31512-1-git-send-email-mathieu.desnoyers@efficios.com>
 <1469135662-31512-2-git-send-email-mathieu.desnoyers@efficios.com>
 <20160803131940.GM6862@twins.programming.kicks-ass.net>
 <656745027.6624.1470773200334.JavaMail.zimbra@efficios.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <656745027.6624.1470773200334.JavaMail.zimbra@efficios.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Aug 09, 2016 at 08:06:40PM +0000, Mathieu Desnoyers wrote:
> >> +static int rseq_increment_event_counter(struct task_struct *t)
> >> +{
> >> +	if (__put_user(++t->rseq_event_counter,
> >> +			&t->rseq->u.e.event_counter))
> >> +		return -1;
> >> +	return 0;
> >> +}

> >> +void __rseq_handle_notify_resume(struct pt_regs *regs)
> >> +{
> >> +	struct task_struct *t = current;
> >> +
> >> +	if (unlikely(t->flags & PF_EXITING))
> >> +		return;
> >> +	if (!access_ok(VERIFY_WRITE, t->rseq, sizeof(*t->rseq)))
> >> +		goto error;
> >> +	if (__put_user(raw_smp_processor_id(), &t->rseq->u.e.cpu_id))
> >> +		goto error;
> >> +	if (rseq_increment_event_counter(t))
> > 
> > It seems a shame to not use a single __put_user() here. You did the
> > layout to explicitly allow for this, but then you don't.
> 
> The event counter increment needs to be performed at least once before
> returning to user-space whenever the thread is preempted or has a signal
> delivered. This counter increment needs to occur even if we are not nested
> over a restartable assembly block. (more detailed explanation about this
> follows at the end of this email)
> 
> The rseq_ip_fixup only ever needs to update the rseq_cs pointer
> field if it preempts/delivers a signal over a restartable
> assembly block, which happens very rarely.
> 
> Therefore, since the event counter increment is more frequent than
> setting rseq_cs ptr, I don't see much value in trying to combine
> those two into a single __put_user().
> 
> The reason why I combined both the cpu_id and event_counter
> fields into the same 64-bit integer is for user-space rseq_start()
> to be able to fetch them through a single load when the architecture
> allows it.

I wasn't talking about the rseq_up_fixup(), I was talking about both
unconditional __put_user()'s on cpu_id and event_counter.

These are 2 unconditinoal u32 stores that could very easily be done as a
single u64 store (on 64bit hardware).