mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Luis Chamberlain <mcgrof@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>
Cc: <alison.schofield@intel.com>, <vishal.l.verma@intel.com>,
	<ira.weiny@intel.com>, <bwidawsk@kernel.org>, <dave@stgolabs.net>,
	<a.manzanares@samsung.com>, <linux-cxl@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [RFC] cxl_test: upgrade as a first class citizen selftests capable driver
Date: Mon, 19 Dec 2022 16:27:10 -0800	[thread overview]
Message-ID: <63a1015e6ec4a_1bf22944c@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <Y6DDPGnwszUAiNh2@bombadil.infradead.org>

Luis Chamberlain wrote:
> On Fri, Dec 16, 2022 at 08:55:19PM -0800, Dan Williams wrote:
> > In other words the suggestion that the current
> > organization ultimately leads to bit rot has not been substantiated in
> > practice.
> 
> On top of this patch I just added a custom debug patch to my tree which
> enables CXL_BUS and CXL_TEST by default when this is currently allowed
> and it got quite a bit of kernel build warnings. Although some of these
> are specific to my change, some of them do not seem to be related to
> that and likely could benefit from fixing:
> 
> https://gist.github.com/mcgrof/73dce72939590c6edc9413b0384ae4c2
> 
> And so although you may not see some build warnings so far, it does not
> negate my suggestion that having cxl_test as a proper upstream driver strategy
> gets you more build testing / coverage.

If autobuild coverage of test components is the main concern then
cxl_test can copy what nfit_test is doing with CONFIG_NVDIMM_TEST_BUILD.
No need for disruptive redesign of how this facility is integrated.

> > The proposed direction to move tests out of the ndctl.git repo into the
> > kernel solves the wrong problem.
> 
> That's not in any way what I suggested and is not the point to my patch.
> 
> The proposed patch does not suggest to ditch ndctl unit tests but to
> *enable* also sefltests to make use of cxl_test using the selftests
> framework, which is very different. It is not saying "abandon" ndctl
> unit tests, but rather, "also enable linux kernel selftests for CXL with
> cxl_test".

I think centralizing test scripts is a virtue, and right now the
momentum is with those located ndctl.git. This is why I jumped to the
conclusion that the long term direction would be to pick one location
for maintainer regression tests.

> But more importantly, it looks for the value of proper kernel
> integration and making use of kconfig for the actual dependencies
> and requirements. This is of high value.
> 
> In addition to this, one possible area I see of value with this change is the
> ability to also use the wrap feature later, even without cxl_test to enable
> error injection. What would this look like? You simply replace one built in
> routine as you do with another which has sprinkled in should_fail() calls,
> which otherwise would be an eyesore upstream. This shold also then not
> depend the rest of cxl_test stuff, but can make use of building
> alternative wrap routines which could be replacement for upstream ones.
> 
> Another benefit of this strategy is you can also test cxl_test *without*
> the need for for requiring modules, which some folks prefer for testing.
> At LSFMM this came up for instance and one of the biggest grudges with
> testing some frameworks was the dependency on modules.

I do think this is the void that QEMU CXL testing would attempt to fill.

  reply	other threads:[~2022-12-20  0:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-17  3:49 Luis Chamberlain
2022-12-17  4:55 ` Dan Williams
2022-12-18 16:08   ` Jonathan Cameron
2022-12-19 18:20     ` Luis Chamberlain
2022-12-20 15:46       ` Jonathan Cameron
2022-12-19 20:02   ` Luis Chamberlain
2022-12-20  0:27     ` Dan Williams [this message]
2022-12-20  2:44       ` Luis Chamberlain
2022-12-20 19:51         ` Dan Williams
2023-01-04 19:49           ` Luis Chamberlain

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=63a1015e6ec4a_1bf22944c@dwillia2-xfh.jf.intel.com.notmuch \
    --to=dan.j.williams@intel.com \
    --cc=a.manzanares@samsung.com \
    --cc=alison.schofield@intel.com \
    --cc=bwidawsk@kernel.org \
    --cc=dave@stgolabs.net \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --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

all inboxes | Powered by JetHome®