From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750793AbdH3EAn (ORCPT ); Wed, 30 Aug 2017 00:00:43 -0400 Received: from mail-pg0-f53.google.com ([74.125.83.53]:36476 "EHLO mail-pg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750723AbdH3EAl (ORCPT ); Wed, 30 Aug 2017 00:00:41 -0400 X-Google-Smtp-Source: ADKCNb6/YwZKrxTbjiFf+OhJjDPizPa1C7uOiolZOqFRE+Pv8PJjIvkvToXLzok4UrsfqWvyaS+UZw== Date: Wed, 30 Aug 2017 12:01:01 +0800 From: Boqun Feng To: Mathieu Desnoyers Cc: "Paul E . McKenney" , Peter Zijlstra , linux-kernel@vger.kernel.org, Andrew Hunter , Maged Michael , gromer@google.com, Avi Kivity , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Dave Watson , Andy Lutomirski , Will Deacon , Hans Boehm Subject: Re: [PATCH v2] membarrier: provide register sync core cmd Message-ID: <20170830040101.GR11771@tardis> References: <20170827205035.25620-1-mathieu.desnoyers@efficios.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MM5RgFPKyuP3gDcV" Content-Disposition: inline In-Reply-To: <20170827205035.25620-1-mathieu.desnoyers@efficios.com> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --MM5RgFPKyuP3gDcV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Mathieu, On Sun, Aug 27, 2017 at 01:50:35PM -0700, Mathieu Desnoyers wrote: > Add a new MEMBARRIER_CMD_REGISTER_SYNC_CORE command to the membarrier > system call. It allows processes to register their intent to have their > threads issue core serializing barriers in addition to memory barriers > whenever a membarrier command is performed. >=20 > It is relevant for reclaim of JIT code, which requires to issue core > serializing barriers on all threads running on behalf of a process > after ensuring the old code is not visible anymore, before re-using > memory for new code. >=20 > When a processes returns from a MEMBARRIER_CMD_REGISTER_SYNC_CORE > command, it is guaranteed that all following MEMBARRIER_CMD_SHARED and > MEMBARRIER_CMD_PRIVATE_EXPEDITED issue core serializing barriers, in > addition to the memory barriers, on all of its running threads. >=20 > * Scheduler Overhead Benchmarks >=20 > Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz > taskset 01 ./perf bench sched pipe -T > Linux v4.13-rc6 >=20 > Avg. usecs/op Std.Dev. usecs/op > Before this change: 2.75 0.12 > Non-registered processes: 2.73 0.08 > Registered processes: 3.07 0.02 >=20 > Changes since v1: > - Add missing MEMBARRIER_CMD_REGISTER_SYNC_CORE header documentation, > - Add benchmarks to commit message. >=20 > Signed-off-by: Mathieu Desnoyers > CC: Peter Zijlstra > CC: Paul E. McKenney > CC: Boqun Feng > CC: Andrew Hunter > CC: Maged Michael > CC: gromer@google.com > CC: Avi Kivity > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Michael Ellerman > CC: Dave Watson > CC: Andy Lutomirski > CC: Will Deacon > CC: Hans Boehm > --- > fs/exec.c | 1 + > include/linux/sched.h | 52 +++++++++++++++++++++++++++++++++++= ++++++ > include/uapi/linux/membarrier.h | 8 +++++++ > kernel/fork.c | 2 ++ > kernel/sched/core.c | 3 +++ > kernel/sched/membarrier.c | 37 ++++++++++++++++++++++++++--- > 6 files changed, 100 insertions(+), 3 deletions(-) >=20 > diff --git a/fs/exec.c b/fs/exec.c > index 62175cbcc801..a4ab3253bac7 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1794,6 +1794,7 @@ static int do_execveat_common(int fd, struct filena= me *filename, > /* execve succeeded */ > current->fs->in_exec =3D 0; > current->in_execve =3D 0; > + membarrier_execve(current); > acct_update_integrals(current); > task_numa_free(current); > free_bprm(bprm); > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 8337e2db0bb2..b1ecdc4e8b84 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1086,6 +1086,9 @@ struct task_struct { > /* Used by LSM modules for access restriction: */ > void *security; > #endif > +#ifdef CONFIG_MEMBARRIER > + int membarrier_sync_core; Why put this in task_struct rather than mm_struct? If the reclaim of JIT code is one of the target users, don't you want to make this as a attribute of an address space? Am I missing something subtle here? Regards, Boqun > +#endif > =20 > /* > * New fields for task_struct should be added above here, so that > @@ -1623,4 +1626,53 @@ extern long sched_getaffinity(pid_t pid, struct cp= umask *mask); > #define TASK_SIZE_OF(tsk) TASK_SIZE > #endif > =20 > +#ifdef CONFIG_MEMBARRIER > +static inline void membarrier_fork(struct task_struct *t, > + unsigned long clone_flags) > +{ > + /* > + * Coherence of membarrier_sync_core against thread fork is > + * protected by siglock. membarrier_fork is called with siglock > + * held. > + */ > + t->membarrier_sync_core =3D current->membarrier_sync_core; > +} > +static inline void membarrier_execve(struct task_struct *t) > +{ > + t->membarrier_sync_core =3D 0; > +} > +static inline void membarrier_sched_out(struct task_struct *t) > +{ > + /* > + * Core serialization is performed before the memory barrier > + * preceding the store to rq->curr. > + */ > + if (unlikely(READ_ONCE(t->membarrier_sync_core))) > + sync_core(); > +} > +static inline void membarrier_sched_in(struct task_struct *t) > +{ > + /* > + * Core serialization is performed after the memory barrier > + * following the store to rq->curr. > + */ > + if (unlikely(READ_ONCE(t->membarrier_sync_core))) > + sync_core(); > +} > +#else > +static inline void membarrier_fork(struct task_struct *t, > + unsigned long clone_flags) > +{ > +} > +static inline void membarrier_execve(struct task_struct *t) > +{ > +} > +static inline void membarrier_sched_out(struct task_struct *t) > +{ > +} > +static inline void membarrier_sched_in(struct task_struct *t) > +{ > +} > +#endif > + > #endif > diff --git a/include/uapi/linux/membarrier.h b/include/uapi/linux/membarr= ier.h > index 6d47b3249d8a..933c35ebcc10 100644 > --- a/include/uapi/linux/membarrier.h > +++ b/include/uapi/linux/membarrier.h > @@ -56,6 +56,13 @@ > * complete faster than the non-expedited ones, > * they never block, but have the downside of > * causing extra overhead. > + * @MEMBARRIER_CMD_REGISTER_SYNC_CORE: > + * Register the caller process so all of its > + * threads will issue core serialization > + * barriers in addition to memory barriers upon > + * SHARED and PRIVATE barriers targeting > + * its threads issued after this registration > + * returns. > * > * Command to be passed to the membarrier system call. The commands need= to > * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assign= ed to > @@ -67,6 +74,7 @@ enum membarrier_cmd { > /* reserved for MEMBARRIER_CMD_SHARED_EXPEDITED (1 << 1) */ > /* reserved for MEMBARRIER_CMD_PRIVATE (1 << 2) */ > MEMBARRIER_CMD_PRIVATE_EXPEDITED =3D (1 << 3), > + MEMBARRIER_CMD_REGISTER_SYNC_CORE =3D (1 << 4), > }; > =20 > #endif /* _UAPI_LINUX_MEMBARRIER_H */ > diff --git a/kernel/fork.c b/kernel/fork.c > index 17921b0390b4..713b3c932671 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1840,6 +1840,8 @@ static __latent_entropy struct task_struct *copy_pr= ocess( > */ > copy_seccomp(p); > =20 > + membarrier_fork(p, clone_flags); > + > /* > * Process group and session signals need to be delivered to just the > * parent before the fork or both the parent and the child after the > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 0e36d9960d91..3ca27d066950 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3292,6 +3292,8 @@ static void __sched notrace __schedule(bool preempt) > local_irq_disable(); > rcu_note_context_switch(preempt); > =20 > + membarrier_sched_out(prev); > + > /* > * Make sure that signal_pending_state()->signal_pending() below > * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) > @@ -3364,6 +3366,7 @@ static void __sched notrace __schedule(bool preempt) > =20 > /* Also unlocks the rq: */ > rq =3D context_switch(rq, prev, next, &rf); > + membarrier_sched_in(next); > } else { > rq->clock_update_flags &=3D ~(RQCF_ACT_SKIP|RQCF_REQ_SKIP); > rq_unlock_irq(rq, &rf); > diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c > index 7eec6914d2d2..f128caf64b49 100644 > --- a/kernel/sched/membarrier.c > +++ b/kernel/sched/membarrier.c > @@ -25,12 +25,16 @@ > * Bitmask made from a "or" of all commands within enum membarrier_cmd, > * except MEMBARRIER_CMD_QUERY. > */ > -#define MEMBARRIER_CMD_BITMASK \ > - (MEMBARRIER_CMD_SHARED | MEMBARRIER_CMD_PRIVATE_EXPEDITED) > +#define MEMBARRIER_CMD_BITMASK \ > + (MEMBARRIER_CMD_SHARED \ > + | MEMBARRIER_CMD_PRIVATE_EXPEDITED \ > + | MEMBARRIER_CMD_REGISTER_SYNC_CORE) > =20 > static void ipi_mb(void *info) > { > - smp_mb(); /* IPIs should be serializing but paranoid. */ > + /* IPIs should be serializing but paranoid. */ > + smp_mb(); > + sync_core(); > } > =20 > static void membarrier_private_expedited(void) > @@ -96,6 +100,30 @@ static void membarrier_private_expedited(void) > smp_mb(); /* exit from system call is not a mb */ > } > =20 > +static void membarrier_register_sync_core(void) > +{ > + struct task_struct *p =3D current, *t; > + > + if (get_nr_threads(p) =3D=3D 1) { > + p->membarrier_sync_core =3D 1; > + return; > + } > + > + /* > + * Coherence of membarrier_sync_core against thread fork is > + * protected by siglock. > + */ > + spin_lock(&p->sighand->siglock); > + for_each_thread(p, t) > + WRITE_ONCE(t->membarrier_sync_core, 1); > + spin_unlock(&p->sighand->siglock); > + /* > + * Ensure all future scheduler execution will observe the new > + * membarrier_sync_core state for this process. > + */ > + synchronize_sched(); > +} > + > /** > * sys_membarrier - issue memory barriers on a set of threads > * @cmd: Takes command values defined in enum membarrier_cmd. > @@ -146,6 +174,9 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) > case MEMBARRIER_CMD_PRIVATE_EXPEDITED: > membarrier_private_expedited(); > return 0; > + case MEMBARRIER_CMD_REGISTER_SYNC_CORE: > + membarrier_register_sync_core(); > + return 0; > default: > return -EINVAL; > } > --=20 > 2.11.0 >=20 --MM5RgFPKyuP3gDcV Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAlmmOHYACgkQSXnow7UH +rgU/Af7BuDF0M/rF4p7xMhLmFhGre0UBFHchgvswp4u7JW80XBzEzqXK0wWI0CN o9J8kn5vsbWifsGAPceHCOi1jrk/6pXIcGgdB6DSwnGotqJlDBc6nMVexonbwt5q tacujXpnMM0iw3A9wJqDXwE0kxXnyUN7vALWv6k9V/LamRYtLGdYvhRF+c+44AKF OmgVLXFy4vhPzf+lYFGdCeaIQtrlzkUiB+KqhFt6sUhI/MDGP7dujOlxRF6kZtd0 sf12kXai6RGd+XixdIijAOtrKELJSvnIDhcKIyYZc7JwSAN6oKwZb/amSuqeUGtQ kA21sSbA0bHdF6VoM+1PaNx7L6ExFA== =Hfs1 -----END PGP SIGNATURE----- --MM5RgFPKyuP3gDcV--