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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F83CC54EBC for ; Tue, 10 Jan 2023 11:47:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 836D48E0002; Tue, 10 Jan 2023 06:47:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7BFCA8E0001; Tue, 10 Jan 2023 06:47:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 688298E0002; Tue, 10 Jan 2023 06:47:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 5A3908E0001 for ; Tue, 10 Jan 2023 06:47:24 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 304A3408E0 for ; Tue, 10 Jan 2023 11:47:24 +0000 (UTC) X-FDA: 80338714008.08.9D93948 Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by imf19.hostedemail.com (Postfix) with ESMTP id D64FE1A0004 for ; Tue, 10 Jan 2023 11:47:21 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=ibm.com header.s=pp1 header.b=PnKNZxhj; dmarc=pass (policy=none) header.from=ibm.com; spf=pass (imf19.hostedemail.com: domain of hca@linux.ibm.com designates 148.163.158.5 as permitted sender) smtp.mailfrom=hca@linux.ibm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673351242; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=qn2LabxLnZEZFumVZ5Olz/OuzFaySPNNAkYo8mYfUpw=; b=otmTzf3HPqk/AYxuJAzGN9pbG83vdh8c6NtAH6ApXY9vmIFYorbyZ+OkLdP7pq1f35t6F4 y4UeXqtGZBhddFCKrlt2EY6fT0EOuwTYWVUXNHp26ftDBxmVBgT/LxlWsyvJUse3xcZMfW r5hr1/GF8wlZtHMqUec4j6+imhKhaUY= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=ibm.com header.s=pp1 header.b=PnKNZxhj; dmarc=pass (policy=none) header.from=ibm.com; spf=pass (imf19.hostedemail.com: domain of hca@linux.ibm.com designates 148.163.158.5 as permitted sender) smtp.mailfrom=hca@linux.ibm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673351242; a=rsa-sha256; cv=none; b=sbhqdo8stWgBgVFjavvWmXPMmDEIO1M7UHi0hXBOHZYVrdYIpCya1PG1xlNfEN0gIghNxg J+Ed0OeJ0D7uKAIrXhFN9qYkGct2LARkw1Pg9htlSRCNUZsTusG6DdyDH3HShIo3YGMmdU Fzh7aOyOqo+nHZIFJLnWWqntP3Lg8Jw= Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 30A9pBo5007510; Tue, 10 Jan 2023 11:46:56 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=pp1; bh=qn2LabxLnZEZFumVZ5Olz/OuzFaySPNNAkYo8mYfUpw=; b=PnKNZxhj6QPF0ilZTvhqmWw+my6urQsEbvko3vN5eyn/jt3mkpMhmz8CydyvT6vf59ZV TKtOtCGXEmsMWgqOpGs4ZXt4Y3SxGiv+kPS094QW+PK6E+u+qeIOe4Vphnb5vUPoLlsQ WPokeXfqXwZ3HTPDNTXqn0atT+4Rf+efA63+fA+0mcGJF+LSxw0opdTXcnpRMGD9+0cb k9wMZLaw4lHCxrJge1FY1dHkkKXu/xBUgNWhFKec1kOmPReKhPqcpns8F+AoTL5oRTM3 LJvvfxeG2xwiMpDIz8Dmjk6Jn6tw/wsNbiVVIr67VyGVz0l2LuBcK4gj7jkd8SA3UpMW 7A== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3n15rk2gya-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 10 Jan 2023 11:46:56 +0000 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 30AB8oju011043; Tue, 10 Jan 2023 11:46:55 GMT Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3n15rk2gxc-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 10 Jan 2023 11:46:54 +0000 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 30A87hem024723; Tue, 10 Jan 2023 11:46:52 GMT Received: from smtprelay06.fra02v.mail.ibm.com ([9.218.2.230]) by ppma03ams.nl.ibm.com (PPS) with ESMTPS id 3my0c6mrqe-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 10 Jan 2023 11:46:52 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay06.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 30ABkmUh17891888 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 10 Jan 2023 11:46:48 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5CBAB20043; Tue, 10 Jan 2023 11:46:48 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6E1C220040; Tue, 10 Jan 2023 11:46:46 +0000 (GMT) Received: from osiris (unknown [9.152.212.250]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTPS; Tue, 10 Jan 2023 11:46:46 +0000 (GMT) Date: Tue, 10 Jan 2023 12:46:44 +0100 From: Heiko Carstens To: Peter Zijlstra Cc: Thomas Richter , torvalds@linux-foundation.org, corbet@lwn.net, will@kernel.org, boqun.feng@gmail.com, mark.rutland@arm.com, catalin.marinas@arm.com, dennis@kernel.org, tj@kernel.org, cl@linux.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, Herbert Xu , davem@davemloft.net, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, joro@8bytes.org, suravee.suthikulpanit@amd.com, robin.murphy@arm.com, dwmw2@infradead.org, baolu.lu@linux.intel.com, Arnd Bergmann , penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, Andrew Morton , vbabka@suse.cz, roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-s390@vger.kernel.org, linux-crypto@vger.kernel.org, iommu@lists.linux.dev, linux-arch@vger.kernel.org Subject: Re: [RFC][PATCH 08/12] s390: Replace cmpxchg_double() with cmpxchg128() Message-ID: References: <20221219153525.632521981@infradead.org> <20221219154119.352918965@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: pKIWmY9vx2HMOPSHFVupmbYl3bdIjBZF X-Proofpoint-GUID: UQEdaMsBDjpJCHJbGYQKtf47xWETq6SH X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2023-01-10_03,2023-01-10_03,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxscore=0 lowpriorityscore=0 malwarescore=0 phishscore=0 suspectscore=0 mlxlogscore=942 priorityscore=1501 clxscore=1015 impostorscore=0 spamscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2301100070 X-Rspamd-Queue-Id: D64FE1A0004 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: uu9z8ray88igjs8anh8wfpxire1ijycf X-HE-Tag: 1673351241-837734 X-HE-Meta: U2FsdGVkX19REJjdMOzuDo/h9UtkRz0RMsHZ4VLuOHEE04rR0bz78OVWIRIvhNC/nETdiZUguwPaC1l++U4gons2r58flF4alcZ5XX/MprVhS44orOzxZdieZbsggkJkEKIzaEkIOQ3nq7ccWvqaX2SR9JwCj3HScHtGgRhulv7BkDPFOCEOe/c0uxHBDRW8wt2XvthZt4P8QQdTX0y0LS06gfxOXA/wJax1qVXOlqH+OM1Yh4ldpqo4Q7CaHKGiOPpp0nT+SPevaPj2Vm3Ysh/HcJKvYBu8jZ7PxJRfryMCA3En6QkV2T0SJEcKx/HXX4bVgbDsZwa+mKuBts7keNhQS6pJKDSmkpIQ9d/SI7zMCqGf6yVhn/ebXiJHG7Ym/RM5gp3rkHKMnVrp6SQEvoyLlWNbErSkCpXVoHocJiKiOkkwXFKAZ6Dmk+G3q4XF6mxSfQ5UGCTrGbwZMGPkBXR3NSe0P6KH9PvCUCt0X+/lI803Lwlp3MYz2yStveqjnwaa482RoD3PSVa6qZCKgGy2ef1eXFoJVoXCgfI8lADN2pySg7nro/1MGv8nTQvGeVgEK5x5Zda3OoYWjYk3gZ97277MDXQuzP+m2v1k4dXvNcsZ8ik5Zku6NQjQRliJ2G50abk48ULpmD98ody2203Kb0J11b5RFj4j5n/nk5Q46xzPI5+YgP7nloLf0d+KGWMJMgmmlQsLXB/Vu9SkDZXf3gVN4Xm0opdzIT28avm8ERVwOaAaL43LYo/e9kWB2BxpzUkf6u9MMMAl7kFwvImol5u8xhF99P/Em+Q2XYEtoAjDF1cIKrGiXpOSt/bhWWWIbu/ooXBL/RPs+Cv1CCohdc/VxF5qpf3BIiuIGIJ85bAZOLYLr07EU6LeHOil+fs3LOtKC60zq39zedLKN2XVIH+CS6pBKlRdRCQwSJo34kiqOh54/LVn+G4BudPeDbx2iZH/eEA9hVLAEOk lSw== 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: On Tue, Jan 10, 2023 at 09:32:55AM +0100, Peter Zijlstra wrote: > On Tue, Jan 10, 2023 at 08:23:05AM +0100, Heiko Carstens wrote: > > So, Alexander Gordeev reported that this code was already prior to your > > changes potentially broken with respect to missing READ_ONCE() within the > > cmpxchg_double() loops. > > Unless there's an early exit, that shouldn't matter. If you managed to > read garbage the cmpxchg itself will simply fail and the loop retries. > > > @@ -1294,12 +1306,16 @@ static void hw_perf_event_update(struct perf_event *event, int flush_all) > > num_sdb++; > > > > /* Reset trailer (using compare-double-and-swap) */ > > + /* READ_ONCE() 16 byte header */ > > + prev.val = __cdsg(&te->header.val, 0, 0); > > do { > > + old.val = prev.val; > > + new.val = prev.val; > > + new.f = 0; > > + new.a = 1; > > + new.overflow = 0; > > + prev.val = __cdsg(&te->header.val, old.val, new.val); > > + } while (prev.val != old.val); > > So this, and ... > this case are just silly and expensive. If that initial read is split > and manages to read gibberish the cmpxchg will fail and we retry anyway. While I do agree that there is no need to necessarily read the whole 16 bytes atomically in advance here, there is still the problem about the missing initial READ_ONCE() in the original code. As I tried to outline here: For example: /* Reset trailer (using compare-double-and-swap) */ do { te_flags = te->flags & ~SDB_TE_BUFFER_FULL_MASK; te_flags |= SDB_TE_ALERT_REQ_MASK; } while (!cmpxchg_double(&te->flags, &te->overflow, te->flags, te->overflow, te_flags, 0ULL)); The compiler could generate code where te->flags used within the cmpxchg_double() call may be refetched from memory and which is not necessarily identical to the previous read version which was used to generate te_flags. Which in turn means that an incorrect update could happen. Is there anything that prevents te->flags from being read several times? > > + /* READ_ONCE() 16 byte header */ > > + prev.val = __cdsg(&te->header.val, 0, 0); > > do { > > + old.val = prev.val; > > + new.val = prev.val; > > + *overflow = old.overflow; > > + if (old.f) { > > /* > > * SDB is already set by hardware. > > * Abort and try to set somewhere > > @@ -1490,10 +1509,10 @@ static bool aux_set_alert(struct aux_buffer *aux, unsigned long alert_index, > > */ > > return false; > > } > > + new.a = 1; > > + new.overflow = 0; > > + prev.val = __cdsg(&te->header.val, old.val, new.val); > > + } while (prev.val != old.val); > > And while this case has an early exit, it only cares about a single bit > (although you made it a full word) and so also shouldn't care. If > aux_reset_buffer() returns false, @overflow isn't consumed. Yes, except that it is anything but obvious that @overflow isn't consumed. > So I really don't see the point of this patch. As stated above: READ_ONCE() is missing. And while at it I wanted to have a consistent complete previous value - also considering that cdsg is not very expensive. And while it also reuse the returned values from cdsg, instead of throwing them away and reading from memory again in a splitted and potentially inconsistent way.