mbox series

[0/4] selftests: add a new test driver for sysfs

Message ID 20210702050543.2693141-1-mcgrof@kernel.org
Headers show
Series selftests: add a new test driver for sysfs | expand

Message

Luis Chamberlain July 2, 2021, 5:05 a.m. UTC
I had posted a patch to fix a theoretical race with sysfs and device
removal [0].  While the issue is no longer present with the patch
present, the zram driver has already a lot of enhancements, so much so,
that the race alone is very difficult to reproduce. Likewise, the zram
driver had a series of other races on module removal which I recently
posted fixes for [1], and it makes it unclear if these paper over the
possible theoretical sysfs race. Although we even have gdb output
from an actual race where this issue presented itself, there are
other races which could happen before that and so what we realy need
is a clean separate driver where we can experiment and try to reproduce
unusual races.

This adds such a driver, a new sysfs_test driver, along with a set of
new tests for it. We take hint of observed issues with the sysfs on the
zram driver, and build sandbox based where wher can try to poke holes at
the kernel with.

There are two main races we're after trying to reproduce:

  1) proving the deadlock is real
  2) allowing for enough slack for us to try to see if we can
     reproduce the syfs / device removal race

In order to tackle the second race, we need a bit of help from kernefs,
given that the race is difficult to reproduce. So we add fault injection
support to kernfs, which allows us to trigger all possible races on
write.

This should be enough evidence for us to drop the suggested patch for
sysfs for the second race. The first race however which leads to a
deadlock is clearly explained now and I hope this shows how we need a
generic solution.

[0] https://lkml.kernel.org/r/20210623215007.862787-1-mcgrof@kernel.org
[1] https://lkml.kernel.org/r/20210702043716.2692247-1-mcgrof@kernel.org

Luis Chamberlain (4):
  selftests: add tests_sysfs module
  kernfs: add initial failure injection support
  test_sysfs: add support to use kernfs failure injection
  test_sysfs: demonstrate deadlock fix

 .../fault-injection/fault-injection.rst       |   22 +
 MAINTAINERS                                   |    9 +-
 fs/kernfs/Makefile                            |    1 +
 fs/kernfs/failure-injection.c                 |   82 +
 fs/kernfs/file.c                              |   13 +
 fs/kernfs/kernfs-internal.h                   |   73 +
 include/linux/kernfs.h                        |    5 +
 lib/Kconfig.debug                             |   23 +
 lib/Makefile                                  |    1 +
 lib/test_sysfs.c                              | 1037 +++++++++++++
 tools/testing/selftests/sysfs/Makefile        |   12 +
 tools/testing/selftests/sysfs/config          |    5 +
 tools/testing/selftests/sysfs/sysfs.sh        | 1376 +++++++++++++++++
 13 files changed, 2658 insertions(+), 1 deletion(-)
 create mode 100644 fs/kernfs/failure-injection.c
 create mode 100644 lib/test_sysfs.c
 create mode 100644 tools/testing/selftests/sysfs/Makefile
 create mode 100644 tools/testing/selftests/sysfs/config
 create mode 100755 tools/testing/selftests/sysfs/sysfs.sh

Comments

Luis Chamberlain July 2, 2021, 7:02 p.m. UTC | #1
On Fri, Jul 02, 2021 at 07:21:12AM +0200, Greg KH wrote:
> On Thu, Jul 01, 2021 at 10:05:40PM -0700, Luis Chamberlain wrote:
> > @@ -0,0 +1,953 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * sysfs test driver
> > + *
> > + * Copyright (C) 2021 Luis Chamberlain <mcgrof@kernel.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the Free
> > + * Software Foundation; either version 2 of the License, or at your option any
> > + * later version; or, when distributed separately from the Linux kernel or
> > + * when incorporated into other software packages, subject to the following
> > + * license:
> 
> This boilerplate should not be here, only the spdx line is needed.

As per Documentation/process/license-rules.rst we use the SPDX license
tag for the license that applies but it also states about dual
licensing:

"Aside from that, individual files can be provided under a dual license,         
e.g. one of the compatible GPL variants and alternatively under a               
permissive license like BSD, MIT etc."

Let me know if things should change somehow here to clarify this better.

> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of copyleft-next (version 0.3.1 or later) as published
> > + * at http://copyleft-next.org/.
> 
> Please no, this is a totally different license :(

Dual licensing copyleft-next / GPLv2 was discussed in 2016 and I have
been using it since for my new drivers. As far as the kernel is
concerned only the GPLv2 applies and this is cleary clarified with the
MODULE_LICENSE("GPL") as per Linus' preference [0] on this topic. Later
due to Ted's and Alans's request I ironed out an "or" language clause to
use [1].  This was also vetted by 2 attorneys at SUSE, and one at Red
Hat [2]. The first driver submission under this dual strategy was
lib/test_sysctl.c through commit 9308f2f9e7f05 ("test_sysctl: add
dedicated proc sysctl test driver") merged in July 2017. Shortly after
that I also added test_kmod through commit d9c6a72d6fa29 ("kmod: add
test driver to stress test the module loader") in the same month. These
two drivers went in just a few months before the SPDX license pratice
kicked in.

And so we already have this practice in place of dual GPLv2 /
copyleft-next. What was missing was the SPDX tag. I can go and
update the other 2 drivers to reflect this as well, but as far as I
can tell, due to the dual licensing the boilerplace is still needed
in this case.

Let me know!

[0] https://lore.kernel.org/lkml/CA+55aFyhxcvD+q7tp+-yrSFDKfR0mOHgyEAe=f_94aKLsOu0Og@mail.gmail.com/
[1] https://lkml.kernel.org/r/1495234558.7848.122.camel@linux.intel.com
[2] https://lore.kernel.org/lkml/20170516232702.GL17314@wotan.suse.de/

  Luis
Luis Chamberlain July 3, 2021, 3:52 p.m. UTC | #2
On Sat, Jul 03, 2021 at 06:46:46AM +0200, Greg KH wrote:
> On Fri, Jul 02, 2021 at 12:02:30PM -0700, Luis Chamberlain wrote:
> > On Fri, Jul 02, 2021 at 07:21:12AM +0200, Greg KH wrote:
> > > On Thu, Jul 01, 2021 at 10:05:40PM -0700, Luis Chamberlain wrote:
> > > > @@ -0,0 +1,953 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + * sysfs test driver
> > > > + *
> > > > + * Copyright (C) 2021 Luis Chamberlain <mcgrof@kernel.org>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify it
> > > > + * under the terms of the GNU General Public License as published by the Free
> > > > + * Software Foundation; either version 2 of the License, or at your option any
> > > > + * later version; or, when distributed separately from the Linux kernel or
> > > > + * when incorporated into other software packages, subject to the following
> > > > + * license:
> > > 
> > > This boilerplate should not be here, only the spdx line is needed.
> > 
> > As per Documentation/process/license-rules.rst we use the SPDX license
> > tag for the license that applies but it also states about dual
> > licensing:
> > 
> > "Aside from that, individual files can be provided under a dual license,         
> > e.g. one of the compatible GPL variants and alternatively under a               
> > permissive license like BSD, MIT etc."
> > 
> > Let me know if things should change somehow here to clarify this better.
> 
> The spdx line is not matching the actual license for the file, which is
> wrong.

We don't have spdx license tag yet for copyleft-next, and although
when using dual gplv2 or copyleft-next gplv2 applies I did fail to see
can use spdx for dual licensing such as:

# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause

> And "copyright-left" is not a valid license according to our list of
> valid licenses in the LICENSES directory, so please do not add it to
> kernel code when it is obviously not needed.

You mean copyleft-next. Yes I'd have to add that. Given that we already
have two test drivers with that license I'll go ahead and add that.

> And given that this is directly interacting with sysfs, which is
> GPLv2-only, trying to claim a different license on the code that tests
> it is going to be a total mess for any lawyer who wants to look into
> this.  Just keep it simple please.

The faul injection code I added follows the exact license for sysfs. The
only interaction with the test_sysfs and sysfs is an exported symbol
for a completion structure. The other dual gpl OR copyleft-next test
drivers already present in the kernel also use exported symbols too, so
I see nothing new here.

  Luis
Richard Fontana July 4, 2021, 8:26 p.m. UTC | #3
On Sat, Jul 3, 2021 at 11:52 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> We don't have spdx license tag yet for copyleft-next,

https://spdx.org/licenses/copyleft-next-0.3.0.html
https://spdx.org/licenses/copyleft-next-0.3.1.html

Richard