From: Heiko Carstens <hca@linux.ibm.com>
To: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>,
Janosch Frank <frankja@linux.ibm.com>,
Claudio Imbrenda <imbrenda@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
David Hildenbrand <david@redhat.com>,
Jonathan Corbet <corbet@lwn.net>,
kvm@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-s390@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
Shuah Khan <shuah@kernel.org>,
Sven Schnelle <svens@linux.ibm.com>
Subject: Re: [PATCH 5/5] s390/uaccess: add cmpxchg_user_key()
Date: Thu, 10 Nov 2022 12:32:06 +0100 [thread overview]
Message-ID: <Y2zhNhFjIJPKJao8@osiris> (raw)
In-Reply-To: <31fcea80f54c5f53012489f29ebedba775672919.camel@linux.ibm.com>
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.
next prev parent reply other threads:[~2022-11-10 11:32 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-12 20:56 [PATCH v2 0/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg Janis Schoetterl-Glausch
2022-10-12 20:56 ` [PATCH v2 1/9] s390/uaccess: Add storage key checked cmpxchg access to user space Janis Schoetterl-Glausch
2022-10-20 11:18 ` Heiko Carstens
2022-10-20 13:40 ` Nico Boehr
2022-10-21 19:22 ` Heiko Carstens
2022-11-02 14:12 ` Heiko Carstens
2022-11-02 14:16 ` [PATCH 1/5] s390/cmpxchg: use symbolic names for inline assembly operands Heiko Carstens
2022-11-02 14:17 ` [PATCH 2/5] s390/cmpxchg: make variables local to each case label Heiko Carstens
2022-11-02 14:18 ` [PATCH 3/5] s390/cmpxchg: remove digits from input constraints Heiko Carstens
2022-11-02 14:18 ` [PATCH 4/5] s390/extable: add EX_TABLE_UA_LOAD_REGPAIR() macro Heiko Carstens
2022-11-02 14:19 ` [PATCH 5/5] s390/uaccess: add cmpxchg_user_key() Heiko Carstens
2022-11-09 15:46 ` Janis Schoetterl-Glausch
2022-11-09 22:24 ` Heiko Carstens
2022-11-10 11:01 ` Janis Schoetterl-Glausch
2022-11-10 11:32 ` Heiko Carstens [this message]
2022-11-13 18:20 ` Heiko Carstens
2022-11-16 19:36 ` [PATCH v2 1/9] s390/uaccess: Add storage key checked cmpxchg access to user space Janis Schoetterl-Glausch
2022-11-17 8:42 ` Nico Boehr
2022-10-12 20:56 ` [PATCH v2 2/9] KVM: s390: Extend MEM_OP ioctl by storage key checked cmpxchg Janis Schoetterl-Glausch
2022-10-13 13:48 ` kernel test robot
2022-10-12 20:56 ` [PATCH v2 3/9] Documentation: KVM: s390: Describe KVM_S390_MEMOP_F_CMPXCHG Janis Schoetterl-Glausch
2022-10-12 20:56 ` [PATCH v2 4/9] KVM: s390: selftest: memop: Pass mop_desc via pointer Janis Schoetterl-Glausch
2022-10-12 20:56 ` [PATCH v2 5/9] KVM: s390: selftest: memop: Replace macros by functions Janis Schoetterl-Glausch
2022-10-12 20:56 ` [PATCH v2 6/9] KVM: s390: selftest: memop: Add cmpxchg tests Janis Schoetterl-Glausch
2022-10-12 20:56 ` [PATCH v2 7/9] KVM: s390: selftest: memop: Add bad address test Janis Schoetterl-Glausch
2022-10-13 13:17 ` Nico Boehr
2022-10-12 20:56 ` [PATCH v2 8/9] KVM: s390: selftest: memop: Fix typo Janis Schoetterl-Glausch
2022-10-13 13:16 ` Nico Boehr
2022-10-12 20:56 ` [PATCH v2 9/9] KVM: s390: selftest: memop: Fix wrong address being used in test Janis Schoetterl-Glausch
2022-10-13 13:15 ` Nico Boehr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y2zhNhFjIJPKJao8@osiris \
--to=hca@linux.ibm.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=corbet@lwn.net \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=scgl@linux.ibm.com \
--cc=shuah@kernel.org \
--cc=svens@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).