[RFC,06/19] ktf: A simple debugfs interface to test results

Commit Message

Knut Omang Aug. 13, 2019, 6:09 a.m. UTC

From: Alan Maguire <[e-mail]>

While test results is available via netlink from user space, sometimes
it may be useful to be able to access the results from the kernel as well,
for instance due to a crash. Make that possible via debugfs.

ktf_debugfs.h:   Support for creating a debugfs representation of test

Signed-off-by: Alan Maguire <[e-mail]>
Signed-off-by: Knut Omang <[e-mail]>
---
 tools/testing/selftests/ktf/kernel/ktf_debugfs.c | 356 ++++++++++++++++-
 tools/testing/selftests/ktf/kernel/ktf_debugfs.h |  34 ++-
 2 files changed, 390 insertions(+)
 create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.c
 create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.h

Comments

Greg KH Aug. 13, 2019, 8:21 a.m. UTC | #1

On Tue, Aug 13, 2019 at 08:09:21AM +0200, Knut Omang wrote:
> From: Alan Maguire <[e-mail]>
> 
> While test results is available via netlink from user space, sometimes
> it may be useful to be able to access the results from the kernel as well,
> for instance due to a crash. Make that possible via debugfs.
> 
> ktf_debugfs.h:   Support for creating a debugfs representation of test
> 
> Signed-off-by: Alan Maguire <[e-mail]>
> Signed-off-by: Knut Omang <[e-mail]>
> ---
>  tools/testing/selftests/ktf/kernel/ktf_debugfs.c | 356 ++++++++++++++++-
>  tools/testing/selftests/ktf/kernel/ktf_debugfs.h |  34 ++-
>  2 files changed, 390 insertions(+)
>  create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.c
>  create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.h
> 
> diff --git a/tools/testing/selftests/ktf/kernel/ktf_debugfs.c b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
> new file mode 100644
> index 0000000..a20fbd2
> --- /dev/null
> +++ b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
> @@ -0,0 +1,356 @@
> +/*
> + * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
> + *    Author: Alan Maguire <[e-mail]>
> + *
> + * SPDX-License-Identifier: GPL-2.0

Has to be the first line of the file, did you run this through
checkpatch?

> +static int ktf_run_test_open(struct inode *inode, struct file *file)
> +{
> +	struct ktf_test *t;
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -EIO;

This is an anti-pattern, and one guaranteed to not work properly.  NEVER
do this.

> +
> +	t = (struct ktf_test *)inode->i_private;
> +
> +	return single_open(file, ktf_debugfs_run, t);
> +}
> +
> +static int ktf_debugfs_release(struct inode *inode, struct file *file)
> +{
> +	module_put(THIS_MODULE);

Same here, not ok.


> +	return single_release(inode, file);
> +}
> +
> +static const struct file_operations ktf_run_test_fops = {
> +	.open = ktf_run_test_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = ktf_debugfs_release,
> +};
> +
> +static int ktf_results_test_open(struct inode *inode, struct file *file)
> +{
> +	struct ktf_test *t;
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -EIO;

Nope!

And why -EIO?  That is not an io issue.

> +void ktf_debugfs_create_test(struct ktf_test *t)
> +{
> +	struct ktf_case *testset = ktf_case_find(t->tclass);
> +
> +	if (!testset)
> +		return;
> +
> +	memset(&t->debugfs, 0, sizeof(t->debugfs));
> +
> +	t->debugfs.debugfs_results_test =
> +		debugfs_create_file(t->name, S_IFREG | 0444,
> +				    testset->debugfs.debugfs_results_test,
> +				 t, &ktf_results_test_fops);
> +
> +	if (t->debugfs.debugfs_results_test) {

How can that variable ever be NULL (hint, it can not.)

> +		t->debugfs.debugfs_run_test =
> +			debugfs_create_file(t->name, S_IFREG | 0444,
> +					    testset->debugfs.debugfs_run_test,
> +				 t, &ktf_run_test_fops);
> +		if (!t->debugfs.debugfs_run_test) {
> +			_ktf_debugfs_destroy_test(t);
> +		} else {
> +			/* Take reference for test for debugfs */
> +			ktf_test_get(t);
> +		}
> +	}

Never test the result of any debugfs call, you do not need to.  Just
call it and move on, your code flow should NEVER be different with, or
without, a successful debugfs call.


