From: dave.hansen at intel.com (Dave Hansen)
Subject: [PATCH v14 08/22] selftests/vm: fix the wrong assert in pkey_disable_set()
Date: Wed, 18 Jul 2018 08:36:50 -0700 [thread overview]
Message-ID: <b8ace761-2140-afce-a1d4-fc2a27c8fd9e@intel.com> (raw)
In-Reply-To: <1531835365-32387-9-git-send-email-linuxram@us.ibm.com>
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 ..."
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: dave.hansen@intel.com (Dave Hansen)
Subject: [PATCH v14 08/22] selftests/vm: fix the wrong assert in pkey_disable_set()
Date: Wed, 18 Jul 2018 08:36:50 -0700 [thread overview]
Message-ID: <b8ace761-2140-afce-a1d4-fc2a27c8fd9e@intel.com> (raw)
Message-ID: <20180718153650.PVQdR21LOwp2IyZ3e7OEAjbk1xWVOeYV7q1sBq7cj-A@z> (raw)
In-Reply-To: <1531835365-32387-9-git-send-email-linuxram@us.ibm.com>
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 ..."
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave.hansen@intel.com>
To: Ram Pai <linuxram@us.ibm.com>,
shuahkh@osg.samsung.com, linux-kselftest@vger.kernel.org
Cc: linux-arch@vger.kernel.org, fweimer@redhat.com, x86@kernel.org,
mhocko@kernel.org, linux-mm@kvack.org, mingo@redhat.com,
aneesh.kumar@linux.vnet.ibm.com, bauerman@linux.vnet.ibm.com,
msuchanek@suse.de, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v14 08/22] selftests/vm: fix the wrong assert in pkey_disable_set()
Date: Wed, 18 Jul 2018 08:36:50 -0700 [thread overview]
Message-ID: <b8ace761-2140-afce-a1d4-fc2a27c8fd9e@intel.com> (raw)
In-Reply-To: <1531835365-32387-9-git-send-email-linuxram@us.ibm.com>
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 ..."
WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave.hansen@intel.com>
To: Ram Pai <linuxram@us.ibm.com>,
shuahkh@osg.samsung.com, linux-kselftest@vger.kernel.org
Cc: mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org,
linux-mm@kvack.org, x86@kernel.org, linux-arch@vger.kernel.org,
mingo@redhat.com, mhocko@kernel.org, bauerman@linux.vnet.ibm.com,
fweimer@redhat.com, msuchanek@suse.de,
aneesh.kumar@linux.vnet.ibm.com
Subject: Re: [PATCH v14 08/22] selftests/vm: fix the wrong assert in pkey_disable_set()
Date: Wed, 18 Jul 2018 08:36:50 -0700 [thread overview]
Message-ID: <b8ace761-2140-afce-a1d4-fc2a27c8fd9e@intel.com> (raw)
Message-ID: <20180718153650.-cM_fOkln23YL3k3YF2jote8JOSU4UzBWOjqLqWU7Gk@z> (raw)
In-Reply-To: <1531835365-32387-9-git-send-email-linuxram@us.ibm.com>
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 ..."
next prev parent reply other threads:[~2018-07-18 15:36 UTC|newest]
Thread overview: 156+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-17 13:49 [PATCH v14 00/22] selftests, powerpc, x86 : Memory Protection Keys linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 01/22] selftests/x86: Move protecton key selftest to arch neutral directory linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:25 ` dave.hansen
2018-07-18 15:25 ` Dave Hansen
2018-07-18 15:25 ` Dave Hansen
2018-07-18 15:25 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 02/22] selftests/vm: rename all references to pkru to a generic name linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 03/22] selftests/vm: move generic definitions to header file linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:26 ` dave.hansen
2018-07-18 15:26 ` Dave Hansen
2018-07-18 15:26 ` Dave Hansen
2018-07-18 15:26 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 04/22] selftests/vm: move arch-specific definitions to arch-specific header linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:27 ` dave.hansen
2018-07-18 15:27 ` Dave Hansen
2018-07-18 15:27 ` Dave Hansen
2018-07-18 15:27 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 05/22] selftests/vm: Make gcc check arguments of sigsafe_printf() linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 06/22] selftests/vm: typecast the pkey register linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:32 ` dave.hansen
2018-07-18 15:32 ` Dave Hansen
2018-07-18 15:32 ` Dave Hansen
2018-07-18 15:32 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 07/22] selftests/vm: generic function to handle shadow key register linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:34 ` dave.hansen
2018-07-18 15:34 ` Dave Hansen
2018-07-18 15:34 ` Dave Hansen
2018-07-18 15:34 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 08/22] selftests/vm: fix the wrong assert in pkey_disable_set() linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:36 ` dave.hansen [this message]
2018-07-18 15:36 ` Dave Hansen
2018-07-18 15:36 ` Dave Hansen
2018-07-18 15:36 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 09/22] selftests/vm: fixed bugs in pkey_disable_clear() linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:43 ` dave.hansen
2018-07-18 15:43 ` Dave Hansen
2018-07-18 15:43 ` Dave Hansen
2018-07-18 15:43 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 10/22] selftests/vm: fix alloc_random_pkey() to make it really random linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:45 ` dave.hansen
2018-07-18 15:45 ` Dave Hansen
2018-07-18 15:45 ` Dave Hansen
2018-07-18 15:45 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 11/22] selftests/vm: introduce two arch independent abstraction linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 15:52 ` dave.hansen
2018-07-18 15:52 ` Dave Hansen
2018-07-18 15:52 ` Dave Hansen
2018-07-18 15:52 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 12/22] selftests/vm: pkey register should match shadow pkey linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:00 ` dave.hansen
2018-07-18 16:00 ` Dave Hansen
2018-07-18 16:00 ` Dave Hansen
2018-07-18 16:00 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 13/22] selftests/vm: generic cleanup linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:06 ` dave.hansen
2018-07-18 16:06 ` Dave Hansen
2018-07-18 16:06 ` Dave Hansen
2018-07-18 16:06 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 14/22] selftests/vm: Introduce generic abstractions linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:38 ` dave.hansen
2018-07-18 16:38 ` Dave Hansen
2018-07-18 16:38 ` Dave Hansen
2018-07-18 16:38 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 15/22] selftests/vm: powerpc implementation to check support for pkey linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:42 ` dave.hansen
2018-07-18 16:42 ` Dave Hansen
2018-07-18 16:42 ` Dave Hansen
2018-07-18 16:42 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 16/22] selftests/vm: fix an assertion in test_pkey_alloc_exhaust() linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:52 ` dave.hansen
2018-07-18 16:52 ` Dave Hansen
2018-07-18 16:52 ` Dave Hansen
2018-07-18 16:52 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 17/22] selftests/vm: associate key on a mapped page and detect access violation linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 18/22] selftests/vm: associate key on a mapped page and detect write violation linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 19/22] selftests/vm: detect write violation on a mapped access-denied-key page linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 20/22] selftests/vm: testcases must restore pkey-permissions linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 16:56 ` dave.hansen
2018-07-18 16:56 ` Dave Hansen
2018-07-18 16:56 ` Dave Hansen
2018-07-18 16:56 ` Dave Hansen
2018-07-17 13:49 ` [PATCH v14 21/22] selftests/vm: sub-page allocator linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` [PATCH v14 22/22] selftests/vm: test correct behavior of pkey-0 linuxram
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-17 13:49 ` Ram Pai
2018-07-18 17:03 ` dave.hansen
2018-07-18 17:03 ` Dave Hansen
2018-07-18 17:03 ` Dave Hansen
2018-07-18 17:03 ` Dave Hansen
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=b8ace761-2140-afce-a1d4-fc2a27c8fd9e@intel.com \
--to=unknown@example.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.