From: Alison Schofield <alison.schofield@intel.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>
Cc: "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [ndctl PATCH v2 5/5] cxl/test: add cxl-poison.sh unit test
Date: Fri, 17 Nov 2023 08:52:37 -0800 [thread overview]
Message-ID: <ZVeaVTCKqjQ9u8nw@aschofie-mobl2> (raw)
In-Reply-To: <6bbc779c174fc03f01382666b4b6b44fdfc0ef8c.camel@intel.com>
On Wed, Nov 15, 2023 at 02:13:48AM -0800, Vishal Verma wrote:
> On Sun, 2023-10-01 at 15:31 -0700, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > Exercise cxl list, libcxl, and driver pieces of the get poison list
> > pathway. Inject and clear poison using debugfs and use cxl-cli to
> > read the poison list by memdev and by region.
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
snip
> > +cxl list
>
> "$CXL" list
>
> Also should reset rc from 77 so that it doesn't show as skipped on a
> real failure.
Done.
>
snip
> > +setup_x2_region()
> > +{
> > + # Find an x2 decoder
> > + decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
>
> I suspect this comes from another test, but test/common defines a
> $cxl_test_bus that can be used here.
Done.
>
snip
> > +find_media_errors()
> > +{
> > + nr=$(echo $json | jq -r ".nr_poison_records")
>
> No need for echo and pipe -
>
> nr="$(jq -r ".nr_poison_records" <<< "$json")"
Done
>
> Also, this currently assumes that a global '$json' will be available
> and up to date. In this test the way it is called, this will always be
> true, but it would be cleaner to actually pass $json to
> find_media_errors() each time, and in here, do something like
>
> local json="$1"
>
Done
> > + if [[ $nr -ne $NR_ERRS ]]; then
>
> If using the bash variant, [[ ]], this should be
>
> if [[ $nr != $NR_ERRS ]]; then
>
Done
> > + echo "$mem: $NR_ERRS poison records expected, $nr found"
> > + err "$LINENO"
> > + fi
> > +}
> > +
snip
> > +find_media_errors
>
> For all of the above debugfs writes -
>
> mem1 is hard-coded - is this supposed to be "$mem1" from when
> setup_x2_region() was done (similar to how the region stuff is done
> below)?
It was intentionally hardcoded based on what I expect in the
cxl-test topology.
Changed it in v3 to look up a memdev.
>
> > +
> > +# Poison by region: inject, list, clear, list.
> > +setup_x2_region
> > +create_region
> > +echo 0x40000000 > /sys/kernel/debug/cxl/"$mem0"/inject_poison
> > +echo 0x40000000 > /sys/kernel/debug/cxl/"$mem1"/inject_poison
> > +NR_ERRS=2
> > +json=$("$CXL" list -r "$region" --poison | jq -r '.[].poison')
> > +find_media_errors
> > +echo 0x40000000 > /sys/kernel/debug/cxl/"$mem0"/clear_poison
> > +echo 0x40000000 > /sys/kernel/debug/cxl/"$mem1"/clear_poison
>
> It might be nice to create a couple of helpers -
>
> inject_poison_sysfs() {
> memdev="$1"
> addr="$2
> ...
> }
>
> And similarly
>
> clear_poison_sysfs()...
>
Done
Thanks for the review Vishal, especially the bash & jq wisdom!
> >
prev parent reply other threads:[~2023-11-17 16:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-01 22:31 [ndctl PATCH v2 0/3] Support poison list retrieval alison.schofield
2023-10-01 22:31 ` [ndctl PATCH v2 1/5] libcxl: add interfaces for GET_POISON_LIST mailbox commands alison.schofield
2023-11-15 10:08 ` Verma, Vishal L
2023-11-17 16:21 ` Alison Schofield
2023-10-01 22:31 ` [ndctl PATCH v2 2/5] cxl: add an optional pid check to event parsing alison.schofield
2023-10-01 22:31 ` [ndctl PATCH v2 3/5] cxl/list: collect and parse the poison list records alison.schofield
2023-11-15 10:09 ` Verma, Vishal L
2023-11-17 16:44 ` Alison Schofield
2023-10-01 22:31 ` [ndctl PATCH v2 4/5] cxl/list: add --poison option to cxl list alison.schofield
2023-10-01 22:31 ` [ndctl PATCH v2 5/5] cxl/test: add cxl-poison.sh unit test alison.schofield
2023-11-15 10:13 ` Verma, Vishal L
2023-11-17 16:52 ` Alison Schofield [this message]
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=ZVeaVTCKqjQ9u8nw@aschofie-mobl2 \
--to=alison.schofield@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=vishal.l.verma@intel.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 read-only IMAP folder(s) and NNTP newsgroup(s).