Hi Christian, Jeff, Gao,
Here are some additional patches for my netfs-lib tree:
(1) Mark netfs_unbuffered_write_iter_locked() static as it's only used in
the file in which it is defined.
(2) Display a counter for DIO writes in /proc/fs/netfs/stats.
(3) Fix the interaction between write-streaming (dirty data in
non-uptodate pages) and the culling of a cache file trying to write
that to the cache.
(4) Fix the loop that unmarks folios after writing to the cache. The
xarray iterator only advances the index by 1, so if we unmarked a
multipage folio and that got split before we advance to the next
folio, we see a repeat of a fragment of the folio.
(5) Fix a mixup with signed/unsigned offsets when prepping for writing to
the cache that leads to missing error detection.
(6) Fix a wrong ifdef hiding a wait.
David
The netfslib postings:
Link: https://lore.kernel.org/r/[email protected]/ # v1
Link: https://lore.kernel.org/r/[email protected]/ # v2
Link: https://lore.kernel.org/r/[email protected]/ # v3
Link: https://lore.kernel.org/r/[email protected]/ # v4
Link: https://lore.kernel.org/r/[email protected]/ # v5
Link: https://lore.kernel.org/r/[email protected]/ # added patches
David Howells (6):
netfs: Mark netfs_unbuffered_write_iter_locked() static
netfs: Count DIO writes
netfs: Fix interaction between write-streaming and cachefiles culling
netfs: Fix the loop that unmarks folios after writing to the cache
cachefiles: Fix signed/unsigned mixup
netfs: Fix wrong #ifdef hiding wait
fs/cachefiles/io.c | 18 +++++++++---------
fs/netfs/buffered_write.c | 27 ++++++++++++++++++++++-----
fs/netfs/direct_write.c | 5 +++--
fs/netfs/fscache_stats.c | 9 ++++++---
fs/netfs/internal.h | 8 ++------
fs/netfs/io.c | 2 +-
fs/netfs/stats.c | 13 +++++++++----
include/linux/fscache-cache.h | 3 +++
include/linux/netfs.h | 1 +
9 files changed, 56 insertions(+), 30 deletions(-)
Provide a counter for DIO writes to match that for DIO reads.
Signed-off-by: David Howells <[email protected]>
cc: Jeff Layton <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
fs/netfs/direct_write.c | 1 +
fs/netfs/internal.h | 1 +
fs/netfs/stats.c | 11 +++++++----
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/netfs/direct_write.c b/fs/netfs/direct_write.c
index b9cbfd6a8a01..60a40d293c87 100644
--- a/fs/netfs/direct_write.c
+++ b/fs/netfs/direct_write.c
@@ -140,6 +140,7 @@ 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));
trace_netfs_write_iter(iocb, from);
+ netfs_stat(&netfs_n_rh_dio_write);
ret = netfs_start_io_direct(inode);
if (ret < 0)
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index a6dfc8888377..3f9620d0fa63 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -104,6 +104,7 @@ int netfs_end_writethrough(struct netfs_io_request *wreq, struct kiocb *iocb);
*/
#ifdef CONFIG_NETFS_STATS
extern atomic_t netfs_n_rh_dio_read;
+extern atomic_t netfs_n_rh_dio_write;
extern atomic_t netfs_n_rh_readahead;
extern atomic_t netfs_n_rh_readpage;
extern atomic_t netfs_n_rh_rreq;
diff --git a/fs/netfs/stats.c b/fs/netfs/stats.c
index 15fd5c3f0f39..42db36528d92 100644
--- a/fs/netfs/stats.c
+++ b/fs/netfs/stats.c
@@ -10,6 +10,7 @@
#include "internal.h"
atomic_t netfs_n_rh_dio_read;
+atomic_t netfs_n_rh_dio_write;
atomic_t netfs_n_rh_readahead;
atomic_t netfs_n_rh_readpage;
atomic_t netfs_n_rh_rreq;
@@ -37,14 +38,13 @@ atomic_t netfs_n_wh_write_failed;
int netfs_stats_show(struct seq_file *m, void *v)
{
- seq_printf(m, "Netfs : DR=%u RA=%u RP=%u WB=%u WBZ=%u rr=%u sr=%u\n",
+ seq_printf(m, "Netfs : DR=%u DW=%u RA=%u RP=%u WB=%u WBZ=%u\n",
atomic_read(&netfs_n_rh_dio_read),
+ atomic_read(&netfs_n_rh_dio_write),
atomic_read(&netfs_n_rh_readahead),
atomic_read(&netfs_n_rh_readpage),
atomic_read(&netfs_n_rh_write_begin),
- atomic_read(&netfs_n_rh_write_zskip),
- atomic_read(&netfs_n_rh_rreq),
- atomic_read(&netfs_n_rh_sreq));
+ atomic_read(&netfs_n_rh_write_zskip));
seq_printf(m, "Netfs : ZR=%u sh=%u sk=%u\n",
atomic_read(&netfs_n_rh_zero),
atomic_read(&netfs_n_rh_short_read),
@@ -66,6 +66,9 @@ int netfs_stats_show(struct seq_file *m, void *v)
atomic_read(&netfs_n_wh_write),
atomic_read(&netfs_n_wh_write_done),
atomic_read(&netfs_n_wh_write_failed));
+ seq_printf(m, "Netfs : rr=%u sr=%u\n",
+ atomic_read(&netfs_n_rh_rreq),
+ atomic_read(&netfs_n_rh_sreq));
return fscache_stats_show(m);
}
EXPORT_SYMBOL(netfs_stats_show);
In __cachefiles_prepare_write(), the start and pos variables were made
unsigned 64-bit so that the casts in the checking could be got rid of -
which should be fine since absolute file offsets can't be negative, except
that an error code may be obtained from vfs_llseek(), which *would* be
negative. This breaks the error check.
Fix this for now by reverting pos and start to be signed and putting back
the casts. Unfortunately, the error value checks cannot be replaced with
IS_ERR_VALUE() as long might be 32-bits.
Fixes: 7097c96411d2 ("cachefiles: Fix __cachefiles_prepare_write()")
Reported-by: Simon Horman <[email protected]>
Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
cc: Gao Xiang <[email protected]>
cc: Yiqun Leng <[email protected]>
cc: Jia Zhu <[email protected]>
cc: Jeff Layton <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
fs/cachefiles/io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index 3eec26967437..9a2cb2868e90 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -522,7 +522,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
bool no_space_allocated_yet)
{
struct cachefiles_cache *cache = object->volume->cache;
- unsigned long long start = *_start, pos;
+ loff_t start = *_start, pos;
size_t len = *_len;
int ret;
@@ -556,7 +556,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
cachefiles_trace_seek_error);
return pos;
}
- if (pos >= start + *_len)
+ if ((u64)pos >= (u64)start + *_len)
goto check_space; /* Unallocated region */
/* We have a block that's at least partially filled - if we're low on
@@ -575,7 +575,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
cachefiles_trace_seek_error);
return pos;
}
- if (pos >= start + *_len)
+ if ((u64)pos >= (u64)start + *_len)
return 0; /* Fully allocated */
/* Partially allocated, but insufficient space: cull. */
In the loop in netfs_rreq_unmark_after_write() that removes the PG_fscache
from folios after they've been written to the cache, as soon as we remove
the mark from a multipage folio, it can get split - and then we might see a
fragment of folio again.
Guard against this by advancing the 'unlocked' tracker to the index of the
last page in the folio to avoid a double removal of the PG_fscache mark.
Reported-by: Marc Dionne <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Matthew Wilcox <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
fs/netfs/buffered_write.c | 1 +
fs/netfs/io.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/netfs/buffered_write.c b/fs/netfs/buffered_write.c
index 6cd8f7422e9a..0b2b7a60dabc 100644
--- a/fs/netfs/buffered_write.c
+++ b/fs/netfs/buffered_write.c
@@ -698,6 +698,7 @@ static void netfs_pages_written_back(struct netfs_io_request *wreq)
end_wb:
if (folio_test_fscache(folio))
folio_end_fscache(folio);
+ xas_advance(&xas, folio_next_index(folio) - 1);
folio_end_writeback(folio);
}
diff --git a/fs/netfs/io.c b/fs/netfs/io.c
index 5b5af96cd4b9..4309edf33862 100644
--- a/fs/netfs/io.c
+++ b/fs/netfs/io.c
@@ -126,7 +126,7 @@ static void netfs_rreq_unmark_after_write(struct netfs_io_request *rreq,
*/
if (have_unlocked && folio_index(folio) <= unlocked)
continue;
- unlocked = folio_index(folio);
+ unlocked = folio_next_index(folio) - 1;
trace_netfs_folio(folio, netfs_folio_trace_end_copy);
folio_end_fscache(folio);
have_unlocked = true;