Tree/Branch: v4.20.14
Git describe: v4.20.14
Commit: bc3041f238 Linux 4.20.14
Build Time: 130 min 21 sec
Passed: 11 / 11 (100.00 %)
Failed: 0 / 11 ( 0.00 %)
Errors: 0
Warnings: 5
Section Mismatches: 0
-------------------------------------------------------------------------------
defconfigs with issues (other than build errors):
1 warnings 0 mismatches : x86_64-allmodconfig
2 warnings 0 mismatches : arm64-allmodconfig
1 warnings 0 mismatches : arm-multi_v7_defconfig
4 warnings 0 mismatches : arm-allmodconfig
1 warnings 0 mismatches : arm64-defconfig
-------------------------------------------------------------------------------
Warnings Summary: 5
5 ../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
1 ../drivers/staging/erofs/unzip_vle.c:185:29: warning: array subscript is above array bounds [-Warray-bounds]
1 ../drivers/scsi/myrs.c:821:24: warning: 'sshdr.sense_key' may be used uninitialized in this function [-Wmaybe-uninitialized]
1 ../drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
1 ../drivers/isdn/hardware/eicon/message.c:5985:1: warning: the frame size of 2064 bytes is larger than 2048 bytes [-Wframe-larger-than=]
===============================================================================
Detailed per-defconfig build reports below:
-------------------------------------------------------------------------------
x86_64-allmodconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings:
../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
-------------------------------------------------------------------------------
arm64-allmodconfig : PASS, 0 errors, 2 warnings, 0 section mismatches
Warnings:
../drivers/isdn/hardware/eicon/message.c:5985:1: warning: the frame size of 2064 bytes is larger than 2048 bytes [-Wframe-larger-than=]
../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
-------------------------------------------------------------------------------
arm-multi_v7_defconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings:
../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
-------------------------------------------------------------------------------
arm-allmodconfig : PASS, 0 errors, 4 warnings, 0 section mismatches
Warnings:
../drivers/net/ethernet/mellanox/mlx5/core/en_stats.c:216:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
../drivers/staging/erofs/unzip_vle.c:185:29: warning: array subscript is above array bounds [-Warray-bounds]
../drivers/scsi/myrs.c:821:24: warning: 'sshdr.sense_key' may be used uninitialized in this function [-Wmaybe-uninitialized]
-------------------------------------------------------------------------------
arm64-defconfig : PASS, 0 errors, 1 warnings, 0 section mismatches
Warnings:
../include/linux/spinlock.h:279:3: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
-------------------------------------------------------------------------------
Passed with no errors, warnings or mismatches:
arm64-allnoconfig
arm-multi_v5_defconfig
x86_64-defconfig
arm-allnoconfig
x86_64-allnoconfig
arm-multi_v4t_defconfig
A successful call to NOTIFY_RETRIEVE by filesystem carries promise from
the kernel to send back NOTIFY_REPLY message. However if the filesystem
is not reading requests with fuse_conn->max_pages capacity,
fuse_dev_do_read might see that the "request is too large" and decide to
"reply with an error and restart the read". "Reply with an error" has
underlying assumption that there is a "requester thread" that is waiting
for request completion, which is true for most requests, but is not true
for NOTIFY_REPLY: NOTIFY_RETRIEVE handler completes with OK status right
after it could successfully queue NOTIFY_REPLY message without waiting
for NOTIFY_REPLY completion. This leads to situation when filesystem
requested to retrieve inode data with NOTIFY_RETRIEVE, got err=OK for
that notification request, but NOTIFY_REPLY is not coming back.
More, since there is no "requester thread" to handle the error, the
situation shows itself as /sys/fs/fuse/connections/X/waiting=1 _and_
/dev/fuse read(s) queued. Which is misleading since NOTIFY_REPLY request
was removed from pending queue and abandoned.
One way to fix would be to change NOTIFY_RETRIEVE handler to wait until
queued NOTIFY_REPLY is actually read back to the server and only then
return NOTIFY_RETRIEVE status. However this is change in behaviour and
would require filesystems to have at least 2 threads. In particular a
single-threaded filesystem that was previously successfully using
NOTIFY_RETRIEVE would become stuck after the change. This way of fixing
is thus not acceptable.
However we can fix it another way - by always returning NOTIFY_REPLY
irregardless of its original size - with so much data as provided read
buffer could fit. This aligns with the way NOTIFY_RETRIEVE handler
works, which already unconditionally caps requested retrieve size to
fuse_conn->max_pages. This way it should not hurt NOTIFY_RETRIEVE
semantic if we return less data than was originally requested.
This fix requires another behaviour change however - to be sure that
read buffer has enough capacity to always fit fixed NOTIFY_REPLY part
plus at least some (0 or more) data, we have to precheck the buffer
before dequeuing and handling a request. And if the buffer is very small -
return EINVAL to read in filesystem with semantic that queued read was
invalid from the viewpoint of FUSE protocol. Even though this is also
behaviour change, this should not practically cause problems: 1d3d752b47
(fuse: clean up request size limit checking), which originally removed
such EINVAL return and reworked fuse_dev_do_read to loop and retry, also
added FUSE_MIN_READ_BUFFER=8K to user-visible fuse.h with comment that
"The read buffer is required to be at least 8k ..." Even though
FUSE_MIN_READ_BUFFER is not currently checked anywhere in the kernel,
libfuse always initializes session with bufsize=32·pages and, since its
beginning, (at least from 2005) issues a warning should user modify
fuse_session->bufsize directly to be sure that queued buffers are at
least as large as that sane minimum:
https://github.com/libfuse/libfuse/blob/fuse-3.3.0-22-g63d53ecc3a/lib/fuse_…https://github.com/libfuse/libfuse/blob/fuse-3.3.0-22-g63d53ecc3a/lib/fuse_…
(semantic added in https://github.com/libfuse/libfuse/commit/044da2e9e0)
This way we should be safe to add the check for minimum read buffer size.
I've hit this bug for real with my filesystem that is using
https://github.com/hanwen/go-fuse: there was no NOTIFY_REPLY after
successful NOTIFY_RETRIEVE and the filesystem was stuck waiting,
because FUSE protocol (definition scattered through many places) states
that NOTIFY_REPLY is guaranteed to come after successful NOTIFY_RETRIEVE
(see 2d45ba381a "fuse: add retrieve request"). After inspecting
/sys/fs/fuse/connections/X/waiting and seeing it was 1, I was initially
suspecting that it was user-space who is not issuing /dev/fuse reads and
NOTIFY_REPLY is there but stuck in kernel pending queue. However tracing
what is going on in /dev/fuse exchange and in both kernel and userspace
(see https://lab.nexedi.com/kirr/wendelin.core/blob/13d2d1f8/wcfs/fusetrace)
showed that there are correctly queued /dev/fuse reads still pending
after NOTIFY_RETRIEVE returns and it is the kernel who is not replying back:
...
P2 2.215710 /dev/fuse <- qread wcfs/11399_4_r:
syscall.Syscall+48
syscall.Read+73
github.com/hanwen/go-fuse/fuse.(*Server).readRequest.func1+85github.com/hanwen/go-fuse/fuse.handleEINTR+39github.com/hanwen/go-fuse/fuse.(*Server).readRequest+355github.com/hanwen/go-fuse/fuse.(*Server).loop+107
runtime.goexit+1
P2 2.215810 /dev/fuse -> read wcfs/11399_4_r:
.56 RELEASE i8 ... (ret=64)
P2 2.215859 /dev/fuse <- write wcfs/11399_5_w:
.56 (0) ...
syscall.Syscall+48
syscall.Write+73
github.com/hanwen/go-fuse/fuse.(*Server).systemWrite.func1+76github.com/hanwen/go-fuse/fuse.handleEINTR+39github.com/hanwen/go-fuse/fuse.(*Server).systemWrite+931github.com/hanwen/go-fuse/fuse.(*Server).write+194github.com/hanwen/go-fuse/fuse.(*Server).handleRequest+179github.com/hanwen/go-fuse/fuse.(*Server).loop+399
runtime.goexit+1
P2 2.215871 /dev/fuse -> write_ack wcfs/11399_5_w (ret=16)
P2 2.215876 /dev/fuse <- qread wcfs/11399_5_r: <-- NOTE
syscall.Syscall+48
syscall.Read+73
github.com/hanwen/go-fuse/fuse.(*Server).readRequest.func1+85github.com/hanwen/go-fuse/fuse.handleEINTR+39github.com/hanwen/go-fuse/fuse.(*Server).readRequest+355github.com/hanwen/go-fuse/fuse.(*Server).loop+107
runtime.goexit+1
P0 2.221527 /dev/fuse <- qread wcfs/11401_1_r: <-- NOTE
syscall.Syscall+48
syscall.Read+73
github.com/hanwen/go-fuse/fuse.(*Server).readRequest.func1+85github.com/hanwen/go-fuse/fuse.handleEINTR+39github.com/hanwen/go-fuse/fuse.(*Server).readRequest+355github.com/hanwen/go-fuse/fuse.(*Server).loop+107
runtime.goexit+1
P1 2.239384 /dev/fuse -> read wcfs/11398_6_r: # woken read that was queued before "..."
.57 READ i5 ... (ret=80)
P0 2.239626 /dev/fuse <- write wcfs/11397_0_w:
NOTIFY_RETRIEVE ...
syscall.Syscall+48
syscall.Write+73
github.com/hanwen/go-fuse/fuse.(*Server).systemWrite.func1+76github.com/hanwen/go-fuse/fuse.handleEINTR+39github.com/hanwen/go-fuse/fuse.(*Server).systemWrite+931github.com/hanwen/go-fuse/fuse.(*Server).write+194github.com/hanwen/go-fuse/fuse.(*Server).InodeRetrieveCache+764github.com/hanwen/go-fuse/fuse/nodefs.(*FileSystemConnector).FileRetrieveCa…
main.(*BigFile).invalidateBlk+232
main.(*Root).zδhandle1.func1+72
golang.org/x/sync/errgroup.(*Group).Go.func1+87
runtime.goexit+1
P0 2.239660 /dev/fuse -> write_ack wcfs/11397_0_w (ret=48)
# stuck
# (full trace: https://lab.nexedi.com/kirr/wendelin.core/commit/96416aaabd)
with queued / served read analysis confirming that two reads were indeed queued
and not served:
grep -w -e '<- qread\>' y.log |awk {'print $6'} |sort >qread.txt
grep -w -e '-> read\>' y.log |awk {'print $6'} |sort >read.txt
# xdiff qread.txt read.txt
diff --git a/qread.txt b/read.txt
index 4ab50d7..fdd2be1 100644
--- a/qread.txt
+++ b/read.txt
@@ -53,7 +53,5 @@ wcfs/11399_1_r:
wcfs/11399_2_r:
wcfs/11399_3_r:
wcfs/11399_4_r:
-wcfs/11399_5_r:
wcfs/11400_0_r:
wcfs/11401_0_r:
-wcfs/11401_1_r:
The bug was hit because go-fuse by default uses 64K for read buffer size
https://github.com/hanwen/go-fuse/blob/33711add/fuse/server.go#L142
and the kernel presets fuse_conn->max_pages to be 128K (= 32·4K pages).
Go-fuse will be likely fixed to both use bufsize=kernel's and to
correctly handle size > bufsize in InodeRetrieveCache. However we should
also fix the kernel to always deliver NOTIFY_REPLY once NOTIFY_RETRIEVE
was successful, so that FUSE protocol guarantee always holds
irregardless of whether userspace used default or other valid buffer
size setting, and so that filesystems can count not to get stuck waiting
for kernel who promised a reply.
This way this patch is here.
Signed-off-by: Kirill Smelkov <kirr(a)nexedi.com>
Cc: Han-Wen Nienhuys <hanwen(a)google.com>
Cc: Jakob Unterwurzacher <jakobunt(a)gmail.com>
Cc: <stable(a)vger.kernel.org> # v2.6.36+
---
First patch version was sent 1 week ago, but got no response:
https://marc.info/?l=linux-fsdevel&m=155000277921155&w=2
Changes since v1: don't forget to also update req->misc.retrieve_in.size
after truncation.
( This is my first patch to fs/fuse, so please forgive me if I missed anything. )
fs/fuse/dev.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 65 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 8a63e52785e9..93deb8e54d88 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -381,6 +381,40 @@ static void queue_request(struct fuse_iqueue *fiq, struct fuse_req *req)
kill_fasync(&fiq->fasync, SIGIO, POLL_IN);
}
+/*
+ * fuse_req_truncate_data truncates data in request that has paged data
+ * (req.in.argpages=1), so that whole request, when serialized, is <= nbytes.
+ *
+ * nbytes must be >= size(request without data).
+ */
+static void fuse_req_truncate_data(struct fuse_req *req, unsigned nbytes) {
+ unsigned size, n;
+
+ BUG_ON(!req->in.argpages);
+ BUG_ON(req->in.numargs < 1);
+
+ /* request size without data */
+ size = sizeof(struct fuse_in_header) +
+ len_args(req->in.numargs - 1, (struct fuse_arg *) req->in.args);
+ BUG_ON(nbytes < size);
+
+ /* truncate paged data */
+ for (n = 0; n < req->num_pages; n++) {
+ struct fuse_page_desc *p = &req->page_descs[n];
+
+ if (size >= nbytes) {
+ p->length = 0;
+ } else {
+ p->length = min_t(unsigned, p->length, nbytes - size);
+ }
+
+ size += p->length;
+ }
+
+ /* update whole request length in the header */
+ req->in.h.len = size;
+}
+
void fuse_queue_forget(struct fuse_conn *fc, struct fuse_forget_link *forget,
u64 nodeid, u64 nlookup)
{
@@ -1317,6 +1351,15 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
unsigned reqsize;
unsigned int hash;
+ /*
+ * Require sane minimum read buffer - that has capacity for fixed part
+ * of any request + some room for data. If the requirement is not
+ * satisfied return EINVAL to the filesystem without dequeueing /
+ * aborting any request.
+ */
+ if (nbytes < FUSE_MIN_READ_BUFFER)
+ return -EINVAL;
+
restart:
spin_lock(&fiq->waitq.lock);
err = -EAGAIN;
@@ -1358,12 +1401,28 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
/* If request is too large, reply with an error and restart the read */
if (nbytes < reqsize) {
- req->out.h.error = -EIO;
- /* SETXATTR is special, since it may contain too large data */
- if (in->h.opcode == FUSE_SETXATTR)
- req->out.h.error = -E2BIG;
- request_end(fc, req);
- goto restart;
+ switch (in->h.opcode) {
+ default:
+ req->out.h.error = -EIO;
+ /* SETXATTR is special, since it may contain too large data */
+ if (in->h.opcode == FUSE_SETXATTR)
+ req->out.h.error = -E2BIG;
+ request_end(fc, req);
+ goto restart;
+
+ /*
+ * NOTIFY_REPLY is special: if it was queued we already
+ * promised to filesystem to deliver it when handling
+ * NOTIFY_RETRIVE. We know that read buffer has capacity for at
+ * least some data. Truncate retrieved data to read buffer size
+ * and deliver it to stay to the promise.
+ */
+ case FUSE_NOTIFY_REPLY:
+ fuse_req_truncate_data(req, nbytes);
+ req->misc.retrieve_in.size -= reqsize - in->h.len;
+ reqsize = in->h.len;
+ }
+
}
spin_lock(&fpq->lock);
list_add(&req->list, &fpq->io);
--
2.21.0.rc0.269.g1a574e7a28
Hi Jason and Doug,
Here are some fixes that didn't quite make the boat for 5.0-rc. So we can
go ahead and send them to -next. The third patch here is really a v2 of:
https://patchwork.kernel.org/patch/10769005/
---
Michael J. Ruhl (2):
IB/rdmavt: Fix concurrency panics in QP post_send and modify to error
IB/hfi1: Close race condition on user context disable and close
Mike Marciniszyn (1):
IB/rdmavt: Fix loopback send with invalidate ordering
drivers/infiniband/hw/hfi1/hfi.h | 2 +
drivers/infiniband/hw/hfi1/init.c | 14 ++++++---
drivers/infiniband/sw/rdmavt/qp.c | 59 ++++++++++++++++++++++++-------------
3 files changed, 49 insertions(+), 26 deletions(-)
--
-Denny