From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,URIBL_DBL_ABUSE_MALW autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DC9AC35247 for ; Tue, 4 Feb 2020 01:34:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0D4932084E for ; Tue, 4 Feb 2020 01:34:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="gbWCmFCA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D4932084E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AE3A16B0274; Mon, 3 Feb 2020 20:34:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A6C256B0275; Mon, 3 Feb 2020 20:34:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 90E306B0276; Mon, 3 Feb 2020 20:34:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0051.hostedemail.com [216.40.44.51]) by kanga.kvack.org (Postfix) with ESMTP id 76F8A6B0274 for ; Mon, 3 Feb 2020 20:34:44 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 172282480 for ; Tue, 4 Feb 2020 01:34:44 +0000 (UTC) X-FDA: 76450725288.23.steam56_c3072f1c1b4a X-HE-Tag: steam56_c3072f1c1b4a X-Filterd-Recvd-Size: 7081 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf29.hostedemail.com (Postfix) with ESMTP for ; Tue, 4 Feb 2020 01:34:43 +0000 (UTC) Received: from localhost.localdomain (c-73-231-172-41.hsd1.ca.comcast.net [73.231.172.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C318921775; Tue, 4 Feb 2020 01:34:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1580780083; bh=btTCX7DmSD0QE2pFv5a67bS1DzCh2k3oGxJ2003AGd4=; h=Date:From:To:Subject:In-Reply-To:From; b=gbWCmFCADGE7yVD3SV/ZbwCcF+YG+sbnMXcLuCQ5M2pdP1N2o+UplnG3z1y42tWOo bcOrb/9cZ3LpV/a9fkpmwSgO19fQ/lmFOa0HTENbxukKyTNm1+ew9ySgo36432QLqf iGmoUBitFxOo6eyKHaRvLLZJJmgQROhbf9d/Z/Uo= Date: Mon, 03 Feb 2020 17:34:42 -0800 From: Andrew Morton To: 1vier1@web.de, akpm@linux-foundation.org, dave@stgolabs.net, linux-mm@kvack.org, longman@redhat.com, manfred@colorfullife.com, mm-commits@vger.kernel.org, peterz@infradead.org, torvalds@linux-foundation.org, will.deacon@arm.com Subject: [patch 18/67] ipc/sem.c: document and update memory barriers Message-ID: <20200204013442.tPVtkj_dX%akpm@linux-foundation.org> In-Reply-To: <20200203173311.6269a8be06a05e5a4aa08a93@linux-foundation.org> User-Agent: s-nail v14.8.16 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: Manfred Spraul Subject: ipc/sem.c: document and update memory barriers Document and update the memory barriers in ipc/sem.c: - Add smp_store_release() to wake_up_sem_queue_prepare() and document why it is needed. - Read q->status using READ_ONCE+smp_acquire__after_ctrl_dep(). as the pair for the barrier inside wake_up_sem_queue_prepare(). - Add comments to all barriers, and mention the rules in the block regarding locking. - Switch to using wake_q_add_safe(). Link: http://lkml.kernel.org/r/20191020123305.14715-6-manfred@colorfullife.com Signed-off-by: Manfred Spraul Cc: Waiman Long Cc: Davidlohr Bueso Cc: <1vier1@web.de> Cc: Peter Zijlstra Cc: Will Deacon Signed-off-by: Andrew Morton --- ipc/sem.c | 66 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 25 deletions(-) --- a/ipc/sem.c~ipc-semc-document-and-update-memory-barriers +++ a/ipc/sem.c @@ -205,15 +205,38 @@ static int sysvipc_sem_proc_show(struct * * Memory ordering: * Most ordering is enforced by using spin_lock() and spin_unlock(). - * The special case is use_global_lock: + * + * Exceptions: + * 1) use_global_lock: (SEM_BARRIER_1) * Setting it from non-zero to 0 is a RELEASE, this is ensured by - * using smp_store_release(). + * using smp_store_release(): Immediately after setting it to 0, + * a simple op can start. * Testing if it is non-zero is an ACQUIRE, this is ensured by using * smp_load_acquire(). * Setting it from 0 to non-zero must be ordered with regards to * this smp_load_acquire(), this is guaranteed because the smp_load_acquire() * is inside a spin_lock() and after a write from 0 to non-zero a * spin_lock()+spin_unlock() is done. + * + * 2) queue.status: (SEM_BARRIER_2) + * Initialization is done while holding sem_lock(), so no further barrier is + * required. + * Setting it to a result code is a RELEASE, this is ensured by both a + * smp_store_release() (for case a) and while holding sem_lock() + * (for case b). + * The AQUIRE when reading the result code without holding sem_lock() is + * achieved by using READ_ONCE() + smp_acquire__after_ctrl_dep(). + * (case a above). + * Reading the result code while holding sem_lock() needs no further barriers, + * the locks inside sem_lock() enforce ordering (case b above) + * + * 3) current->state: + * current->state is set to TASK_INTERRUPTIBLE while holding sem_lock(). + * The wakeup is handled using the wake_q infrastructure. wake_q wakeups may + * happen immediately after calling wake_q_add. As wake_q_add_safe() is called + * when holding sem_lock(), no further barriers are required. + * + * See also ipc/mqueue.c for more details on the covered races. */ #define sc_semmsl sem_ctls[0] @@ -344,12 +367,8 @@ static void complexmode_tryleave(struct return; } if (sma->use_global_lock == 1) { - /* - * Immediately after setting use_global_lock to 0, - * a simple op can start. Thus: all memory writes - * performed by the current operation must be visible - * before we set use_global_lock to 0. - */ + + /* See SEM_BARRIER_1 for purpose/pairing */ smp_store_release(&sma->use_global_lock, 0); } else { sma->use_global_lock--; @@ -400,7 +419,7 @@ static inline int sem_lock(struct sem_ar */ spin_lock(&sem->lock); - /* pairs with smp_store_release() */ + /* see SEM_BARRIER_1 for purpose/pairing */ if (!smp_load_acquire(&sma->use_global_lock)) { /* fast path successful! */ return sops->sem_num; @@ -766,15 +785,12 @@ would_block: static inline void wake_up_sem_queue_prepare(struct sem_queue *q, int error, struct wake_q_head *wake_q) { - wake_q_add(wake_q, q->sleeper); - /* - * Rely on the above implicit barrier, such that we can - * ensure that we hold reference to the task before setting - * q->status. Otherwise we could race with do_exit if the - * task is awoken by an external event before calling - * wake_up_process(). - */ - WRITE_ONCE(q->status, error); + get_task_struct(q->sleeper); + + /* see SEM_BARRIER_2 for purpuse/pairing */ + smp_store_release(&q->status, error); + + wake_q_add_safe(wake_q, q->sleeper); } static void unlink_queue(struct sem_array *sma, struct sem_queue *q) @@ -2148,9 +2164,11 @@ static long do_semtimedop(int semid, str } do { + /* memory ordering ensured by the lock in sem_lock() */ WRITE_ONCE(queue.status, -EINTR); queue.sleeper = current; + /* memory ordering is ensured by the lock in sem_lock() */ __set_current_state(TASK_INTERRUPTIBLE); sem_unlock(sma, locknum); rcu_read_unlock(); @@ -2173,13 +2191,8 @@ static long do_semtimedop(int semid, str */ error = READ_ONCE(queue.status); if (error != -EINTR) { - /* - * User space could assume that semop() is a memory - * barrier: Without the mb(), the cpu could - * speculatively read in userspace stale data that was - * overwritten by the previous owner of the semaphore. - */ - smp_mb(); + /* see SEM_BARRIER_2 for purpose/pairing */ + smp_acquire__after_ctrl_dep(); goto out_free; } @@ -2189,6 +2202,9 @@ static long do_semtimedop(int semid, str if (!ipc_valid_object(&sma->sem_perm)) goto out_unlock_free; + /* + * No necessity for any barrier: We are protect by sem_lock() + */ error = READ_ONCE(queue.status); /* _