> +static int ktf_run_testset_open(struct inode *inode, struct file *file)
> +{
> +	struct ktf_case *testset;
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -EIO;

Again no.  I hate to know what code you copied this all from, as that
code is very wrong.  Do you have a pointer to that code anywhere so we
can fix that up?

> +
> +	testset = (struct ktf_case *)inode->i_private;
> +
> +	return single_open(file, ktf_debugfs_run_all, testset);
> +}
> +
> +static const struct file_operations ktf_run_testset_fops = {
> +	.open = ktf_run_testset_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = ktf_debugfs_release,

If you really care about module references you should be setting the
owner of the module here.

> +};
> +
> +static void _ktf_debugfs_destroy_testset(struct ktf_case *testset)
> +{
> +	debugfs_remove(testset->debugfs.debugfs_run_testset);
> +	debugfs_remove(testset->debugfs.debugfs_run_test);
> +	debugfs_remove(testset->debugfs.debugfs_results_testset);
> +	debugfs_remove(testset->debugfs.debugfs_results_test);

Why not just recursivly remove the directory?  That way you do not have
to keep track of any individual files.


> +}
> +
> +void ktf_debugfs_create_testset(struct ktf_case *testset)
> +{
> +	char tests_subdir[KTF_DEBUGFS_NAMESZ];
> +	const char *name = ktf_case_name(testset);
> +
> +	memset(&testset->debugfs, 0, sizeof(testset->debugfs));
> +
> +	/* First add /sys/kernel/debug/ktf/[results|run]/<testset> */
> +	testset->debugfs.debugfs_results_testset =
> +		debugfs_create_file(name, S_IFREG | 0444,
> +				    ktf_debugfs_resultsdir,
> +				 testset, &ktf_results_testset_fops);
> +	if (!testset->debugfs.debugfs_results_testset)
> +		goto err;

Again, can never happen, and again, do not do different things depending
on the result of a debugfs call.

> +
> +	testset->debugfs.debugfs_run_testset =
> +		debugfs_create_file(name, S_IFREG | 0444,
> +				    ktf_debugfs_rundir,
> +				    testset, &ktf_run_testset_fops);
> +	if (!testset->debugfs.debugfs_run_testset)
> +		goto err;

Again, nope.

> +
> +	/* Now add parent directories for individual test result/run tests
> +	 * which live in
> +	 * /sys/kernel/debug/ktf/[results|run]/<testset>-tests/<testname>
> +	 */
> +	(void)snprintf(tests_subdir, sizeof(tests_subdir), "%s%s",
> +			name, KTF_DEBUGFS_TESTS_SUFFIX);

why (void)?


> +
> +	testset->debugfs.debugfs_results_test =
> +		debugfs_create_dir(tests_subdir, ktf_debugfs_resultsdir);
> +	if (!testset->debugfs.debugfs_results_test)
> +		goto err;

nope :)

> +
> +	testset->debugfs.debugfs_run_test =
> +		debugfs_create_dir(tests_subdir, ktf_debugfs_rundir);
> +	if (!testset->debugfs.debugfs_run_test)
> +		goto err;

Nope :)

