Re: [RFC PATCH 6/9] livepatch: create per-task consistency model

From: Peter Zijlstra <peterz@infradead.org>
To: Jiri Kosina <jkosina@suse.cz>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	Seth Jennings <sjenning@redhat.com>,
	Vojtech Pavlik <vojtech@suse.cz>
Subject: Re: [RFC PATCH 6/9] livepatch: create per-task consistency model
Date: Thu, 12 Feb 2015 13:36:20 +0100	[thread overview]
Message-ID: <20150212123620.GB23367@worktop.ger.corp.intel.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1502121319440.20672@pobox.suse.cz>

On Thu, Feb 12, 2015 at 01:25:14PM +0100, Jiri Kosina wrote:
> On Thu, 12 Feb 2015, Peter Zijlstra wrote:

> > FWIW, the barriers in klp_update_task_universe() and 
> > klp_set_universe_goal() look like complete crack, and their comments are 
> > seriously deficient.
> 
> These particular barriers seem correct to me; you basically need to make 
> sure that whenever a thread with TIF_KLP_NEED_UPDATE goes through 
> do_notify_resume(), it sees proper universe number to be converted to.

I'm not seeing how they're going to help with that.

The comment should describe the data race and how the barriers are
making it not happen.

putting wmb after a store and rmb before a read doesn't avoid the reader
seeing the old value in any universe I know of.

Barriers are about order, you need two consecutive stores for a wmb to
make sense, and two consecutive reads for an rmb, and if they're paired
the stores and reads need to be to the same addresses.

Without that they're pointless.

The comment doesn't describe which two variables are ordered how.