From: Rafael David Tinoco <rafael.tinoco@linaro.org>
To: Shuah Khan <shuah@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
gregkh@linuxfoundation.org, mathieu.desnoyers@efficios.com,
mingo@kernel.org, peterz@infradead.org, tglx@linutronix.de
Subject: Re: [PATCH] selftests: membarrier: fix test by checking supported commands
Date: Wed, 8 Aug 2018 11:09:00 -0300 [thread overview]
Message-ID: <20180808140900.mureac7fptuoaba6@mobile.celeiro.br> (raw)
In-Reply-To: <20180731031537.bs64cdcxfqgmjwp4@mobile.celeiro.br>
On Tue, Jul 31, 2018 at 12:15:37AM -0300, Rafael David Tinoco wrote:
> Hello Shuah,
>
> On Mon, Jul 30, 2018 at 05:32:30PM -0600, Shuah Khan wrote:
> > Hi Rafael,
> >
> > On 07/30/2018 10:05 AM, Rafael David Tinoco wrote:
> > > Makes membarrier_test compatible with older kernels (LTS) by checking if
> > > the membarrier features exist before running the tests.
> > >
> > > Link: https://bugs.linaro.org/show_bug.cgi?id=3771
> > > Signed-off-by: Rafael David Tinoco <rafael.tinoco@linaro.org>
> > > Cc: <stable@vger.kernel.org> #v4.17
> > > ---
> > > .../selftests/membarrier/membarrier_test.c | 69 +++++++++++--------
> > > 1 file changed, 41 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c
> > > index 6793f8ecc8e7..b96caa096e2f 100644
> > > --- a/tools/testing/selftests/membarrier/membarrier_test.c
> > > +++ b/tools/testing/selftests/membarrier/membarrier_test.c
> > > @@ -225,7 +225,14 @@ static int test_membarrier_global_expedited_success(void)
> > >
> > > static int test_membarrier(void)
> > > {
> > > - int status;
> > > + int supported, status;
> > > +
> > > + supported = sys_membarrier(MEMBARRIER_CMD_QUERY, 0);
> > > + if (supported < 0) {
> > > + ksft_test_result_fail(
> > > + "sys_membarrier() failed to query supported cmds\n");
> > > + return supported;
> > > + }
> > >
> >
> > ksft_exit_skip() is the right interface to use here. If feature isn't supported,
> > it should exit skip as opposed fail.
> >
>
> Not sure this is the case here. This part was just a positional change.
>
> This check is extending an existing logic (for MEMBARRIER_CMD_PRIVATE_
> EXPEDITED_SYNC_CORE tests). Calling membarrier with MEMBARRIER_CMD_QUERY
> will return us MEMBARRIER_CMD_BITMASK, telling us which features are
> enabled for the running kernel (thus which tests can be executed).
>
> The query command was added in v4.3 and should (could ?) be considered a
> fundament for a working test by now, I suppose, no ?
>
> It is used to decide which further tests to run. Not receiving anything
> back from this call would mean something is broken (since at least
> MEMBARRIER_CMD_GLOBAL should have always existed as a membarrier
> feature/command).
>
> I think your concern is addressed in the beginning of the test.
> test_membarrier_query() tests for ENOSYS and calls ksft_exit_skip() if
> CONFIG_MEMBARRIER is disabled.
>
> This part is not about checking if the test can run, but which one can.
> What do you think ? Tks for reviewing!
Shuah,
Never mind, I'll remove the 2nd MEMBARRIER_CMD_QUERY call, and cache the
first call results into a global status. This way, the function
test_membarrier_query() will test for availability, and initial issues
(like not having MEMBARRIER_CMD_GLOBAL), and skip or return error
approprietly like you said. No need to call it twice, just use cached
status. Tks for the review.
I'll send a v2.
Thank you
next prev parent reply other threads:[~2018-08-08 14:09 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-30 16:05 [PATCH] selftests: membarrier: fix test by checking supported commands Rafael David Tinoco
2018-07-30 16:13 ` Mathieu Desnoyers
2018-07-30 23:32 ` Shuah Khan
2018-07-31 3:15 ` Rafael David Tinoco
2018-08-08 14:09 ` Rafael David Tinoco [this message]
2018-08-09 20:21 ` [PATCH v2] " Rafael David Tinoco
2018-08-27 22:52 ` Shuah Khan
2018-09-03 2:12 ` [PATCH v3] membarrier_test: work in progress Rafael David Tinoco
2018-09-21 22:48 ` Shuah Khan
2018-09-03 19:04 ` [PATCH v4] selftests: membarrier: reorganized test for LTS supportability Rafael David Tinoco
2018-09-03 19:11 ` Rafael David Tinoco
2018-09-21 22:53 ` Shuah Khan
2018-11-09 15:49 ` [PATCH v5] selftests: membarrier: re-organize test Rafael David Tinoco
2018-11-18 20:44 ` Rafael David Tinoco
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=20180808140900.mureac7fptuoaba6@mobile.celeiro.br \
--to=rafael.tinoco@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
/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).