From: Luis Chamberlain <mcgrof@kernel.org>
To: tj@kernel.org, gregkh@linuxfoundation.org,
akpm@linux-foundation.org, minchan@kernel.org, jeyu@kernel.org,
shuah@kernel.org
Cc: bvanassche@acm.org, dan.j.williams@intel.com, joe@perches.com,
tglx@linutronix.de, mcgrof@kernel.org, keescook@chromium.org,
rostedt@goodmis.org, linux-spdx@vger.kernel.org,
linux-doc@vger.kernel.org, linux-block@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [PATCH v8 00/12] syfs: generic deadlock fix with module removal
Date: Mon, 27 Sep 2021 09:37:53 -0700 [thread overview]
Message-ID: <20210927163805.808907-1-mcgrof@kernel.org> (raw)
This is a follow up to my v7 series of fixes for the zram driver [0]
which ended up uncovering a generic deadlock issue with sysfs and module
removal. I've reported this issue and proposed a few patches first since
March 2021 [1]. At the end of this email you will find an itemized list
of changes since that v1 series, you can also find these changes on my
branch 20210927-sysfs-generic-deadlock-fix [4] which is based on
linux-next tag next-20210927.
Just a heads up, I'm goin on vacation in two days, won't be back until
Monday October 11th.
On this v8 I incorporate feedback from the v7 series, namely:
- Tejun requested I move the struct module to the last attribute when
extending functions
- As per discussion with Tejun, trimmed and clarified the commit log
and documentation on the generic fix on patch 7
- As requested by Bart Van Assche, I simplied the setting of the
struct test_config *config into one line instead of two on many
places on patch 3 which adds the new sysfs selftest
- Dan Williams had some questions about patch 7, and so clarified these
questions using a more elaborate example on the commit log to show
where the lock call was happening.
- Trimmed the Cc list considerably as it was way too long before
- Rebased onto linux-next tag next-20210927
Below a list of changes of this patch set since its inception:
On v1:
- Open coded the sysfs deadlock race to only be localized by the zram
driver
Changes on v2:
- used bdgrab() as well for another race which was speculated by
Minchan
- improved documentation of fixes
Changes on v3:
- used a localized zram macros for the sysfs attributes instead of
open coding on each routine
- replaced bdget() stuff for a generic get_device() and bus_get() on
dev_attr_show() / dev_attr_store() for the issue speculated by
Michan
Changes on v4:
- Cosmetic fixes on the zram fixes as requested by Greg
- Split out the driver core fix as requested by Greg for the
issue speculated by Michan. This fix ended up getting up to its 4th
patch iteration [2] and eventually hit linux-next. We got a 0day
0day suspend stres fail for this patch [3]
Changes on v5:
- I ended up writing a test_sysfs driver and with it I ended up
proving that the issue speculated by Michen was not possible and
so I asked Greg to drop the patch from his queue titled
"sysfs: fix kobject refcount to address races with kobject removal"
- checkpatch fixes for the zram changes
Changes on v6:
- I submitted my test_sysfs driver for inclusion upstream which easily
abstracted the deadlock issue in a driver generically [4]
- I rebased the zram fixes and added also a new patch for zram to use
ATTRIBUTE_GROUPS As per Minchen I sent the patches to be merged
through Andrew Morton.
- Greg ended up NACK'ing the patchset because he was not sure the fix
was correct still
Changes on v7:
- Formalizes the original proposed generic sysfs fix intead of using
macro helpers to work around the issue
- I decided it is best to merge all the effort together into
one patch set because communication was being lost when I split the
patches up. This was not helping in any way to either fix the zram
issues or come to consensus on a generic solution. The patches are
also merged now because they are all related now.
- Running checkpatch exposed that S_IRWXUGO and S_IRWXU|S_IRUGO|S_IXUGO
should be replaced, so I did that in this series in two new patches
- Adds a try_module_get() documentation extension with tribal
knowledge and new information I don't think some folks still believe
in. The new test_sysfs selftest however proves this information to
be correct, the same selftest can be used to try to prove that
documentation incorrect
- Because the fix is now generic zram's deadlock can easily be fixed
now by just making it use ATTRIBUTE_GROUPS().
[0] https://lkml.kernel.org/r/YUjLAbnEB5qPfnL8@slm.duckdns.org
[1] https://lkml.kernel.org/r/20210306022035.11266-1-mcgrof@kernel.org
[2] https://lkml.kernel.org/r/20210623215007.862787-1-mcgrof@kernel.org
[3] https://lkml.kernel.org/r/20210701022737.GC21279@xsang-OptiPlex-9020
[4] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20210927-sysfs-generic-deadlock-fix
Luis Chamberlain (12):
LICENSES: Add the copyleft-next-0.3.1 license
testing: use the copyleft-next-0.3.1 SPDX tag
selftests: add tests_sysfs module
kernfs: add initial failure injection support
test_sysfs: add support to use kernfs failure injection
kernel/module: add documentation for try_module_get()
fs/kernfs/symlink.c: replace S_IRWXUGO with 0777 on
kernfs_create_link()
fs/sysfs/dir.c: replace S_IRWXU|S_IRUGO|S_IXUGO with 0755
sysfs_create_dir_ns()
sysfs: fix deadlock race with module removal
test_sysfs: enable deadlock tests by default
zram: fix crashes with cpu hotplug multistate
zram: use ATTRIBUTE_GROUPS to fix sysfs deadlock module removal
.../fault-injection/fault-injection.rst | 22 +
LICENSES/dual/copyleft-next-0.3.1 | 237 +++
MAINTAINERS | 9 +-
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 4 +-
drivers/block/zram/zram_drv.c | 74 +-
fs/kernfs/Makefile | 1 +
fs/kernfs/dir.c | 44 +-
fs/kernfs/failure-injection.c | 91 ++
fs/kernfs/file.c | 19 +-
fs/kernfs/kernfs-internal.h | 75 +-
fs/kernfs/symlink.c | 4 +-
fs/sysfs/dir.c | 5 +-
fs/sysfs/file.c | 6 +-
fs/sysfs/group.c | 3 +-
include/linux/kernfs.h | 19 +-
include/linux/module.h | 34 +-
include/linux/sysfs.h | 52 +-
kernel/cgroup/cgroup.c | 2 +-
lib/Kconfig.debug | 25 +
lib/Makefile | 1 +
lib/test_kmod.c | 12 +-
lib/test_sysctl.c | 12 +-
lib/test_sysfs.c | 952 ++++++++++++
tools/testing/selftests/kmod/kmod.sh | 13 +-
tools/testing/selftests/sysctl/sysctl.sh | 12 +-
tools/testing/selftests/sysfs/Makefile | 12 +
tools/testing/selftests/sysfs/config | 5 +
tools/testing/selftests/sysfs/sysfs.sh | 1383 +++++++++++++++++
28 files changed, 3026 insertions(+), 102 deletions(-)
create mode 100644 LICENSES/dual/copyleft-next-0.3.1
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
--
2.30.2
next reply other threads:[~2021-09-27 16:38 UTC|newest]
Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-27 16:37 Luis Chamberlain [this message]
2021-09-27 16:37 ` [PATCH v8 01/12] LICENSES: Add the copyleft-next-0.3.1 license Luis Chamberlain
[not found] ` <202110050907.35FBD2A1@keescook>
[not found] ` <YWR2ZrtzChamY1y4@bombadil.infradead.org>
2021-10-11 17:57 ` Kees Cook
2021-09-27 16:37 ` [PATCH v8 02/12] testing: use the copyleft-next-0.3.1 SPDX tag Luis Chamberlain
2021-10-05 16:11 ` Kees Cook
2021-09-27 16:37 ` [PATCH v8 03/12] selftests: add tests_sysfs module Luis Chamberlain
2021-10-05 14:16 ` Greg KH
2021-10-05 16:57 ` Tim.Bird
2021-10-11 17:40 ` Luis Chamberlain
2021-10-11 17:38 ` Luis Chamberlain
2021-10-07 14:23 ` Miroslav Benes
2021-10-11 19:11 ` Luis Chamberlain
[not found] ` <202110050912.3DF681ED@keescook>
2021-10-11 19:03 ` Luis Chamberlain
2021-09-27 16:37 ` [PATCH v8 04/12] kernfs: add initial failure injection support Luis Chamberlain
2021-10-05 19:47 ` Kees Cook
2021-10-11 20:44 ` Luis Chamberlain
2021-09-27 16:37 ` [PATCH v8 05/12] test_sysfs: add support to use kernfs failure injection Luis Chamberlain
2021-10-05 19:51 ` Kees Cook
2021-10-11 20:56 ` Luis Chamberlain
2021-09-27 16:37 ` [PATCH v8 06/12] kernel/module: add documentation for try_module_get() Luis Chamberlain
2021-10-05 19:58 ` Kees Cook
2021-10-11 21:16 ` Luis Chamberlain
2021-09-27 16:38 ` [PATCH v8 07/12] fs/kernfs/symlink.c: replace S_IRWXUGO with 0777 on kernfs_create_link() Luis Chamberlain
2021-10-05 19:59 ` Kees Cook
2021-09-27 16:38 ` [PATCH v8 08/12] fs/sysfs/dir.c: replace S_IRWXU|S_IRUGO|S_IXUGO with 0755 sysfs_create_dir_ns() Luis Chamberlain
2021-10-05 16:05 ` Kees Cook
2021-09-27 16:38 ` [PATCH v8 09/12] sysfs: fix deadlock race with module removal Luis Chamberlain
2021-10-05 9:24 ` Ming Lei
2021-10-11 21:25 ` Luis Chamberlain
2021-10-12 0:20 ` Ming Lei
2021-10-12 21:18 ` Luis Chamberlain
2021-10-13 1:07 ` Ming Lei
2021-10-13 12:35 ` Luis Chamberlain
2021-10-13 15:04 ` Ming Lei
2021-10-13 21:16 ` Luis Chamberlain
2021-10-05 20:50 ` Kees Cook
2021-10-11 22:26 ` Luis Chamberlain
2021-10-13 12:41 ` Luis Chamberlain
2021-09-27 16:38 ` [PATCH v8 10/12] test_sysfs: enable deadlock tests by default Luis Chamberlain
2021-09-27 16:38 ` [PATCH v8 11/12] zram: fix crashes with cpu hotplug multistate Luis Chamberlain
2021-10-05 20:55 ` Kees Cook
2021-10-11 18:27 ` Luis Chamberlain
2021-10-14 1:55 ` Ming Lei
2021-10-14 2:11 ` Ming Lei
2021-10-14 20:24 ` Luis Chamberlain
2021-10-14 23:52 ` Ming Lei
2021-10-15 0:22 ` Luis Chamberlain
2021-10-15 8:36 ` Ming Lei
2021-10-15 8:52 ` Greg KH
2021-10-15 17:31 ` Luis Chamberlain
2021-10-16 11:28 ` Ming Lei
2021-10-18 19:32 ` Luis Chamberlain
2021-10-19 2:34 ` Ming Lei
2021-10-19 6:23 ` Miroslav Benes
2021-10-19 9:23 ` Ming Lei
2021-10-20 6:43 ` Miroslav Benes
2021-10-20 7:49 ` Ming Lei
2021-10-20 8:19 ` Miroslav Benes
2021-10-20 8:28 ` Greg KH
2021-10-25 9:58 ` Miroslav Benes
2021-10-20 10:09 ` Ming Lei
2021-10-26 8:48 ` Petr Mladek
2021-10-26 15:37 ` Ming Lei
2021-10-26 17:01 ` Luis Chamberlain
2021-10-27 11:57 ` Miroslav Benes
2021-10-27 14:27 ` Luis Chamberlain
2021-11-02 15:24 ` Petr Mladek
2021-11-02 16:25 ` Luis Chamberlain
2021-11-03 0:01 ` Ming Lei
2021-11-03 12:44 ` Luis Chamberlain
2021-10-27 11:42 ` Miroslav Benes
2021-11-02 14:15 ` Petr Mladek
2021-11-02 14:51 ` Petr Mladek
2021-11-02 15:17 ` Ming Lei
2021-11-02 14:56 ` Ming Lei
2021-10-19 15:28 ` Luis Chamberlain
2021-10-19 16:29 ` Ming Lei
2021-10-19 19:36 ` Luis Chamberlain
2021-10-20 1:15 ` Ming Lei
2021-10-20 15:48 ` Luis Chamberlain
2021-10-21 0:39 ` Ming Lei
2021-10-21 17:18 ` Luis Chamberlain
2021-10-22 0:05 ` Ming Lei
2021-10-19 15:50 ` Luis Chamberlain
2021-10-19 16:25 ` Greg KH
2021-10-19 16:30 ` Luis Chamberlain
2021-10-19 17:28 ` Greg KH
2021-10-19 19:46 ` Luis Chamberlain
2021-10-19 16:39 ` Ming Lei
2021-10-19 19:38 ` Luis Chamberlain
2021-10-20 0:55 ` Ming Lei
2021-09-27 16:38 ` [PATCH v8 12/12] zram: use ATTRIBUTE_GROUPS to fix sysfs deadlock module removal Luis Chamberlain
2021-10-05 20:57 ` Kees Cook
2021-10-11 18:28 ` 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=20210927163805.808907-1-mcgrof@kernel.org \
--to=mcgrof@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=bvanassche@acm.org \
--cc=dan.j.williams@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=jeyu@kernel.org \
--cc=joe@perches.com \
--cc=keescook@chromium.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-spdx@vger.kernel.org \
--cc=minchan@kernel.org \
--cc=rostedt@goodmis.org \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.