Received: by 2002:a25:ad19:0:0:0:0:0 with SMTP id y25csp1253475ybi; Tue, 16 Jul 2019 12:00:28 -0700 (PDT) X-Google-Smtp-Source: APXvYqxb9bEBnfYr+G1EoOpd12JBi5s2RqWRWZoWU+qwmey/c9MtJ2dAXgGEPE5WbHv0bnHhE7CQ X-Received: by 2002:a17:902:2862:: with SMTP id e89mr38004926plb.258.1563303628444; Tue, 16 Jul 2019 12:00:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1563303628; cv=none; d=google.com; s=arc-20160816; b=wu7Qh7lSwnP6g6X290zLiQbbAptGr++WxxrXBeWAZ2N2JuXYGyNMd+Rs4/91VEuyGa +ArsjowtP1K/iZzO9QKq6I5O+lxoNxnwG/Pyo/FCgvU5qciF9bnZCpotvp7T6s4rFoLH UTC6yzgK2bHl8p1u78oYZ1a+T5NXIEqPwoDN1ACLTj1UVAtO/SEfFwfuHVMtJlZOpUqc 9ObOjYzA8KiXJnDudenvKRT+QwcYl7w1F1EEWXS3sYfIU8GllTgJaKetlX1aQFb/UTjU YXMDFyaFPeLEOP1hOIt0okG7djVzf2skAkSW+p16fKu8+fRCRHua7QsaWnyKSQqEcDcM yLXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=NQzPm2qSJHyNNPNoCIrDmmIjlWz77+rLZs+LoDSCCnc=; b=m8hXUDkSw9w3nGo9cg6ozBbxlXyQKcTzX3Jt6kzQo9xsyzKvLvrCZmHIwxxqBOYPWb LIsZEO6vW/1y4EPyOpltwXgiYPbGr/QJ7VCx9alJozN92UTSTJiMpcAKJMZ3mRXH8hvL sYbtH6v7b+Rfb4bFTyMo1465g/M0Tvvsr8gfsnM2vw6X2/9KRGQzvmP7ML3yMasdb3tB Jd/5A8KglnwSZ64gqJOb2f3rDlfw5annye1uuXuKfR9gJdMAfa+Ni79D0g/KMoASM8kq npi7QlhhkkyDM24E9CAVlKMQizLKit8Xp8vddVrKutaLIWnDd489tl1EWSedd+xaqxBP dQWg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@infradead.org header.s=merlin.20170209 header.b=mCAefUio; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id m7si20570859pls.110.2019.07.16.12.00.11; Tue, 16 Jul 2019 12:00:28 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@infradead.org header.s=merlin.20170209 header.b=mCAefUio; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388358AbfGPS6Q (ORCPT + 99 others); Tue, 16 Jul 2019 14:58:16 -0400 Received: from merlin.infradead.org ([205.233.59.134]:59766 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728137AbfGPS6Q (ORCPT ); Tue, 16 Jul 2019 14:58:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=merlin.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=NQzPm2qSJHyNNPNoCIrDmmIjlWz77+rLZs+LoDSCCnc=; b=mCAefUio78c06/zAfjCgyR/l1 US4MU1Rihn1Wf7CcksM3U2gEr9nyYf9bQ+2J2KEVqOQ8H8JagVJxlOoTIUgifAtPBiIgKr/QCL7td Yo8Mfogwya+jaUhJpuVcG2kudW6OQGxAXjJCXH7yy/MQv3fFKq1scWavZRygdyVs+CP/8m0OHSOSp Heev3pTb+bPb/pDzRfP3i+V5L50n5HZhJTvOxi0WxiOcblN//8+9Om5JaX4/QXzD6TFc6jDVDEz7/ oDVPmYvo9c8D8faZsBOZgM9wFOpkuIf6dvJQTQxnjqkByy4O8XNn0P4Yqvzop/M17vxidI8EQSSjT P26KH8Hag==; Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=hirez.programming.kicks-ass.net) by merlin.infradead.org with esmtpsa (Exim 4.92 #3 (Red Hat Linux)) id 1hnSeR-0002qo-9I; Tue, 16 Jul 2019 18:58:11 +0000 Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id 0A12220B15D60; Tue, 16 Jul 2019 20:58:08 +0200 (CEST) Date: Tue, 16 Jul 2019 20:58:08 +0200 From: Peter Zijlstra To: Waiman Long Cc: Jan Stancek , linux-kernel@vger.kernel.org, dbueso@suse.de, will@kernel.org, mingo@redhat.com Subject: Re: [PATCH] locking/rwsem: use read_acquire in read_slowpath exit when queue is empty Message-ID: <20190716185807.GJ3402@hirez.programming.kicks-ass.net> References: <4ef66a01-7937-1eb7-c58b-0992a0142c92@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4ef66a01-7937-1eb7-c58b-0992a0142c92@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 16, 2019 at 12:53:14PM -0400, Waiman Long wrote: > On 7/16/19 12:04 PM, Jan Stancek wrote: > > 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 5+ 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. > > Fixes: 4b486b535c33 ("locking/rwsem: Exit read lock slowpath if queue empty & no writer") > > Signed-off-by: Jan Stancek > > Cc: Waiman Long > > Cc: Davidlohr Bueso > > Cc: Will Deacon > > Cc: Peter Zijlstra > > Cc: Ingo Molnar > > --- > > kernel/locking/rwsem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c > > index 37524a47f002..757b198d7a5b 100644 > > --- a/kernel/locking/rwsem.c > > +++ b/kernel/locking/rwsem.c > > @@ -1030,7 +1030,7 @@ static inline bool rwsem_reader_phase_trylock(struct rw_semaphore *sem, > > * exit the slowpath and return immediately as its > > * RWSEM_READER_BIAS has already been set in the count. > > */ > > - if (adjustment && !(atomic_long_read(&sem->count) & > > + if (adjustment && !(atomic_long_read_acquire(&sem->count) & > > (RWSEM_WRITER_MASK | RWSEM_FLAG_HANDOFF))) { > > raw_spin_unlock_irq(&sem->wait_lock); > > rwsem_set_reader_owned(sem); > > The chance of taking this path is not that high. So instead of > increasing the cost of the test by adding an acquire barrier, how about > just adding smp_mb__after_spinlock() before spin_unlock_irq(). This > should have the same effect of making sure that no stale data will be > used in the read-lock critical section. That's actually more expensive on something like ARM64 I expect. The far cheaper alternative is smp_acquire__after_ctrl_dep(), however in general Will seems to prefer using load-acquire over separate barriers, and for x86 it doesn't matter anyway. For PowerPC these two are a wash, both end up with LWSYNC (over SYNC for your alternative).