| Message ID | 20240129094924.1221977-3-dhowells@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | netfs: Miscellaneous fixes | expand |
On Mon, 2024-01-29 at 09:49 +0000, David Howells wrote: > Fix netfs_unbuffered_write_iter() to return immediately if > generic_write_checks() returns 0, indicating there's nothing to write. > Note that netfs_file_write_iter() already does this. > > Also, whilst we're at it, put in checks for the size being zero before we > even take the locks. Note that generic_write_checks() can still reduce the > size to zero, so we still need that check. > > Without this, a warning similar to the following is logged to dmesg: > > netfs: Zero-sized write [R=1b6da] > > and the syscall fails with EIO, e.g.: > > /sbin/ldconfig.real: Writing of cache extension data failed: Input/output error > > This can be reproduced on 9p by: > > xfs_io -f -c 'pwrite 0 0' /xfstest.test/foo > > Fixes: 153a9961b551 ("netfs: Implement unbuffered/DIO write support") > Reported-by: Eric Van Hensbergen <ericvh@kernel.org> > Link: https://lore.kernel.org/r/ZbQUU6QKmIftKsmo@FV7GG9FTHL/ > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Dominique Martinet <asmadeus@codewreck.org> > cc: Jeff Layton <jlayton@kernel.org> > cc: v9fs@lists.linux.dev > cc: linux_oss@crudebyte.com > cc: netfs@lists.linux.dev > cc: linux-fsdevel@vger.kernel.org > --- > fs/netfs/buffered_write.c | 3 +++ > fs/netfs/direct_write.c | 5 ++++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c > index a3059b3168fd..9a0d32e4b422 100644 > --- a/fs/netfs/buffered_write.c > +++ b/fs/netfs/buffered_write.c > @@ -477,6 +477,9 @@ ssize_t netfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) > > _enter("%llx,%zx,%llx", iocb->ki_pos, iov_iter_count(from), i_size_read(inode)); > > + if (!iov_iter_count(from)) > + return 0; > + > if ((iocb->ki_flags & IOCB_DIRECT) || > test_bit(NETFS_ICTX_UNBUFFERED, &ictx->flags)) > return netfs_unbuffered_write_iter(iocb, from); > diff --git a/fs/netfs/direct_write.c b/fs/netfs/direct_write.c > index 60a40d293c87..bee047e20f5d 100644 > --- a/fs/netfs/direct_write.c > +++ b/fs/netfs/direct_write.c > @@ -139,6 +139,9 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from) > > _enter("%llx,%zx,%llx", iocb->ki_pos, iov_iter_count(from), i_size_read(inode)); > > + if (!iov_iter_count(from)) > + return 0; > + > trace_netfs_write_iter(iocb, from); > netfs_stat(&netfs_n_rh_dio_write); > > @@ -146,7 +149,7 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from) > if (ret < 0) > return ret; > ret = generic_write_checks(iocb, from); > - if (ret < 0) > + if (ret <= 0) > goto out; > ret = file_remove_privs(file); > if (ret < 0) > Reviewed-by: Jeff Layton <jlayton@kernel.org>
David Howells wrote on Mon, Jan 29, 2024 at 09:49:19AM +0000: > Fix netfs_unbuffered_write_iter() to return immediately if > generic_write_checks() returns 0, indicating there's nothing to write. > Note that netfs_file_write_iter() already does this. > > Also, whilst we're at it, put in checks for the size being zero before we > even take the locks. Note that generic_write_checks() can still reduce the > size to zero, so we still need that check. > > Without this, a warning similar to the following is logged to dmesg: > > netfs: Zero-sized write [R=1b6da] > > and the syscall fails with EIO, e.g.: > > /sbin/ldconfig.real: Writing of cache extension data failed: Input/output error > > This can be reproduced on 9p by: > > xfs_io -f -c 'pwrite 0 0' /xfstest.test/foo > > Fixes: 153a9961b551 ("netfs: Implement unbuffered/DIO write support") > Reported-by: Eric Van Hensbergen <ericvh@kernel.org> > Link: https://lore.kernel.org/r/ZbQUU6QKmIftKsmo@FV7GG9FTHL/ > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Dominique Martinet <asmadeus@codewreck.org> Thanks! Tested-by: Dominique Martinet <asmadeus@codewreck.org>
On 29.01.24 10:49, David Howells wrote: > Fix netfs_unbuffered_write_iter() to return immediately if > generic_write_checks() returns 0, indicating there's nothing to write. > Note that netfs_file_write_iter() already does this. > > Also, whilst we're at it, put in checks for the size being zero before we > even take the locks. Note that generic_write_checks() can still reduce the > size to zero, so we still need that check. > > Without this, a warning similar to the following is logged to dmesg: > > netfs: Zero-sized write [R=1b6da] > > and the syscall fails with EIO, e.g.: > > /sbin/ldconfig.real: Writing of cache extension data failed: Input/output error > > This can be reproduced on 9p by: > > xfs_io -f -c 'pwrite 0 0' /xfstest.test/foo > > Fixes: 153a9961b551 ("netfs: Implement unbuffered/DIO write support") > Reported-by: Eric Van Hensbergen <ericvh@kernel.org> > Link: https://lore.kernel.org/r/ZbQUU6QKmIftKsmo@FV7GG9FTHL/ David, thx for fixing Eric's regression, which I'm tracking. Christian, just wondering: that patch afaics is sitting in vfs.netfs for about three weeks now -- is that intentional or did it maybe fell through the cracks somehow? > [...] Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
On Mon, Feb 19, 2024 at 09:38:33AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote: > On 29.01.24 10:49, David Howells wrote: > > Fix netfs_unbuffered_write_iter() to return immediately if > > generic_write_checks() returns 0, indicating there's nothing to write. > > Note that netfs_file_write_iter() already does this. > > > > Also, whilst we're at it, put in checks for the size being zero before we > > even take the locks. Note that generic_write_checks() can still reduce the > > size to zero, so we still need that check. > > > > Without this, a warning similar to the following is logged to dmesg: > > > > netfs: Zero-sized write [R=1b6da] > > > > and the syscall fails with EIO, e.g.: > > > > /sbin/ldconfig.real: Writing of cache extension data failed: Input/output error > > > > This can be reproduced on 9p by: > > > > xfs_io -f -c 'pwrite 0 0' /xfstest.test/foo > > > > Fixes: 153a9961b551 ("netfs: Implement unbuffered/DIO write support") > > Reported-by: Eric Van Hensbergen <ericvh@kernel.org> > > Link: https://lore.kernel.org/r/ZbQUU6QKmIftKsmo@FV7GG9FTHL/ > > David, thx for fixing Eric's regression, which I'm tracking. > > Christian, just wondering: that patch afaics is sitting in vfs.netfs for > about three weeks now -- is that intentional or did it maybe fell > through the cracks somehow? I've moved it to vfs.fixes now and will send later this week. Thanks for the reminder!
diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c index a3059b3168fd..9a0d32e4b422 100644 --- a/fs/netfs/buffered_write.c +++ b/fs/netfs/buffered_write.c @@ -477,6 +477,9 @@ ssize_t netfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) _enter("%llx,%zx,%llx", iocb->ki_pos, iov_iter_count(from), i_size_read(inode)); + if (!iov_iter_count(from)) + return 0; + if ((iocb->ki_flags & IOCB_DIRECT) || test_bit(NETFS_ICTX_UNBUFFERED, &ictx->flags)) return netfs_unbuffered_write_iter(iocb, from); diff --git a/fs/netfs/direct_write.c b/fs/netfs/direct_write.c index 60a40d293c87..bee047e20f5d 100644 --- a/fs/netfs/direct_write.c +++ b/fs/netfs/direct_write.c @@ -139,6 +139,9 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from) _enter("%llx,%zx,%llx", iocb->ki_pos, iov_iter_count(from), i_size_read(inode)); + if (!iov_iter_count(from)) + return 0; + trace_netfs_write_iter(iocb, from); netfs_stat(&netfs_n_rh_dio_write); @@ -146,7 +149,7 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from) if (ret < 0) return ret; ret = generic_write_checks(iocb, from); - if (ret < 0) + if (ret <= 0) goto out; ret = file_remove_privs(file); if (ret < 0)
Fix netfs_unbuffered_write_iter() to return immediately if generic_write_checks() returns 0, indicating there's nothing to write. Note that netfs_file_write_iter() already does this. Also, whilst we're at it, put in checks for the size being zero before we even take the locks. Note that generic_write_checks() can still reduce the size to zero, so we still need that check. Without this, a warning similar to the following is logged to dmesg: netfs: Zero-sized write [R=1b6da] and the syscall fails with EIO, e.g.: /sbin/ldconfig.real: Writing of cache extension data failed: Input/output error This can be reproduced on 9p by: xfs_io -f -c 'pwrite 0 0' /xfstest.test/foo Fixes: 153a9961b551 ("netfs: Implement unbuffered/DIO write support") Reported-by: Eric Van Hensbergen <ericvh@kernel.org> Link: https://lore.kernel.org/r/ZbQUU6QKmIftKsmo@FV7GG9FTHL/ Signed-off-by: David Howells <dhowells@redhat.com> cc: Dominique Martinet <asmadeus@codewreck.org> cc: Jeff Layton <jlayton@kernel.org> cc: v9fs@lists.linux.dev cc: linux_oss@crudebyte.com cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org --- fs/netfs/buffered_write.c | 3 +++ fs/netfs/direct_write.c | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-)