| Message ID | 1531835365-32387-9-git-send-email-linuxram@us.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/17/2018 06:49 AM, Ram Pai wrote: > If the flag is 0, no bits will be set. Hence we cant expect > the resulting bitmap to have a higher value than what it > was earlier. ... > --- a/tools/testing/selftests/vm/protection_keys.c > +++ b/tools/testing/selftests/vm/protection_keys.c > @@ -415,7 +415,7 @@ void pkey_disable_set(int pkey, int flags) > dprintf1("%s(%d) pkey_reg: 0x"PKEY_REG_FMT"\n", > __func__, pkey, read_pkey_reg()); > if (flags) > - pkey_assert(read_pkey_reg() > orig_pkey_reg); > + pkey_assert(read_pkey_reg() >= orig_pkey_reg); > dprintf1("END<---%s(%d, 0x%x)\n", __func__, > pkey, flags); > } I know these are just selftests, but this change makes zero sense without the context from how powerpc works. It's also totally non-obvious from the patch itself what is going on, even though I specifically called this out in a previous review. Please add a comment here that either specifically calls out powerpc or talks about "an architecture that does this ..."
diff --git a/tools/testing/selftests/vm/protection_keys.c b/tools/testing/selftests/vm/protection_keys.c index fb7dd32..2dd94c3 100644 --- a/tools/testing/selftests/vm/protection_keys.c +++ b/tools/testing/selftests/vm/protection_keys.c @@ -415,7 +415,7 @@ void pkey_disable_set(int pkey, int flags) dprintf1("%s(%d) pkey_reg: 0x"PKEY_REG_FMT"\n", __func__, pkey, read_pkey_reg()); if (flags) - pkey_assert(read_pkey_reg() > orig_pkey_reg); + pkey_assert(read_pkey_reg() >= orig_pkey_reg); dprintf1("END<---%s(%d, 0x%x)\n", __func__, pkey, flags); }
If the flag is 0, no bits will be set. Hence we cant expect the resulting bitmap to have a higher value than what it was earlier. cc: Dave Hansen <dave.hansen@intel.com> cc: Florian Weimer <fweimer@redhat.com> Signed-off-by: Ram Pai <linuxram@us.ibm.com> --- tools/testing/selftests/vm/protection_keys.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)