From: Waiman Long <longman@redhat.com>
To: Jan Stancek <jstancek@redhat.com>, linux-kernel@vger.kernel.org
Cc: dbueso@suse.de, will@kernel.org, peterz@infradead.org, mingo@redhat.com
Subject: Re: [PATCH v2] locking/rwsem: add acquire barrier to read_slowpath exit when queue is empty
Date: Wed, 17 Jul 2019 11:33:50 -0400 [thread overview]
Message-ID: <5313e3de-ca8d-3f7c-eff0-620803303a28@redhat.com> (raw)
In-Reply-To: <a524cf95ab0dbdd1eb65e9decb9283e73d416b1d.1563352912.git.jstancek@redhat.com>
On 7/17/19 8:02 AM, Jan Stancek wrote:
> LTP mtest06 has been observed to rarely hit "still mapped when deleted"
> and following BUG_ON on arm64:
> page:ffff7e02fa37e480 refcount:3 mapcount:1 mapping:ffff80be3d678ab0 index:0x0
> xfs_address_space_operations [xfs]
> flags: 0xbfffe000000037(locked|referenced|uptodate|lru|active)
> page dumped because: VM_BUG_ON_PAGE(page_mapped(page))
> ------------[ cut here ]------------
> kernel BUG at mm/filemap.c:171!
> Internal error: Oops - BUG: 0 [#1] SMP
> CPU: 220 PID: 154292 Comm: mmap1 Not tainted 5.2.0-0ecfebd.cki #1
> Hardware name: HPE Apollo 70 /C01_APACHE_MB , BIOS L50_5.13_1.10 05/17/2019
> pstate: 40400089 (nZcv daIf +PAN -UAO)
> pc : unaccount_page_cache_page+0x17c/0x1a0
> lr : unaccount_page_cache_page+0x17c/0x1a0
> Call trace:
> unaccount_page_cache_page+0x17c/0x1a0
> delete_from_page_cache_batch+0xa0/0x300
> truncate_inode_pages_range+0x1b8/0x640
> truncate_inode_pages_final+0x88/0xa8
> evict+0x1a0/0x1d8
> iput+0x150/0x240
> dentry_unlink_inode+0x120/0x130
> __dentry_kill+0xd8/0x1d0
> dentry_kill+0x88/0x248
> dput+0x168/0x1b8
> __fput+0xe8/0x208
> ____fput+0x20/0x30
> task_work_run+0xc0/0xf0
> do_notify_resume+0x2b0/0x328
> work_pending+0x8/0x10
>
> The extra mapcount originated from pagefault handler, which handled
> pagefault for vma that has already been detached. vma is detached
> under mmap_sem write lock by detach_vmas_to_be_unmapped(), which
> also invalidates vmacache.
>
> When pagefault handler (under mmap_sem read lock) called find_vma(),
> vmacache_valid() wrongly reported vmacache as valid.
>
> After rwsem down_read() returns via 'queue empty' path (as of v5.2),
> it does so without issuing read_acquire on sem->count:
> down_read
> __down_read
> rwsem_down_read_failed
> __rwsem_down_read_failed_common
> raw_spin_lock_irq(&sem->wait_lock);
> if (list_empty(&sem->wait_list)) {
> if (atomic_long_read(&sem->count) >= 0) {
> raw_spin_unlock_irq(&sem->wait_lock);
> return sem;
>
> Suspected problem here is that last *_acquire on down_read() side
> happens before write side issues *_release:
> 1. writer: has the lock
> 2. reader: down_read() issues *read_acquire on entry
> 3. writer: mm->vmacache_seqnum++; downgrades lock (*fetch_add_release)
> 4. reader: __rwsem_down_read_failed_common() finds it can take lock and returns
> 5. reader: observes stale mm->vmacache_seqnum
>
> I can reproduce the problem by running LTP mtest06 in a loop and building
> kernel (-j $NCPUS) in parallel. It does reproduce since v4.20 up to v5.2
> on arm64 HPE Apollo 70 (224 CPUs, 256GB RAM, 2 nodes). It triggers reliably
> within ~hour. Patched kernel ran fine for 10+ hours with clean dmesg.
> Tests were done against v5.2, since commit cf69482d62d9 ("locking/rwsem:
> Enable readers spinning on writer") makes it much harder to reproduce.
>
> v2: Move barrier after test (Waiman Long)
> Use smp_acquire__after_ctrl_dep() (Peter Zijlstra)
>
> Related: https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/mtest06/mmap1.c
> Related: commit dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
> Fixes: 4b486b535c33 ("locking/rwsem: Exit read lock slowpath if queue empty & no writer")
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: stable@vger.kernel.org # v4.20+
> Cc: Waiman Long <longman@redhat.com>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: Will Deacon <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> ---
> kernel/locking/rwsem.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c
> index 37524a47f002..5ac72b60608b 100644
> --- a/kernel/locking/rwsem.c
> +++ b/kernel/locking/rwsem.c
> @@ -1032,6 +1032,7 @@ static inline bool rwsem_reader_phase_trylock(struct rw_semaphore *sem,
> */
> if (adjustment && !(atomic_long_read(&sem->count) &
> (RWSEM_WRITER_MASK | RWSEM_FLAG_HANDOFF))) {
> + smp_acquire__after_ctrl_dep();
> raw_spin_unlock_irq(&sem->wait_lock);
> rwsem_set_reader_owned(sem);
> lockevent_inc(rwsem_rlock_fast);
The corresponding change for 5.2 or earlier kernels are:
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index fbe96341beee..2fbbb2d46396 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -246,6 +246,7 @@ __rwsem_down_read_failed_common(struct rw_semaphore
*sem, in
* been set in the count.
*/
if (atomic_long_read(&sem->count) >= 0) {
+ smp_acquire__after_ctrl_dep();
raw_spin_unlock_irq(&sem->wait_lock);
return sem;
}
-Longman
prev parent reply other threads:[~2019-07-17 15:33 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 16:04 [PATCH] locking/rwsem: use read_acquire in read_slowpath exit when queue is empty Jan Stancek
2019-07-16 16:53 ` Waiman Long
2019-07-16 18:34 ` Jan Stancek
2019-07-16 18:58 ` Peter Zijlstra
2019-07-16 19:09 ` Waiman Long
2019-07-17 12:02 ` [PATCH v2] locking/rwsem: add acquire barrier to " Jan Stancek
2019-07-17 13:13 ` Will Deacon
2019-07-17 14:19 ` Waiman Long
2019-07-17 19:22 ` Jan Stancek
2019-07-17 19:39 ` Waiman Long
2019-07-18 8:51 ` [PATCH v3] " Jan Stancek
2019-07-25 16:00 ` [tip:locking/core] locking/rwsem: Add missing ACQUIRE " tip-bot for Jan Stancek
2019-07-18 9:26 ` [PATCH v2] locking/rwsem: add acquire barrier " Will Deacon
2019-07-18 10:50 ` Jan Stancek
2019-07-18 11:04 ` Peter Zijlstra
2019-07-18 11:09 ` Peter Zijlstra
2019-07-18 11:36 ` Jan Stancek
2019-07-18 12:12 ` Paul E. McKenney
2019-07-18 10:58 ` Peter Zijlstra
2019-07-18 11:45 ` Will Deacon
2019-07-18 12:23 ` Peter Zijlstra
2019-07-17 15:33 ` Waiman Long [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5313e3de-ca8d-3f7c-eff0-620803303a28@redhat.com \
--to=longman@redhat.com \
--cc=dbueso@suse.de \
--cc=jstancek@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).