> +
> +	/* Take reference count for testset.  One will do as we will always
> +	 * free testset debugfs resources together.
> +	 */
> +	ktf_case_get(testset);
> +	return;
> +err:
> +	_ktf_debugfs_destroy_testset(testset);
> +}
> +
> +void ktf_debugfs_destroy_testset(struct ktf_case *testset)
> +{
> +	tlog(T_DEBUG, "Destroying debugfs testset %s", ktf_case_name(testset));
> +	_ktf_debugfs_destroy_testset(testset);
> +	/* Remove our debugfs reference cout to testset */
> +	ktf_case_put(testset);
> +}
> +
> +/* /sys/kernel/debug/ktf/coverage shows coverage statistics. */
> +static int ktf_debugfs_cov(struct seq_file *seq, void *v)
> +{
> +	ktf_cov_seq_print(seq);
> +
> +	return 0;
> +}
> +
> +static int ktf_cov_open(struct inode *inode, struct file *file)
> +{
> +	if (!try_module_get(THIS_MODULE))
> +		return -EIO;

{sigh}  I'll stop reviewing now :)

thanks,

greg k-h

Knut Omang Aug. 14, 2019, 5:17 p.m. UTC | #2

On Tue, 2019-08-13 at 10:21 +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 13, 2019 at 08:09:21AM +0200, Knut Omang wrote:
> > From: Alan Maguire <[e-mail]>
> > 
> > While test results is available via netlink from user space, sometimes
> > it may be useful to be able to access the results from the kernel as well,
> > for instance due to a crash. Make that possible via debugfs.
> > 
> > ktf_debugfs.h:   Support for creating a debugfs representation of test
> > 
> > Signed-off-by: Alan Maguire <[e-mail]>
> > Signed-off-by: Knut Omang <[e-mail]>
> > ---
> >  tools/testing/selftests/ktf/kernel/ktf_debugfs.c | 356 ++++++++++++++++-
> >  tools/testing/selftests/ktf/kernel/ktf_debugfs.h |  34 ++-
> >  2 files changed, 390 insertions(+)
> >  create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.c
> >  create mode 100644 tools/testing/selftests/ktf/kernel/ktf_debugfs.h
> > 
> > diff --git a/tools/testing/selftests/ktf/kernel/ktf_debugfs.c b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
> > new file mode 100644
> > index 0000000..a20fbd2
> > --- /dev/null
> > +++ b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
> > @@ -0,0 +1,356 @@
> > +/*
> > + * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
> > + *    Author: Alan Maguire <[e-mail]>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> 
> Has to be the first line of the file, did you run this through
> checkpatch?

Yes, the code has been subject to continuous integration which uses 
a version of my runchecks tool (https://lkml.org/lkml/2018/1/19/157)
to ensure that it is not possible to "forget" to run checkpatch 
(or sparse, smatch doc.check for that sake)

Ironically though I fell victim to my own tooling here,
as I postponed fixing the SPDX_LICENSE_TAG class of issues 
once that test appeared, while working on something else, 
and just forgot to re-enable it again..

> > +static int ktf_run_test_open(struct inode *inode, struct file *file)
> > +{
> > +	struct ktf_test *t;
> > +
> > +	if (!try_module_get(THIS_MODULE))
> > +		return -EIO;
> 
> This is an anti-pattern, and one guaranteed to not work properly.  NEVER
> do this.

Sorry, I didn't know this, and the origin is probably my responsibility.

I know the feeling of never being able to get rid of bad examples 
because they keep getting copied..

The pattern seemed to be widely used the first time I saw it, and although 
somewhat awkward, it seemed to be the standard way then, but as you know, 
my Infiniband driver (
https://github.com/oracle/linux-uek/blob/uek4/qu7/drivers/infiniband/hw/sif/sif_debug.c)
unfortunately never made it to the scrutiny of LKML, since the hardware project 
got cancelled.
The -EIO return value was also copied from merged kernel code back then.

I notice the discussion and your response here: 
http://linux-kernel.2935.n7.nabble.com/debugfs-and-module-unloading-td865175.html
I assume that means that protection against module unload while a debugfs file
is open is now safe.

On older kernels, having this code in place is far better than an unprotected 
debugfs entry/exit - I have tested it extensively in the past :-)

Back when I first used it, I had this cool set of polymorphic 
debugfs file code to list the set of active MRs, CQs, QPs, AHs etc 
that the whole infiniband driver, database and hardware teams loved 
so much that multiple users ended up using it in multiple windows 
from within watch for live observations of state changes, 
and often also running driver load/unloads for testing purposes.

I perfectly agree with you that reducing the hole for a race condition 
is generally a bad idea, but from the above mail thread 
it seems that's the only available choice for older kernels?

(I am asking because I still want to be able to support rather 
old kernels with the github version of KTF)

Anyway, great to know that a better solution now exists!

We'll fix the rest of the issues below as well for the next version..

Thanks!
Knut

> > +
> > +	t = (struct ktf_test *)inode->i_private;
> > +
> > +	return single_open(file, ktf_debugfs_run, t);
> > +}
> > +
> > +static int ktf_debugfs_release(struct inode *inode, struct file *file)
> > +{
> > +	module_put(THIS_MODULE);
> 
> Same here, not ok.
> 
> 
> > +	return single_release(inode, file);
> > +}
> > +
> > +static const struct file_operations ktf_run_test_fops = {
> > +	.open = ktf_run_test_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = ktf_debugfs_release,
> > +};
> > +
> > +static int ktf_results_test_open(struct inode *inode, struct file *file)
> > +{
> > +	struct ktf_test *t;
> > +
> > +	if (!try_module_get(THIS_MODULE))
> > +		return -EIO;
> 
> Nope!
> 
> And why -EIO?  That is not an io issue.

Agreed
> 
> > +void ktf_debugfs_create_test(struct ktf_test *t)
> > +{
> > +	struct ktf_case *testset = ktf_case_find(t->tclass);
> > +
> > +	if (!testset)
> > +		return;
> > +
> > +	memset(&t->debugfs, 0, sizeof(t->debugfs));
> > +
> > +	t->debugfs.debugfs_results_test =
> > +		debugfs_create_file(t->name, S_IFREG | 0444,
> > +				    testset->debugfs.debugfs_results_test,
> > +				 t, &ktf_results_test_fops);
> > +
> > +	if (t->debugfs.debugfs_results_test) {
> 
> How can that variable ever be NULL (hint, it can not.)
> 
> > +		t->debugfs.debugfs_run_test =
> > +			debugfs_create_file(t->name, S_IFREG | 0444,
> > +					    testset->debugfs.debugfs_run_test,
> > +				 t, &ktf_run_test_fops);
> > +		if (!t->debugfs.debugfs_run_test) {
> > +			_ktf_debugfs_destroy_test(t);
> > +		} else {
> > +			/* Take reference for test for debugfs */
> > +			ktf_test_get(t);
> > +		}
> > +	}
> 
> Never test the result of any debugfs call, you do not need to.  Just
> call it and move on, your code flow should NEVER be different with, or
> without, a successful debugfs call.
> 
> 
> > +static int ktf_run_testset_open(struct inode *inode, struct file *file)
> > +{
> > +	struct ktf_case *testset;
> > +
> > +	if (!try_module_get(THIS_MODULE))
> > +		return -EIO;
> 
> Again no.  I hate to know what code you copied this all from, as that
> code is very wrong.  Do you have a pointer to that code anywhere so we
> can fix that up?
> 
> > +
> > +	testset = (struct ktf_case *)inode->i_private;
> > +
> > +	return single_open(file, ktf_debugfs_run_all, testset);
> > +}
> > +
> > +static const struct file_operations ktf_run_testset_fops = {
> > +	.open = ktf_run_testset_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = ktf_debugfs_release,
> 
> If you really care about module references you should be setting the
> owner of the module here.
> 
> > +};
> > +
> > +static void _ktf_debugfs_destroy_testset(struct ktf_case *testset)
> > +{
> > +	debugfs_remove(testset->debugfs.debugfs_run_testset);
> > +	debugfs_remove(testset->debugfs.debugfs_run_test);
> > +	debugfs_remove(testset->debugfs.debugfs_results_testset);
> > +	debugfs_remove(testset->debugfs.debugfs_results_test);
> 
> Why not just recursivly remove the directory?  That way you do not have
> to keep track of any individual files.
> 
> 
> > +}
> > +
> > +void ktf_debugfs_create_testset(struct ktf_case *testset)
> > +{
> > +	char tests_subdir[KTF_DEBUGFS_NAMESZ];
> > +	const char *name = ktf_case_name(testset);
> > +
> > +	memset(&testset->debugfs, 0, sizeof(testset->debugfs));
> > +
> > +	/* First add /sys/kernel/debug/ktf/[results|run]/<testset> */
> > +	testset->debugfs.debugfs_results_testset =
> > +		debugfs_create_file(name, S_IFREG | 0444,
> > +				    ktf_debugfs_resultsdir,
> > +				 testset, &ktf_results_testset_fops);
> > +	if (!testset->debugfs.debugfs_results_testset)
> > +		goto err;
> 
> Again, can never happen, and again, do not do different things depending
> on the result of a debugfs call.
> 
> > +
> > +	testset->debugfs.debugfs_run_testset =
> > +		debugfs_create_file(name, S_IFREG | 0444,
> > +				    ktf_debugfs_rundir,
> > +				    testset, &ktf_run_testset_fops);
> > +	if (!testset->debugfs.debugfs_run_testset)
> > +		goto err;
> 
> Again, nope.
> 
> > +
> > +	/* Now add parent directories for individual test result/run tests
> > +	 * which live in
> > +	 * /sys/kernel/debug/ktf/[results|run]/<testset>-tests/<testname>
> > +	 */
> > +	(void)snprintf(tests_subdir, sizeof(tests_subdir), "%s%s",
> > +			name, KTF_DEBUGFS_TESTS_SUFFIX);
> 
> why (void)?
> 
> 
> > +
> > +	testset->debugfs.debugfs_results_test =
> > +		debugfs_create_dir(tests_subdir, ktf_debugfs_resultsdir);
> > +	if (!testset->debugfs.debugfs_results_test)
> > +		goto err;
> 
> nope :)
> 
> > +
> > +	testset->debugfs.debugfs_run_test =
> > +		debugfs_create_dir(tests_subdir, ktf_debugfs_rundir);
> > +	if (!testset->debugfs.debugfs_run_test)
> > +		goto err;
> 
> Nope :)
> 
> > +
> > +	/* Take reference count for testset.  One will do as we will always
> > +	 * free testset debugfs resources together.
> > +	 */
> > +	ktf_case_get(testset);
> > +	return;
> > +err:
> > +	_ktf_debugfs_destroy_testset(testset);
> > +}
> > +
> > +void ktf_debugfs_destroy_testset(struct ktf_case *testset)
> > +{
> > +	tlog(T_DEBUG, "Destroying debugfs testset %s", ktf_case_name(testset));
> > +	_ktf_debugfs_destroy_testset(testset);
> > +	/* Remove our debugfs reference cout to testset */
> > +	ktf_case_put(testset);
> > +}
> > +
> > +/* /sys/kernel/debug/ktf/coverage shows coverage statistics. */
> > +static int ktf_debugfs_cov(struct seq_file *seq, void *v)
> > +{
> > +	ktf_cov_seq_print(seq);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ktf_cov_open(struct inode *inode, struct file *file)
> > +{
> > +	if (!try_module_get(THIS_MODULE))
> > +		return -EIO;
> 
> {sigh}  I'll stop reviewing now :)


> 
> thanks,
> 
> greg k-h

Greg KH Aug. 15, 2019, 8:49 a.m. UTC | #3

On Wed, Aug 14, 2019 at 07:17:07PM +0200, Knut Omang wrote:
> I notice the discussion and your response here: 
> http://linux-kernel.2935.n7.nabble.com/debugfs-and-module-unloading-td865175.html
> I assume that means that protection against module unload while a debugfs file
> is open is now safe.

It should be, if you set the *owner field of your file_operations
properly.  Try it and see!

> On older kernels, having this code in place is far better than an unprotected 
> debugfs entry/exit - I have tested it extensively in the past :-)

Yes, it seems to work, but again, it really is racy and will fail.
Please don't use it.

> I perfectly agree with you that reducing the hole for a race condition 
> is generally a bad idea, but from the above mail thread 
> it seems that's the only available choice for older kernels?

I have no idea, but please, do not use that pattern of code as it is
racy in all kernels, from all of time.

thanks,

greg k-h

Knut Omang Aug. 15, 2019, 10:35 a.m. UTC | #4

On Thu, 2019-08-15 at 10:49 +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 14, 2019 at 07:17:07PM +0200, Knut Omang wrote:
> > I notice the discussion and your response here: 
> > http://linux-kernel.2935.n7.nabble.com/debugfs-and-module-unloading-td865175.html
> > I assume that means that protection against module unload while a debugfs file
> > is open is now safe.
> 
> It should be, if you set the *owner field of your file_operations
> properly.  Try it and see!

Might be a case for a KTF selftest to play with the timing to increase the chance :)
Wasn't able to make it crash with these simple, short files.

