linux-kernel - Re: [RFC] cxl_test: upgrade as a first class citizen selftests capable driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y6DDPGnwszUAiNh2@bombadil.infradead.org>
Date:   Mon, 19 Dec 2022 12:02:04 -0800
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     alison.schofield@...el.com, vishal.l.verma@...el.com,
        ira.weiny@...el.com, bwidawsk@...nel.org, dave@...olabs.net,
        a.manzanares@...sung.com, linux-cxl@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC] cxl_test: upgrade as a first class citizen selftests
 capable driver

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.

> 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".

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.

So requiring modules is also limitting from test scrope perspective.

> So in terms of benefits of code being colocated, tests + libcxl + tools in the
> same repo is more impactful than tests + kernel in the same repo.

That usage does not negate any possible benefit of enabling selftests upstream
too.

> I know Jonathan has some latent ideas about building up a CXL regression
> suite on top of QEMU, but even there it's not clear that would benefit
> from being developed in linux.git given the tight coupling to QEMU.git.

Definitely not. Such tests should exist outside of the kernel tree.

  Luis

Content of type "application/pgp-keys" skipped