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 99297C46467 for ; Tue, 10 Jan 2023 08:33:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 324358E0005; Tue, 10 Jan 2023 03:33:40 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2D4D68E0001; Tue, 10 Jan 2023 03:33:40 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 19CED8E0005; Tue, 10 Jan 2023 03:33:40 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 0CEA58E0001 for ; Tue, 10 Jan 2023 03:33:40 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D8FC01C89E6 for ; Tue, 10 Jan 2023 08:33:39 +0000 (UTC) X-FDA: 80338225758.03.8FFE976 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) by imf04.hostedemail.com (Postfix) with ESMTP id C090A4000F for ; Tue, 10 Jan 2023 08:33:36 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=KEFepgJr; spf=none (imf04.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673339617; a=rsa-sha256; cv=none; b=A4NgQ1e4aYpQJedgQZ+TmM5bejcFoOrUmcCOOCpcN4MuDOFMAbpPrd0d5rS4X0so2nZhKH dJR1xenhN1Jz3yhRwDOrwcrIyDOAbcWAH9MVbcPaxUmtoJJ7TQEGslc7Q842tJz8LgnpHv TyvRgDSV4bmOOCPotsElEOUzHIGdAZA= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=KEFepgJr; spf=none (imf04.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673339617; 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=t0g8sAMw0YuigClEe8J6Of8uYBawzqc1qw15eQtTooo=; b=n2eknCB24ZsSYYULN8/kaCuBApp9MnHFoc1wIHgBrS4++7Aq7EItEncsgZrknpoGD+uk00 KDPSMgOeam6hkpwy8DNmNpUwtAXhKMh5XqOGI3rvAWQovWQ0uzew2MZEeFVRrA3p3XKnYN HnZRN0Y0UIxltSS2BkDf5x+wfxgHUKs= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; 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; bh=t0g8sAMw0YuigClEe8J6Of8uYBawzqc1qw15eQtTooo=; b=KEFepgJrWEeL9uBmLyGXAalt+U Jm4LcNJKx5mAdHe57CNNrg4oj1j9hd1AqdvItVQRtm35W5v6TQ/B49MYBUcKX83cd2sxykixLdens 8jhJ8pzVNn91Rh2XPC1hSMP2uABFLZcrCe9GEh3q+8P0+2R2OqKSgyLcJbPq5hPwGINuqzl71a4mI X49DYcRt1ylSGtFdZpTLwoGxLb+KGoauNXiPa7vOZzBtgrkEDDjGRG1Vi2GA2BML1dLqBevRADk+9 MlCuhRDzSLaHRh2zyvNm/Ijuu/qQxDjQLZ7ToLhyEqv2J3dVAwzQy5ULTGGQgSERvvbnfS0KVpfj9 a0WPVTmA==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.96 #2 (Red Hat Linux)) id 1pFA3w-0033Qg-1y; Tue, 10 Jan 2023 08:32:53 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 4BE59300033; Tue, 10 Jan 2023 09:32:55 +0100 (CET) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id BF2782C4AA552; Tue, 10 Jan 2023 09:32:55 +0100 (CET) Date: Tue, 10 Jan 2023 09:32:55 +0100 From: Peter Zijlstra To: Heiko Carstens 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-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: C090A4000F X-Stat-Signature: fcot6td5twn4dxez6138zgq15yumgfyx X-HE-Tag: 1673339616-762226 X-HE-Meta: U2FsdGVkX1+rCp7VTC97k0cmxZT6qv9iL3QyjT293WceNla4Y3Is9MWE62LZBWF7yoF3/oeJBeWrFcIGRJJaxShsadrAO5ax2FeWRBIdIXCbHxfbvpovI7/Lrt7o1t/Q2MMd9qb3fOIOv6ak5lRDcK+WAvHa5HyQGPxeMH8OFh9wM5szASB+wJ9tV9W3jfhqWcqg6BgaxxVgbbmnU3sIV4DACwTUJPvSkByTtXrnjtAHZWzFQ0FPqR0zyWx4ueS9rheTjigh18OSIp7vvPyN/ctglXq5vOI8vR2+Pi3l0k6C9s6WJxU1oWp2Vjs8iLFQZaIgkMEGLeWerFKK58rJn+NB0cEaXPgEjsQHZxDXLB22G9saf5Gox3766aHBpyR13vHoCa9rrV3VsCJ8XVnvXMe9ebKi9dp1DvkGdHGTUzSUoSaDKGBN9i+u8gg/LGdafIzgj7HnyaWzydUpqZ61ec82KJMZHLCkp5ytISWidxH/peYEIQFHXxPzvvXClOi4zpBchchC+7Bhp40vs9kAyWHqDlKuIshH64hpVKwMTnxZ7YMCQ8WniW7/f8oT8VD2S+X4Bmyttm/DWMufIjH5PF3hPT5kPM7dZ7BKzSH+UVpcQcsQyUbpu/56SZJi6kNObAE3++U1HcZQ7cAu57ZZPvKDrlK8IWzfpxWKdkiLyAAQJadLJ/U85+oxIEb/RKs3A1DTokmy+cWiXyX57V+AXb6/m4CDHc5J839+C/oCgmmn4CwoZXX6I3T21GkZAw85k6sKzmwZq4GUOCVix+gxuqD6OoxT0/4Kzg7UMfsFJYZqDsj8D+OyuNTwmsROMydOX9WjDV6No2t13OnyOerVdlcH3vd5HJAypQR49XbljJFa9nXFp9FQhGcRIvIz+VM5443duBwsz0nZFG5uX5I1bmIzE+va8JLmk/RUVQ+byfOvmc9d2G7NDBrZavEOr5ajxTySnppu6cchsZ5HQrZ lNaMzWFJ n0QkU 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 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 > + /* READ_ONCE() 16 byte header */ > + prev.val = __cdsg(&te->header.val, 0, 0); > do { > + old.val = prev.val; > + new.val = prev.val; > + orig_overflow = old.overflow; > + new.f = 0; > + new.overflow = 0; > if (idx == aux->alert_mark) > + new.a = 1; > else > + new.a = 0; > + prev.val = __cdsg(&te->header.val, old.val, new.val); > + } while (prev.val != old.val); 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. > + /* 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. So I really don't see the point of this patch.