I notice I had set the .owner field correctly myself in that driver 
code I referred to, so that's a "copy regression".

> > On older kernels, having this code in place is far better than an unprotected 
> > debugfs entry/exit - I have tested it extensively in the past :-)
> 
> Yes, it seems to work, but again, it really is racy and will fail.
> Please don't use it.
> 
> > I perfectly agree with you that reducing the hole for a race condition 
> > is generally a bad idea, but from the above mail thread 
> > it seems that's the only available choice for older kernels?
> 
> I have no idea, but please, do not use that pattern of code as it is
> racy in all kernels, from all of time.

Ok, will remove it :-)

I tried in vain to find the commit from Al Viro that made the code safe,
to identify which kernels that are safe from this issue,
but he has a **lot** of commits, do you have a clue for what/where to look?

It will be good to have a mention/comment on this for future reference, 
like the earliest kernel version where this is safe.

Maybe we can even get rid of some more of the remaining of these too..
(I notice there's 65 cases of 'if (!try_module_get(THIS_MODULE))'
right now)

Thanks!
Knut

> 
> thanks,
> 
> greg k-h

Greg KH Aug. 15, 2019, 10:52 a.m. UTC | #5

On Thu, Aug 15, 2019 at 12:35:26PM +0200, Knut Omang wrote:
> On Thu, 2019-08-15 at 10:49 +0200, Greg Kroah-Hartman wrote:
> > > I perfectly agree with you that reducing the hole for a race condition 
> > > is generally a bad idea, but from the above mail thread 
> > > it seems that's the only available choice for older kernels?
> > 
> > I have no idea, but please, do not use that pattern of code as it is
> > racy in all kernels, from all of time.
> 
> Ok, will remove it :-)
> 
> I tried in vain to find the commit from Al Viro that made the code safe,
> to identify which kernels that are safe from this issue,
> but he has a **lot** of commits, do you have a clue for what/where to look?
> 
> It will be good to have a mention/comment on this for future reference, 
> like the earliest kernel version where this is safe.

