Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 Jan 2023 12:12:48 +0100
From:   Alexander Gordeev <agordeev@linux.ibm.com>
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Thomas Richter <tmricht@linux.ibm.com>,
        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, borntraeger@linux.ibm.com,
        svens@linux.ibm.com, Herbert Xu <herbert@gondor.apana.org.au>,
        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 <arnd@arndb.de>,
        penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com,
        Andrew Morton <akpm@linux-foundation.org>, 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: <Y7/rMJ75lW7z9PAb@li-4a3a4a4c-28e5-11b2-a85c-a8d192c6f089.ibm.com>
References: <20221219153525.632521981@infradead.org>
 <20221219154119.352918965@infradead.org>
 <Y70SWXHDmOc3RhMd@osiris>
 <Y70it59wuvsnKJK1@hirez.programming.kicks-ass.net>
 <Y71QJBhNTIatvxUT@osiris>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y71QJBhNTIatvxUT@osiris>
Precedence: bulk

On Tue, Jan 10, 2023 at 12:46:44PM +0100, Heiko Carstens wrote:
> > > +	/* 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;

I guess, it would also make sense to place write to overflow 
after the while loop. So the output variable left intact in
case the function bailed out. Not sure if it should be part
of this patch though.

> > > +		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.