From: "Cheatham, Benjamin" <benjamin.cheatham@amd.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Cc: "Schofield, Alison" <alison.schofield@intel.com>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"Jiang, Dave" <dave.jiang@intel.com>
Subject: Re: [PATCH 7/7] Documentation: Add docs for inject/clear-error commands
Date: Fri, 30 Jan 2026 15:45:09 -0600 [thread overview]
Message-ID: <ff553d4a-a1cd-4d23-b0a2-baf4d7aa72a6@amd.com> (raw)
In-Reply-To: <34210a8339035800430a9d4084154e17c285ee86.camel@intel.com>
On 1/30/2026 2:58 PM, Verma, Vishal L wrote:
> On Fri, 2026-01-30 at 13:59 -0600, Cheatham, Benjamin wrote:
>>>
>>> It feels to me like the two injection 'modes' should really be two
>>> separate commands, especially since they act on different classes of
>>> targets.
>>>
>>> So essentially, split both the injection and clear commands into:
>>>
>>> inject-protocol-error
>>> inject-media-error
>>> clear-protocol-error
>>> clear-media-error.
>>
>> This seems reasonable but I should clarify it would only be 3 commands,
>> clear-protocol-error wouldn't be a thing since there's only an injection
>> action for protocol errors.
>
> Ah I see, makes sense on 3 commands. I assume the clear command would
> still be clear-media-error for symmetry.
>
>>
>> Should I keep this all in one file or split into two separate files
>> on protocol/media error lines? Could also do inject/clear files if that
>> seems more logical.
>
> For the code - a single inject.c file probably seems fine. There's
> precedent of implementing multiple related commands in one file, and it
> makes sense to me here.
Ok, sounds good.
>
>>>
>>> That way the target operands for them are well defined - i.e. port
>>> objects for protocol errors and memdevs for media errors.
>>>
>>>
>>> Another thing - and I'm not too attached to either way for this -
>>>
>>> The -t 'long-string' feels a bit awkward. Could it be split into
>>> something like:
>>>
>>> --target={mem,cache} --type={correctable,uncorrectable,fatal}
>>>
>>> And then 'compose' the actual thing being injected from those options?
>>> Or is that unnecessary gymnastics?
>>>
>>
>> No, I like that idea. I do think the argument names could be better though.
>> What about:
>>
>> # inject-protocol-error <port> --protocol={mem,cache} --severity={correctable,uncorrectable,fatal}
>>
>> with the short flags for --protocol and --severity being -p and -s, respectively?
>
> Yes, I like those better!
>
>>
>> For inject/clear-media-error it could stay as-is, i.e.:
>>
>> # inject-media-error <memdev> -t={poison} -a=<device physical address>
>>
>> or I could update it to be something like:
>>
>> # inject-media-error <memdev> --poison -a=<device physical address>
>
> Either of those seems reasonable to me. What's the possibility of a
> /lot/ of types getting added? Probably small? If so, maybe --poison is
> cleanest, no string parsing needed.
I'm not sure, but I doubt it's many. I'll look into it and if it's less than ~5 or
so I'll change it to --poison.
Thanks,
Ben
next prev parent reply other threads:[~2026-01-30 21:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-22 20:37 [ndctl PATCH v7 0/7] Add error injection support Ben Cheatham
2026-01-22 20:37 ` [PATCH 1/7] libcxl: Add debugfs path to CXL context Ben Cheatham
2026-01-22 20:37 ` [PATCH 2/7] libcxl: Add CXL protocol errors Ben Cheatham
2026-01-22 20:37 ` [PATCH 3/7] libcxl: Add poison injection support Ben Cheatham
2026-01-22 20:37 ` [PATCH 4/7] cxl: Add inject-error command Ben Cheatham
2026-01-22 20:37 ` [PATCH 5/7] cxl: Add clear-error command Ben Cheatham
2026-01-22 20:37 ` [PATCH 6/7] cxl/list: Add injectable errors in output Ben Cheatham
2026-01-22 20:37 ` [PATCH 7/7] Documentation: Add docs for inject/clear-error commands Ben Cheatham
2026-01-29 19:45 ` Verma, Vishal L
2026-01-30 19:59 ` Cheatham, Benjamin
2026-01-30 20:58 ` Verma, Vishal L
2026-01-30 21:45 ` Cheatham, Benjamin [this message]
2026-02-03 1:31 ` Alison Schofield
-- strict thread matches above, loose matches on Subject: below --
2026-01-09 16:07 [ndctl PATCH v6 0/7] Add error injection support Ben Cheatham
2026-01-09 16:07 ` [PATCH 7/7] Documentation: Add docs for inject/clear-error commands Ben Cheatham
2026-01-09 22:25 ` Dave Jiang
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=ff553d4a-a1cd-4d23-b0a2-baf4d7aa72a6@amd.com \
--to=benjamin.cheatham@amd.com \
--cc=alison.schofield@intel.com \
--cc=dave.jiang@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