Always use a "newer" kernel to be "safe" and you will be fine :)

> Maybe we can even get rid of some more of the remaining of these too..
> (I notice there's 65 cases of 'if (!try_module_get(THIS_MODULE))'
> right now)

Something to put on a TODO list somewhere...

thanks,

greg k-h

Patch

diff --git a/tools/testing/selftests/ktf/kernel/ktf_debugfs.c b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
new file mode 100644
index 0000000..a20fbd2
--- /dev/null
+++ b/tools/testing/selftests/ktf/kernel/ktf_debugfs.c
@@ -0,0 +1,356 @@ 
+/*
+ * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
+ *    Author: Alan Maguire <[e-mail]>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ */
+#include <asm/unistd.h>
+#include <linux/module.h>
+#include <linux/time.h>
+#include "ktf_debugfs.h"
+#include "ktf.h"
+#include "ktf_test.h"
+#include "ktf_cov.h"
+
+/* Create a debugfs representation of test sets/tests.  Hierarchy looks like
+ * this:
+ *
+ * Path					Semantics
+ * /sys/kernel/debug/ktf/run/<testset>		Run all tests in testset
+ * /sys/kernel/debug/ktf/run/<testset>/<test>	Run specific test in testset
+ * /sys/kernel/debug/ktf/results/<testset>	Show results of last run for
+ *						testset
+ *
+ * /sys/kernel/debug/ktf/results/<testset>/<test>
+ *						Show results of last run for
+ *						test
+ *
+ */
+
+static struct dentry *ktf_debugfs_rootdir;
+static struct dentry *ktf_debugfs_rundir;
+static struct dentry *ktf_debugfs_resultsdir;
+static struct dentry *ktf_debugfs_cov_file;
+
+static void ktf_debugfs_print_result(struct seq_file *seq, struct ktf_test *t)
+{
+	struct timespec now;
+
+	if (t && strlen(t->log) > 0) {
+		getnstimeofday(&now);
+		seq_printf(seq, "[%s/%s, %ld seconds ago] %s\n",
+			   t->tclass, t->name,
+			   now.tv_sec - t->lastrun.tv_sec, t->log);
+	}
+}
+
+/* /sys/kernel/debug/ktf/results/<testset>-tests/<test> shows specific result */
+static int ktf_debugfs_result(struct seq_file *seq, void *v)
+{
+	struct ktf_test *t = (struct ktf_test *)seq->private;
+
+	ktf_debugfs_print_result(seq, t);
+
+	return 0;
+}
+
+/* /sys/kernel/debug/ktf/results/<testset> shows all results for testset. */
+static int ktf_debugfs_results_all(struct seq_file *seq, void *v)
+{
+	struct ktf_case *testset = (struct ktf_case *)seq->private;
+	struct ktf_test *t;
+
+	if (!testset)
+		return 0;
+
+	seq_printf(seq, "%s results:\n", ktf_case_name(testset));
+	ktf_testcase_for_each_test(t, testset)
+		ktf_debugfs_print_result(seq, t);
+
+	return 0;
+}
+
+/* /sys/kernel/debug/ktf/run/<testset>-tests/<test> runs specific test. */
+static int ktf_debugfs_run(struct seq_file *seq, void *v)
+{
+	struct ktf_test *t = (struct ktf_test *)seq->private;
+
+	if (!t)
+		return 0;
+
+	ktf_run_hook(NULL, NULL, t, 0, NULL, 0);
+	ktf_debugfs_print_result(seq, t);
+
+	return 0;
+}
+
+/* /sys/kernel/debug/ktf/run/<testset> runs all tests in testset. */
+static int ktf_debugfs_run_all(struct seq_file *seq, void *v)
+{
+	struct ktf_case *testset = (struct ktf_case *)seq->private;
+	struct ktf_test *t;
+
+	if (!testset)
+		return 0;
+
+	seq_printf(seq, "Running %s\n", ktf_case_name(testset));
+	ktf_testcase_for_each_test(t, testset) {
+		ktf_run_hook(NULL, NULL, t, 0, NULL, 0);
+		ktf_debugfs_print_result(seq, t);
+	}
+
+	return 0;
+}
+
+static int ktf_run_test_open(struct inode *inode, struct file *file)
+{
+	struct ktf_test *t;
+
+	if (!try_module_get(THIS_MODULE))
+		return -EIO;
+
+	t = (struct ktf_test *)inode->i_private;
+
+	return single_open(file, ktf_debugfs_run, t);
+}
+
+static int ktf_debugfs_release(struct inode *inode, struct file *file)
+{
+	module_put(THIS_MODULE);
+	return single_release(inode, file);
+}
+
+static const struct file_operations ktf_run_test_fops = {
+	.open = ktf_run_test_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = ktf_debugfs_release,
+};
+
+static int ktf_results_test_open(struct inode *inode, struct file *file)
+{
+	struct ktf_test *t;
+
+	if (!try_module_get(THIS_MODULE))
+		return -EIO;
+
+	t = (struct ktf_test *)inode->i_private;
+
+	return single_open(file, ktf_debugfs_result, t);
+}
+
+static const struct file_operations ktf_results_test_fops = {
+	.open = ktf_results_test_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = ktf_debugfs_release,
+};
+
+static void _ktf_debugfs_destroy_test(struct ktf_test *t)
+{
+	if (!t)
+		return;
+
+	tlog(T_DEBUG, "Destroying debugfs test %s", t->name);
+	debugfs_remove(t->debugfs.debugfs_results_test);
+	debugfs_remove(t->debugfs.debugfs_run_test);
+	memset(&t->debugfs, 0, sizeof(t->debugfs));
+}
+
+void ktf_debugfs_create_test(struct ktf_test *t)
+{
+	struct ktf_case *testset = ktf_case_find(t->tclass);
+
+	if (!testset)
+		return;
+
+	memset(&t->debugfs, 0, sizeof(t->debugfs));
+
+	t->debugfs.debugfs_results_test =
+		debugfs_create_file(t->name, S_IFREG | 0444,
+				    testset->debugfs.debugfs_results_test,
+				 t, &ktf_results_test_fops);
+
+	if (t->debugfs.debugfs_results_test) {
+		t->debugfs.debugfs_run_test =
+			debugfs_create_file(t->name, S_IFREG | 0444,
+					    testset->debugfs.debugfs_run_test,
+				 t, &ktf_run_test_fops);
+		if (!t->debugfs.debugfs_run_test) {
+			_ktf_debugfs_destroy_test(t);
+		} else {
+			/* Take reference for test for debugfs */
+			ktf_test_get(t);
+		}
+	}
+	/* Drop reference to testset from ktf_case_find(). */
+	ktf_case_put(testset);
+}
+
+void ktf_debugfs_destroy_test(struct ktf_test *t)
+{
+	_ktf_debugfs_destroy_test(t);
+	/* Release reference now debugfs files are gone. */
+	ktf_test_put(t);
+}
+
+static int ktf_results_testset_open(struct inode *inode, struct file *file)
+{
+	struct ktf_case *testset;
+
+	if (!try_module_get(THIS_MODULE))
+		return -EIO;
+
+	testset = (struct ktf_case *)inode->i_private;
+
+	return single_open(file, ktf_debugfs_results_all, testset);
+}
+
+static const struct file_operations ktf_results_testset_fops = {
+	.open = ktf_results_testset_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = ktf_debugfs_release,
+};
+
+static int ktf_run_testset_open(struct inode *inode, struct file *file)
+{
+	struct ktf_case *testset;
+
+	if (!try_module_get(THIS_MODULE))
+		return -EIO;
+
+	testset = (struct ktf_case *)inode->i_private;
+
+	return single_open(file, ktf_debugfs_run_all, testset);
+}
+
+static const struct file_operations ktf_run_testset_fops = {
+	.open = ktf_run_testset_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = ktf_debugfs_release,
+};
+
+static void _ktf_debugfs_destroy_testset(struct ktf_case *testset)
+{
+	debugfs_remove(testset->debugfs.debugfs_run_testset);
+	debugfs_remove(testset->debugfs.debugfs_run_test);
+	debugfs_remove(testset->debugfs.debugfs_results_testset);
+	debugfs_remove(testset->debugfs.debugfs_results_test);
+}
+
+void ktf_debugfs_create_testset(struct ktf_case *testset)
+{
+	char tests_subdir[KTF_DEBUGFS_NAMESZ];
+	const char *name = ktf_case_name(testset);
+
+	memset(&testset->debugfs, 0, sizeof(testset->debugfs));
+
+	/* First add /sys/kernel/debug/ktf/[results|run]/<testset> */
+	testset->debugfs.debugfs_results_testset =
+		debugfs_create_file(name, S_IFREG | 0444,
+				    ktf_debugfs_resultsdir,
+				 testset, &ktf_results_testset_fops);
+	if (!testset->debugfs.debugfs_results_testset)
+		goto err;
+
+	testset->debugfs.debugfs_run_testset =
+		debugfs_create_file(name, S_IFREG | 0444,
+				    ktf_debugfs_rundir,
+				    testset, &ktf_run_testset_fops);
+	if (!testset->debugfs.debugfs_run_testset)
+		goto err;
+
+	/* Now add parent directories for individual test result/run tests
+	 * which live in
+	 * /sys/kernel/debug/ktf/[results|run]/<testset>-tests/<testname>
+	 */
+	(void)snprintf(tests_subdir, sizeof(tests_subdir), "%s%s",
+			name, KTF_DEBUGFS_TESTS_SUFFIX);
+
+	testset->debugfs.debugfs_results_test =
+		debugfs_create_dir(tests_subdir, ktf_debugfs_resultsdir);
+	if (!testset->debugfs.debugfs_results_test)
+		goto err;
+
+	testset->debugfs.debugfs_run_test =
+		debugfs_create_dir(tests_subdir, ktf_debugfs_rundir);
+	if (!testset->debugfs.debugfs_run_test)
+		goto err;
+
+	/* Take reference count for testset.  One will do as we will always
+	 * free testset debugfs resources together.
+	 */
+	ktf_case_get(testset);
+	return;
+err:
+	_ktf_debugfs_destroy_testset(testset);
+}
+
+void ktf_debugfs_destroy_testset(struct ktf_case *testset)
+{
+	tlog(T_DEBUG, "Destroying debugfs testset %s", ktf_case_name(testset));
+	_ktf_debugfs_destroy_testset(testset);
+	/* Remove our debugfs reference cout to testset */
+	ktf_case_put(testset);
+}
+
+/* /sys/kernel/debug/ktf/coverage shows coverage statistics. */
+static int ktf_debugfs_cov(struct seq_file *seq, void *v)
+{
+	ktf_cov_seq_print(seq);
+
+	return 0;
+}
+
+static int ktf_cov_open(struct inode *inode, struct file *file)
+{
+	if (!try_module_get(THIS_MODULE))
+		return -EIO;
+
+	return single_open(file, ktf_debugfs_cov, NULL);
+}
+
+static const struct file_operations ktf_cov_fops = {
+	.open = ktf_cov_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = ktf_debugfs_release,
+};
+
+void ktf_debugfs_cleanup(void)
+{
+	tlog(T_DEBUG, "Removing ktf debugfs dirs...");
+	debugfs_remove(ktf_debugfs_cov_file);
+	debugfs_remove(ktf_debugfs_rundir);
+	debugfs_remove(ktf_debugfs_resultsdir);
+	debugfs_remove(ktf_debugfs_rootdir);
+}
+
+void ktf_debugfs_init(void)
+{
+	ktf_debugfs_rootdir = debugfs_create_dir(KTF_DEBUGFS_ROOT, NULL);
+	if (!ktf_debugfs_rootdir)
+		goto err;
+	ktf_debugfs_rundir = debugfs_create_dir(KTF_DEBUGFS_RUN,
+						ktf_debugfs_rootdir);
+	if (!ktf_debugfs_rundir)
+		goto err;
+	ktf_debugfs_resultsdir = debugfs_create_dir(KTF_DEBUGFS_RESULTS,
+						    ktf_debugfs_rootdir);
+	if (!ktf_debugfs_resultsdir)
+		goto err;
+
+	ktf_debugfs_cov_file = debugfs_create_file(KTF_DEBUGFS_COV,
+						   S_IFREG | 0444,
+						   ktf_debugfs_rootdir,
+						   NULL,
+						   &ktf_cov_fops);
+	if (ktf_debugfs_cov_file)
+		return;
+err:
+	terr("Could not init %s\n", KTF_DEBUGFS_ROOT);
+	ktf_debugfs_cleanup();
+}
diff --git a/tools/testing/selftests/ktf/kernel/ktf_debugfs.h b/tools/testing/selftests/ktf/kernel/ktf_debugfs.h
new file mode 100644
index 0000000..4dab1ea
--- /dev/null
+++ b/tools/testing/selftests/ktf/kernel/ktf_debugfs.h
@@ -0,0 +1,34 @@ 
+/*
+ * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
+ *    Author: Alan Maguire <[e-mail]>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * ktf_debugfs.h: Support for creating a debugfs representation of test
+ * sets/tests.
+ */
+
+#ifndef KTF_DEBUGFS_H
+#define KTF_DEBUGFS_H
+#include <linux/debugfs.h>
+
+#define KTF_DEBUGFS_ROOT                        "ktf"
+#define KTF_DEBUGFS_RUN                         "run"
+#define KTF_DEBUGFS_RESULTS                     "results"
+#define KTF_DEBUGFS_COV				"coverage"
+#define KTF_DEBUGFS_TESTS_SUFFIX                "-tests"
+
+#define KTF_DEBUGFS_NAMESZ                      256
+
+struct ktf_test;
+struct ktf_case;
+
+void ktf_debugfs_create_test(struct ktf_test *);
+void ktf_debugfs_destroy_test(struct ktf_test *);
+void ktf_debugfs_create_testset(struct ktf_case *);
+void ktf_debugfs_destroy_testset(struct ktf_case *);
+void ktf_debugfs_init(void);
+void ktf_debugfs_cleanup(void);
+
+
+#endif