From: Andreas Gruenbacher <agruenba@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andreas Gruenbacher <agruenba@redhat.com>,
cluster-devel@redhat.com, linux-kernel@vger.kernel.org,
Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
Matthew Wilcox <willy@infradead.org>
Subject: [RFC 0/9] gfs2: handle page faults during read and write
Date: Mon, 31 May 2021 19:01:14 +0200 [thread overview]
Message-ID: <20210531170123.243771-1-agruenba@redhat.com> (raw)
Hi Linus,
here's a set of fixes for how gfs2 handles page faults during read and
write syscalls. The patch queue is ready for merging except for two
open issues. May I ask you to shed some light or ask the right person
to help us out?
Right now, the filesystem can end up trying to take a lock it's already
holding, which causes it to BUG. We can recognize and mostly deal with
that simple case, but more complex scenarios exist which involve
multiple locks and / or incompatible locking modes. To handle those, we
switch to trylocks where necessary.
The patches appear to be working as itended, with the following
remaining questions:
(1) Jan Kara has pointed out [*] that returning VM_FAULT_SIGBUS from the
.fault / .page_mkwrite ops will raise SIGBUS, which would be visible to
user space. This hasn't been observed in our testing; instead, accesses
to the mmapped memory via call chains like:
iov_iter_fault_in_readable -> fault_in_pages_readable -> __get_user
would simply fail with -EFAULT, as we're expecting.
From looking at do_user_addr_fault and do_sigbus in arch/x86/mm/fault.c,
my impression is that VM_FAULT_SIGBUS will not cause SIGBUS to be raised
in kernel mode, and that we can rely on the -EFAULT behavior for
triggering the retries at the filesystem level.
(2) The patch queue introduces the same kind of trylock behavior for
both .fault (gfs2_fault) and .page_mkwrite (gfs2_page_mkwrite). I'm not
aware of any situation in which we can actually end up in .page_mkwrite
during a .read_iter or .write_iter operation, so the trylock behavior in
.page_mkwrite might be harmless but unnecessary.
Thank you very much,
Andreas
[*] https://listman.redhat.com/archives/cluster-devel/2021-May/msg00080.html
Previous posting of this patch queue:
https://listman.redhat.com/archives/cluster-devel/2021-May/msg00073.html
New xfstest for mmap + page faults during read / write:
https://lore.kernel.org/fstests/20210531152604.240462-1-agruenba@redhat.com/
Andreas Gruenbacher (9):
gfs2: Clean up the error handling in gfs2_page_mkwrite
gfs2: Add wrapper for iomap_file_buffered_write
gfs2: Add gfs2_holder_is_compatible helper
gfs2: Fix mmap + page fault deadlocks (part 1)
iov_iter: Add iov_iter_fault_in_writeable()
gfs2: Add wrappers for accessing journal_info
gfs2: Encode glock holding and retry flags in journal_info
gfs2: Add LM_FLAG_OUTER glock holder flag
gfs2: Fix mmap + page fault deadlocks (part 2)
fs/gfs2/aops.c | 6 +-
fs/gfs2/bmap.c | 31 ++++----
fs/gfs2/file.c | 175 +++++++++++++++++++++++++++++++++-----------
fs/gfs2/glock.c | 12 +++
fs/gfs2/glock.h | 27 ++++++-
fs/gfs2/incore.h | 41 +++++++++++
fs/gfs2/inode.c | 2 +-
fs/gfs2/log.c | 4 +-
fs/gfs2/lops.c | 2 +-
fs/gfs2/meta_io.c | 6 +-
fs/gfs2/super.c | 2 +-
fs/gfs2/trans.c | 16 ++--
include/linux/uio.h | 1 +
lib/iov_iter.c | 20 ++++-
14 files changed, 265 insertions(+), 80 deletions(-)
--
2.26.3
WARNING: multiple messages have this Message-ID (diff)
From: Andreas Gruenbacher <agruenba@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [RFC 0/9] gfs2: handle page faults during read and write
Date: Mon, 31 May 2021 19:01:14 +0200 [thread overview]
Message-ID: <20210531170123.243771-1-agruenba@redhat.com> (raw)
Hi Linus,
here's a set of fixes for how gfs2 handles page faults during read and
write syscalls. The patch queue is ready for merging except for two
open issues. May I ask you to shed some light or ask the right person
to help us out?
Right now, the filesystem can end up trying to take a lock it's already
holding, which causes it to BUG. We can recognize and mostly deal with
that simple case, but more complex scenarios exist which involve
multiple locks and / or incompatible locking modes. To handle those, we
switch to trylocks where necessary.
The patches appear to be working as itended, with the following
remaining questions:
(1) Jan Kara has pointed out [*] that returning VM_FAULT_SIGBUS from the
.fault / .page_mkwrite ops will raise SIGBUS, which would be visible to
user space. This hasn't been observed in our testing; instead, accesses
to the mmapped memory via call chains like:
iov_iter_fault_in_readable -> fault_in_pages_readable -> __get_user
would simply fail with -EFAULT, as we're expecting.
From looking at do_user_addr_fault and do_sigbus in arch/x86/mm/fault.c,
my impression is that VM_FAULT_SIGBUS will not cause SIGBUS to be raised
in kernel mode, and that we can rely on the -EFAULT behavior for
triggering the retries at the filesystem level.
(2) The patch queue introduces the same kind of trylock behavior for
both .fault (gfs2_fault) and .page_mkwrite (gfs2_page_mkwrite). I'm not
aware of any situation in which we can actually end up in .page_mkwrite
during a .read_iter or .write_iter operation, so the trylock behavior in
.page_mkwrite might be harmless but unnecessary.
Thank you very much,
Andreas
[*] https://listman.redhat.com/archives/cluster-devel/2021-May/msg00080.html
Previous posting of this patch queue:
https://listman.redhat.com/archives/cluster-devel/2021-May/msg00073.html
New xfstest for mmap + page faults during read / write:
https://lore.kernel.org/fstests/20210531152604.240462-1-agruenba at redhat.com/
Andreas Gruenbacher (9):
gfs2: Clean up the error handling in gfs2_page_mkwrite
gfs2: Add wrapper for iomap_file_buffered_write
gfs2: Add gfs2_holder_is_compatible helper
gfs2: Fix mmap + page fault deadlocks (part 1)
iov_iter: Add iov_iter_fault_in_writeable()
gfs2: Add wrappers for accessing journal_info
gfs2: Encode glock holding and retry flags in journal_info
gfs2: Add LM_FLAG_OUTER glock holder flag
gfs2: Fix mmap + page fault deadlocks (part 2)
fs/gfs2/aops.c | 6 +-
fs/gfs2/bmap.c | 31 ++++----
fs/gfs2/file.c | 175 +++++++++++++++++++++++++++++++++-----------
fs/gfs2/glock.c | 12 +++
fs/gfs2/glock.h | 27 ++++++-
fs/gfs2/incore.h | 41 +++++++++++
fs/gfs2/inode.c | 2 +-
fs/gfs2/log.c | 4 +-
fs/gfs2/lops.c | 2 +-
fs/gfs2/meta_io.c | 6 +-
fs/gfs2/super.c | 2 +-
fs/gfs2/trans.c | 16 ++--
include/linux/uio.h | 1 +
lib/iov_iter.c | 20 ++++-
14 files changed, 265 insertions(+), 80 deletions(-)
--
2.26.3
next reply other threads:[~2021-05-31 17:10 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-31 17:01 Andreas Gruenbacher [this message]
2021-05-31 17:01 ` [Cluster-devel] [RFC 0/9] gfs2: handle page faults during read and write Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 1/9] gfs2: Clean up the error handling in gfs2_page_mkwrite Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 2/9] gfs2: Add wrapper for iomap_file_buffered_write Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 3/9] gfs2: Add gfs2_holder_is_compatible helper Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 4/9] gfs2: Fix mmap + page fault deadlocks (part 1) Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-06-01 6:00 ` Linus Torvalds
2021-06-01 6:00 ` [Cluster-devel] " Linus Torvalds
2021-06-02 11:16 ` Andreas Gruenbacher
2021-06-02 11:16 ` [Cluster-devel] " Andreas Gruenbacher
2021-06-11 16:25 ` Al Viro
2021-06-11 16:25 ` [Cluster-devel] " Al Viro
2021-06-12 21:05 ` Al Viro
2021-06-12 21:05 ` [Cluster-devel] " Al Viro
2021-06-12 21:35 ` Al Viro
2021-06-12 21:35 ` [Cluster-devel] " Al Viro
2021-06-13 8:44 ` Steven Whitehouse
2021-06-13 8:44 ` Steven Whitehouse
2021-05-31 17:01 ` [RFC 5/9] iov_iter: Add iov_iter_fault_in_writeable() Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:12 ` Al Viro
2021-05-31 17:12 ` [Cluster-devel] " Al Viro
2021-06-12 21:12 ` Al Viro
2021-06-12 21:12 ` [Cluster-devel] " Al Viro
2021-06-12 21:33 ` Linus Torvalds
2021-06-12 21:33 ` [Cluster-devel] " Linus Torvalds
2021-06-12 21:47 ` Al Viro
2021-06-12 21:47 ` [Cluster-devel] " Al Viro
2021-06-12 23:17 ` Linus Torvalds
2021-06-12 23:17 ` [Cluster-devel] " Linus Torvalds
2021-06-12 23:38 ` Al Viro
2021-06-12 23:38 ` [Cluster-devel] " Al Viro
2021-05-31 17:01 ` [RFC 6/9] gfs2: Add wrappers for accessing journal_info Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 7/9] gfs2: Encode glock holding and retry flags in journal_info Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 8/9] gfs2: Add LM_FLAG_OUTER glock holder flag Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-05-31 17:01 ` [RFC 9/9] gfs2: Fix mmap + page fault deadlocks (part 2) Andreas Gruenbacher
2021-05-31 17:01 ` [Cluster-devel] " Andreas Gruenbacher
2021-06-01 5:47 ` Linus Torvalds
2021-06-01 5:47 ` [Cluster-devel] " Linus Torvalds
2021-05-31 17:57 ` [Cluster-devel] [RFC 0/9] gfs2: handle page faults during read and write Linus Torvalds
2021-05-31 20:35 ` Andreas Gruenbacher
2021-05-31 20:35 ` [Cluster-devel] " Andreas Gruenbacher
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=20210531170123.243771-1-agruenba@redhat.com \
--to=agruenba@redhat.com \
--cc=cluster-devel@redhat.com \
--cc=jack@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.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.