On Thu, Nov 10, 2022 at 12:01:23PM +0100, Janis Schoetterl-Glausch wrote: > On Wed, 2022-11-09 at 23:24 +0100, Heiko Carstens wrote: > > On Wed, Nov 09, 2022 at 04:46:29PM +0100, Janis Schoetterl-Glausch wrote: > > > Are you only entertaining cosmetic changes to cmpxchg.h? > > > > I fail to parse what you are trying to say. Please elaborate. > > > > > The loop condition being imprecise seems non-ideal. > > > > What exactly is imprecise? > > The loop retries the CS if bits outside the target byte changed instead > of retrying until the target byte differs from the old value. > So if you attempt to exchange (prev_left_0 old_byte prev_right_0) and > that fails because the word at the address is (prev_left_1 x prev_right_1) > where both x != old_byte and one of the prev_*_1 values differs from the respective > prev_*_0 value, the CS is retried. If there were a native 1 byte compare and swap, > the exchange would just fail here. Instead the loop retries the CS until the margin > values are stable and it can infer from that that the CS failed because of the target value. > (Assuming that doesn't change to the old_byte value.) > > It's not a problem, but it struck me as non-ideal, which is why for v2 I inverted the mask > after using it to punch the hole for the old/new values. > Then you can use it to test if bits inside the target byte differ. > > That's why I asked about cmpxchg.h. If you don't want non-cosmetic changes to the existing > cmpxchg function and consistency of the new key checked function, then obviously the loop > condition needs to be the same. Such a change is fine of course, even though compare-and-swap for one and two byte patterns don't really matter. I would appreciate if you could send one or two patches on-top of this series which adds the improved logic to (now) both variants. And, since the question will come up anyway: as soon as we agreed on a complete patch series, I think we should go for a features branch on s390's kernel.org tree which would contain the first five patches sent by me plus potential addon patches provided by you. This tree can then be pulled in by the kvms390 tree where your kvm specific patches can then be applied on top.
