* [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
@ 2023-10-13 15:56 ` David Howells
2023-10-16 14:44 ` Jeff Layton
2023-10-13 15:56 ` [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data David Howells
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: David Howells @ 2023-10-13 15:56 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Dominique Martinet,
Ilya Dryomov, Christian Brauner, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm, netdev,
linux-kernel, linux-cachefs
Add a procfile, /proc/fs/netfs/requests, to list in-progress netfslib I/O
requests.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/netfs/internal.h | 22 +++++++++++
fs/netfs/main.c | 91 +++++++++++++++++++++++++++++++++++++++++++
fs/netfs/objects.c | 4 +-
include/linux/netfs.h | 6 ++-
4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index 43fac1b14e40..1f067aa96c50 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -29,6 +29,28 @@ int netfs_begin_read(struct netfs_io_request *rreq, bool sync);
* main.c
*/
extern unsigned int netfs_debug;
+extern struct list_head netfs_io_requests;
+extern spinlock_t netfs_proc_lock;
+
+#ifdef CONFIG_PROC_FS
+static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq)
+{
+ spin_lock(&netfs_proc_lock);
+ list_add_tail_rcu(&rreq->proc_link, &netfs_io_requests);
+ spin_unlock(&netfs_proc_lock);
+}
+static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq)
+{
+ if (!list_empty(&rreq->proc_link)) {
+ spin_lock(&netfs_proc_lock);
+ list_del_rcu(&rreq->proc_link);
+ spin_unlock(&netfs_proc_lock);
+ }
+}
+#else
+static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq) {}
+static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq) {}
+#endif
/*
* objects.c
diff --git a/fs/netfs/main.c b/fs/netfs/main.c
index 068568702957..21f814eee6af 100644
--- a/fs/netfs/main.c
+++ b/fs/netfs/main.c
@@ -7,6 +7,8 @@
#include <linux/module.h>
#include <linux/export.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
#include "internal.h"
#define CREATE_TRACE_POINTS
#include <trace/events/netfs.h>
@@ -18,3 +20,92 @@ MODULE_LICENSE("GPL");
unsigned netfs_debug;
module_param_named(debug, netfs_debug, uint, S_IWUSR | S_IRUGO);
MODULE_PARM_DESC(netfs_debug, "Netfs support debugging mask");
+
+#ifdef CONFIG_PROC_FS
+LIST_HEAD(netfs_io_requests);
+DEFINE_SPINLOCK(netfs_proc_lock);
+
+static const char *netfs_origins[] = {
+ [NETFS_READAHEAD] = "RA",
+ [NETFS_READPAGE] = "RP",
+ [NETFS_READ_FOR_WRITE] = "RW",
+};
+
+/*
+ * Generate a list of I/O requests in /proc/fs/netfs/requests
+ */
+static int netfs_requests_seq_show(struct seq_file *m, void *v)
+{
+ struct netfs_io_request *rreq;
+
+ if (v == &netfs_io_requests) {
+ seq_puts(m,
+ "REQUEST OR REF FL ERR OPS COVERAGE\n"
+ "======== == === == ==== === =========\n"
+ );
+ return 0;
+ }
+
+ rreq = list_entry(v, struct netfs_io_request, proc_link);
+ seq_printf(m,
+ "%08x %s %3d %2lx %4d %3d @%04llx %zx/%zx",
+ rreq->debug_id,
+ netfs_origins[rreq->origin],
+ refcount_read(&rreq->ref),
+ rreq->flags,
+ rreq->error,
+ atomic_read(&rreq->nr_outstanding),
+ rreq->start, rreq->submitted, rreq->len);
+ seq_putc(m, '\n');
+ return 0;
+}
+
+static void *netfs_requests_seq_start(struct seq_file *m, loff_t *_pos)
+ __acquires(rcu)
+{
+ rcu_read_lock();
+ return seq_list_start_head(&netfs_io_requests, *_pos);
+}
+
+static void *netfs_requests_seq_next(struct seq_file *m, void *v, loff_t *_pos)
+{
+ return seq_list_next(v, &netfs_io_requests, _pos);
+}
+
+static void netfs_requests_seq_stop(struct seq_file *m, void *v)
+ __releases(rcu)
+{
+ rcu_read_unlock();
+}
+
+static const struct seq_operations netfs_requests_seq_ops = {
+ .start = netfs_requests_seq_start,
+ .next = netfs_requests_seq_next,
+ .stop = netfs_requests_seq_stop,
+ .show = netfs_requests_seq_show,
+};
+#endif /* CONFIG_PROC_FS */
+
+static int __init netfs_init(void)
+{
+ if (!proc_mkdir("fs/netfs", NULL))
+ goto error;
+
+ if (!proc_create_seq("fs/netfs/requests", S_IFREG | 0444, NULL,
+ &netfs_requests_seq_ops))
+ goto error_proc;
+
+ return 0;
+
+error_proc:
+ remove_proc_entry("fs/netfs", NULL);
+error:
+ return -ENOMEM;
+}
+fs_initcall(netfs_init);
+
+static void __exit netfs_exit(void)
+{
+ remove_proc_entry("fs/netfs", NULL);
+}
+module_exit(netfs_exit);
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index e17cdf53f6a7..85f428fc52e6 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -45,6 +45,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
}
}
+ netfs_proc_add_rreq(rreq);
netfs_stat(&netfs_n_rh_rreq);
return rreq;
}
@@ -76,12 +77,13 @@ static void netfs_free_request(struct work_struct *work)
container_of(work, struct netfs_io_request, work);
trace_netfs_rreq(rreq, netfs_rreq_trace_free);
+ netfs_proc_del_rreq(rreq);
netfs_clear_subrequests(rreq, false);
if (rreq->netfs_ops->free_request)
rreq->netfs_ops->free_request(rreq);
if (rreq->cache_resources.ops)
rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
- kfree(rreq);
+ kfree_rcu(rreq, rcu);
netfs_stat_d(&netfs_n_rh_rreq);
}
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b11a84f6c32b..b447cb67f599 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -175,10 +175,14 @@ enum netfs_io_origin {
* operations to a variety of data stores and then stitch the result together.
*/
struct netfs_io_request {
- struct work_struct work;
+ union {
+ struct work_struct work;
+ struct rcu_head rcu;
+ };
struct inode *inode; /* The file being accessed */
struct address_space *mapping; /* The mapping being accessed */
struct netfs_cache_resources cache_resources;
+ struct list_head proc_link; /* Link in netfs_iorequests */
struct list_head subrequests; /* Contributory I/O operations */
void *netfs_priv; /* Private data for the netfs */
unsigned int debug_id;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests
2023-10-13 15:56 ` [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests David Howells
@ 2023-10-16 14:44 ` Jeff Layton
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2023-10-16 14:44 UTC (permalink / raw)
To: David Howells, Steve French
Cc: Matthew Wilcox, Marc Dionne, Paulo Alcantara, Ronnie Sahlberg,
Shyam Prasad N, Tom Talpey, Dominique Martinet, Ilya Dryomov,
Christian Brauner, linux-afs, linux-cifs, linux-nfs, ceph-devel,
v9fs, linux-fsdevel, linux-mm, netdev, linux-kernel,
linux-cachefs
On Fri, 2023-10-13 at 16:56 +0100, David Howells wrote:
> Add a procfile, /proc/fs/netfs/requests, to list in-progress netfslib I/O
> requests.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: linux-cachefs@redhat.com
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-mm@kvack.org
> ---
> fs/netfs/internal.h | 22 +++++++++++
> fs/netfs/main.c | 91 +++++++++++++++++++++++++++++++++++++++++++
> fs/netfs/objects.c | 4 +-
> include/linux/netfs.h | 6 ++-
> 4 files changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
> index 43fac1b14e40..1f067aa96c50 100644
> --- a/fs/netfs/internal.h
> +++ b/fs/netfs/internal.h
> @@ -29,6 +29,28 @@ int netfs_begin_read(struct netfs_io_request *rreq, bool sync);
> * main.c
> */
> extern unsigned int netfs_debug;
> +extern struct list_head netfs_io_requests;
> +extern spinlock_t netfs_proc_lock;
> +
> +#ifdef CONFIG_PROC_FS
> +static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq)
> +{
> + spin_lock(&netfs_proc_lock);
> + list_add_tail_rcu(&rreq->proc_link, &netfs_io_requests);
> + spin_unlock(&netfs_proc_lock);
> +}
> +static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq)
> +{
> + if (!list_empty(&rreq->proc_link)) {
> + spin_lock(&netfs_proc_lock);
> + list_del_rcu(&rreq->proc_link);
> + spin_unlock(&netfs_proc_lock);
> + }
> +}
> +#else
> +static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq) {}
> +static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq) {}
> +#endif
>
> /*
> * objects.c
> diff --git a/fs/netfs/main.c b/fs/netfs/main.c
> index 068568702957..21f814eee6af 100644
> --- a/fs/netfs/main.c
> +++ b/fs/netfs/main.c
> @@ -7,6 +7,8 @@
>
> #include <linux/module.h>
> #include <linux/export.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> #include "internal.h"
> #define CREATE_TRACE_POINTS
> #include <trace/events/netfs.h>
> @@ -18,3 +20,92 @@ MODULE_LICENSE("GPL");
> unsigned netfs_debug;
> module_param_named(debug, netfs_debug, uint, S_IWUSR | S_IRUGO);
> MODULE_PARM_DESC(netfs_debug, "Netfs support debugging mask");
> +
> +#ifdef CONFIG_PROC_FS
> +LIST_HEAD(netfs_io_requests);
> +DEFINE_SPINLOCK(netfs_proc_lock);
> +
> +static const char *netfs_origins[] = {
> + [NETFS_READAHEAD] = "RA",
> + [NETFS_READPAGE] = "RP",
> + [NETFS_READ_FOR_WRITE] = "RW",
> +};
> +
> +/*
> + * Generate a list of I/O requests in /proc/fs/netfs/requests
> + */
> +static int netfs_requests_seq_show(struct seq_file *m, void *v)
> +{
> + struct netfs_io_request *rreq;
> +
> + if (v == &netfs_io_requests) {
> + seq_puts(m,
> + "REQUEST OR REF FL ERR OPS COVERAGE\n"
> + "======== == === == ==== === =========\n"
> + );
> + return 0;
> + }
> +
> + rreq = list_entry(v, struct netfs_io_request, proc_link);
> + seq_printf(m,
> + "%08x %s %3d %2lx %4d %3d @%04llx %zx/%zx",
> + rreq->debug_id,
> + netfs_origins[rreq->origin],
> + refcount_read(&rreq->ref),
> + rreq->flags,
> + rreq->error,
> + atomic_read(&rreq->nr_outstanding),
> + rreq->start, rreq->submitted, rreq->len);
> + seq_putc(m, '\n');
> + return 0;
> +}
> +
> +static void *netfs_requests_seq_start(struct seq_file *m, loff_t *_pos)
> + __acquires(rcu)
> +{
> + rcu_read_lock();
> + return seq_list_start_head(&netfs_io_requests, *_pos);
> +}
> +
> +static void *netfs_requests_seq_next(struct seq_file *m, void *v, loff_t *_pos)
> +{
> + return seq_list_next(v, &netfs_io_requests, _pos);
> +}
> +
> +static void netfs_requests_seq_stop(struct seq_file *m, void *v)
> + __releases(rcu)
> +{
> + rcu_read_unlock();
> +}
> +
> +static const struct seq_operations netfs_requests_seq_ops = {
> + .start = netfs_requests_seq_start,
> + .next = netfs_requests_seq_next,
> + .stop = netfs_requests_seq_stop,
> + .show = netfs_requests_seq_show,
> +};
> +#endif /* CONFIG_PROC_FS */
> +
> +static int __init netfs_init(void)
> +{
> + if (!proc_mkdir("fs/netfs", NULL))
> + goto error;
> +
It seems like this should go under debugfs instead.
> + if (!proc_create_seq("fs/netfs/requests", S_IFREG | 0444, NULL,
> + &netfs_requests_seq_ops))
> + goto error_proc;
> +
> + return 0;
> +
> +error_proc:
> + remove_proc_entry("fs/netfs", NULL);
> +error:
> + return -ENOMEM;
> +}
> +fs_initcall(netfs_init);
> +
> +static void __exit netfs_exit(void)
> +{
> + remove_proc_entry("fs/netfs", NULL);
> +}
> +module_exit(netfs_exit);
> diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
> index e17cdf53f6a7..85f428fc52e6 100644
> --- a/fs/netfs/objects.c
> +++ b/fs/netfs/objects.c
> @@ -45,6 +45,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
> }
> }
>
> + netfs_proc_add_rreq(rreq);
> netfs_stat(&netfs_n_rh_rreq);
> return rreq;
> }
> @@ -76,12 +77,13 @@ static void netfs_free_request(struct work_struct *work)
> container_of(work, struct netfs_io_request, work);
>
> trace_netfs_rreq(rreq, netfs_rreq_trace_free);
> + netfs_proc_del_rreq(rreq);
> netfs_clear_subrequests(rreq, false);
> if (rreq->netfs_ops->free_request)
> rreq->netfs_ops->free_request(rreq);
> if (rreq->cache_resources.ops)
> rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
> - kfree(rreq);
> + kfree_rcu(rreq, rcu);
> netfs_stat_d(&netfs_n_rh_rreq);
> }
>
> diff --git a/include/linux/netfs.h b/include/linux/netfs.h
> index b11a84f6c32b..b447cb67f599 100644
> --- a/include/linux/netfs.h
> +++ b/include/linux/netfs.h
> @@ -175,10 +175,14 @@ enum netfs_io_origin {
> * operations to a variety of data stores and then stitch the result together.
> */
> struct netfs_io_request {
> - struct work_struct work;
> + union {
> + struct work_struct work;
> + struct rcu_head rcu;
> + };
> struct inode *inode; /* The file being accessed */
> struct address_space *mapping; /* The mapping being accessed */
> struct netfs_cache_resources cache_resources;
> + struct list_head proc_link; /* Link in netfs_iorequests */
> struct list_head subrequests; /* Contributory I/O operations */
> void *netfs_priv; /* Private data for the netfs */
> unsigned int debug_id;
>
ACK on the general concept however. This is useful debugging info.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
2023-10-13 15:56 ` [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests David Howells
@ 2023-10-13 15:56 ` David Howells
2023-10-16 15:43 ` Jeff Layton
2023-10-16 16:31 ` David Howells
2023-10-13 15:56 ` [RFC PATCH 03/53] netfs: Note nonblockingness in the netfs_io_request struct David Howells
` (3 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: David Howells @ 2023-10-13 15:56 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Dominique Martinet,
Ilya Dryomov, Christian Brauner, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm, netdev,
linux-kernel, linux-cachefs
Track the file position above which the server is not expected to have any
data and preemptively assume that we can simply fill blocks with zeroes
locally rather than attempting to download them - even if we've written
data back to the server. Assume that any data that was written back above
that position is held in the local cache. Call this the "zero point".
Make use of this to optimise away some reads from the server. We need to
set the zero point in the following circumstances:
(1) When we see an extant remote inode and have no cache for it, we set
the zero_point to i_size.
(2) On local inode creation, we set zero_point to 0.
(3) On local truncation down, we reduce zero_point to the new i_size if
the new i_size is lower.
(4) On local truncation up, we don't change zero_point.
(5) On local modification, we don't change zero_point.
(6) On remote invalidation, we set zero_point to the new i_size.
(7) If stored data is culled from the local cache, we must set zero_point
above that if the data also got written to the server.
(8) If dirty data is written back to the server, but not the local cache,
we must set zero_point above that.
Assuming the above, any read from the server at or above the zero_point
position will return all zeroes.
The zero_point value can be stored in the cache, provided the above rules
are applied to it by any code that culls part of the local cache.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/afs/inode.c | 13 +++++++------
fs/netfs/buffered_read.c | 40 +++++++++++++++++++++++++---------------
include/linux/netfs.h | 5 +++++
3 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 1c794a1896aa..46bc5574d6f5 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -252,6 +252,7 @@ static void afs_apply_status(struct afs_operation *op,
vnode->netfs.remote_i_size = status->size;
if (change_size) {
afs_set_i_size(vnode, status->size);
+ vnode->netfs.zero_point = status->size;
inode_set_ctime_to_ts(inode, t);
inode->i_atime = t;
}
@@ -865,17 +866,17 @@ static void afs_setattr_success(struct afs_operation *op)
static void afs_setattr_edit_file(struct afs_operation *op)
{
struct afs_vnode_param *vp = &op->file[0];
- struct inode *inode = &vp->vnode->netfs.inode;
+ struct afs_vnode *vnode = vp->vnode;
if (op->setattr.attr->ia_valid & ATTR_SIZE) {
loff_t size = op->setattr.attr->ia_size;
loff_t i_size = op->setattr.old_i_size;
- if (size < i_size)
- truncate_pagecache(inode, size);
- if (size != i_size)
- fscache_resize_cookie(afs_vnode_cache(vp->vnode),
- vp->scb.status.size);
+ if (size != i_size) {
+ truncate_pagecache(&vnode->netfs.inode, size);
+ netfs_resize_file(&vnode->netfs, size);
+ fscache_resize_cookie(afs_vnode_cache(vnode), size);
+ }
}
}
diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 2cd3ccf4c439..a2852fa64ad0 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -147,6 +147,22 @@ static void netfs_rreq_expand(struct netfs_io_request *rreq,
}
}
+/*
+ * Begin an operation, and fetch the stored zero point value from the cookie if
+ * available.
+ */
+static int netfs_begin_cache_operation(struct netfs_io_request *rreq,
+ struct netfs_inode *ctx)
+{
+ int ret = -ENOBUFS;
+
+ if (ctx->ops->begin_cache_operation) {
+ ret = ctx->ops->begin_cache_operation(rreq);
+ /* TODO: Get the zero point value from the cache */
+ }
+ return ret;
+}
+
/**
* netfs_readahead - Helper to manage a read request
* @ractl: The description of the readahead request
@@ -180,11 +196,9 @@ void netfs_readahead(struct readahead_control *ractl)
if (IS_ERR(rreq))
return;
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
- if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
- goto cleanup_free;
- }
+ ret = netfs_begin_cache_operation(rreq, ctx);
+ if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
+ goto cleanup_free;
netfs_stat(&netfs_n_rh_readahead);
trace_netfs_read(rreq, readahead_pos(ractl), readahead_length(ractl),
@@ -238,11 +252,9 @@ int netfs_read_folio(struct file *file, struct folio *folio)
goto alloc_error;
}
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
- if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
- goto discard;
- }
+ ret = netfs_begin_cache_operation(rreq, ctx);
+ if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
+ goto discard;
netfs_stat(&netfs_n_rh_readpage);
trace_netfs_read(rreq, rreq->start, rreq->len, netfs_read_trace_readpage);
@@ -390,11 +402,9 @@ int netfs_write_begin(struct netfs_inode *ctx,
rreq->no_unlock_folio = folio_index(folio);
__set_bit(NETFS_RREQ_NO_UNLOCK_FOLIO, &rreq->flags);
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
- if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
- goto error_put;
- }
+ ret = netfs_begin_cache_operation(rreq, ctx);
+ if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
+ goto error_put;
netfs_stat(&netfs_n_rh_write_begin);
trace_netfs_read(rreq, pos, len, netfs_read_trace_write_begin);
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b447cb67f599..282511090ead 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -129,6 +129,8 @@ struct netfs_inode {
struct fscache_cookie *cache;
#endif
loff_t remote_i_size; /* Size of the remote file */
+ loff_t zero_point; /* Size after which we assume there's no data
+ * on the server */
};
/*
@@ -330,6 +332,7 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
{
ctx->ops = ops;
ctx->remote_i_size = i_size_read(&ctx->inode);
+ ctx->zero_point = ctx->remote_i_size;
#if IS_ENABLED(CONFIG_FSCACHE)
ctx->cache = NULL;
#endif
@@ -345,6 +348,8 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
static inline void netfs_resize_file(struct netfs_inode *ctx, loff_t new_i_size)
{
ctx->remote_i_size = new_i_size;
+ if (new_i_size < ctx->zero_point)
+ ctx->zero_point = new_i_size;
}
/**
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data
2023-10-13 15:56 ` [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data David Howells
@ 2023-10-16 15:43 ` Jeff Layton
2023-10-16 16:31 ` David Howells
1 sibling, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2023-10-16 15:43 UTC (permalink / raw)
To: David Howells, Steve French
Cc: Matthew Wilcox, Marc Dionne, Paulo Alcantara, Ronnie Sahlberg,
Shyam Prasad N, Tom Talpey, Dominique Martinet, Ilya Dryomov,
Christian Brauner, linux-afs, linux-cifs, linux-nfs, ceph-devel,
v9fs, linux-fsdevel, linux-mm, netdev, linux-kernel,
linux-cachefs
On Fri, 2023-10-13 at 16:56 +0100, David Howells wrote:
> Track the file position above which the server is not expected to have any
> data and preemptively assume that we can simply fill blocks with zeroes
> locally rather than attempting to download them - even if we've written
> data back to the server. Assume that any data that was written back above
> that position is held in the local cache. Call this the "zero point".
>
> Make use of this to optimise away some reads from the server. We need to
> set the zero point in the following circumstances:
>
> (1) When we see an extant remote inode and have no cache for it, we set
> the zero_point to i_size.
>
> (2) On local inode creation, we set zero_point to 0.
>
> (3) On local truncation down, we reduce zero_point to the new i_size if
> the new i_size is lower.
>
> (4) On local truncation up, we don't change zero_point.
>
> (5) On local modification, we don't change zero_point.
>
> (6) On remote invalidation, we set zero_point to the new i_size.
>
> (7) If stored data is culled from the local cache, we must set zero_point
> above that if the data also got written to the server.
>
When you say culled here, it sounds like you're just throwing out the
dirty cache without writing the data back. That shouldn't be allowed
though, so I must be misunderstanding what you mean here. Can you
explain?
> (8) If dirty data is written back to the server, but not the local cache,
> we must set zero_point above that.
>
How do you write back without writing to the local cache? I'm guessing
this means you're doing a non-buffered write?
> Assuming the above, any read from the server at or above the zero_point
> position will return all zeroes.
>
> The zero_point value can be stored in the cache, provided the above rules
> are applied to it by any code that culls part of the local cache.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: linux-cachefs@redhat.com
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-mm@kvack.org
> ---
> fs/afs/inode.c | 13 +++++++------
> fs/netfs/buffered_read.c | 40 +++++++++++++++++++++++++---------------
> include/linux/netfs.h | 5 +++++
> 3 files changed, 37 insertions(+), 21 deletions(-)
>
> diff --git a/fs/afs/inode.c b/fs/afs/inode.c
> index 1c794a1896aa..46bc5574d6f5 100644
> --- a/fs/afs/inode.c
> +++ b/fs/afs/inode.c
> @@ -252,6 +252,7 @@ static void afs_apply_status(struct afs_operation *op,
> vnode->netfs.remote_i_size = status->size;
> if (change_size) {
> afs_set_i_size(vnode, status->size);
> + vnode->netfs.zero_point = status->size;
> inode_set_ctime_to_ts(inode, t);
> inode->i_atime = t;
> }
> @@ -865,17 +866,17 @@ static void afs_setattr_success(struct afs_operation *op)
> static void afs_setattr_edit_file(struct afs_operation *op)
> {
> struct afs_vnode_param *vp = &op->file[0];
> - struct inode *inode = &vp->vnode->netfs.inode;
> + struct afs_vnode *vnode = vp->vnode;
>
> if (op->setattr.attr->ia_valid & ATTR_SIZE) {
> loff_t size = op->setattr.attr->ia_size;
> loff_t i_size = op->setattr.old_i_size;
>
> - if (size < i_size)
> - truncate_pagecache(inode, size);
> - if (size != i_size)
> - fscache_resize_cookie(afs_vnode_cache(vp->vnode),
> - vp->scb.status.size);
> + if (size != i_size) {
> + truncate_pagecache(&vnode->netfs.inode, size);
> + netfs_resize_file(&vnode->netfs, size);
> + fscache_resize_cookie(afs_vnode_cache(vnode), size);
> + }
Isn't this an existing bug? AFS is not setting remote_i_size in the
setattr path currently? I think this probably ought to be done in a
preliminary AFS patch.
>
> }
> }
>
> diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
> index 2cd3ccf4c439..a2852fa64ad0 100644
> --- a/fs/netfs/buffered_read.c
> +++ b/fs/netfs/buffered_read.c
> @@ -147,6 +147,22 @@ static void netfs_rreq_expand(struct netfs_io_request *rreq,
> }
> }
>
> +/*
> + * Begin an operation, and fetch the stored zero point value from the cookie if
> + * available.
> + */
> +static int netfs_begin_cache_operation(struct netfs_io_request *rreq,
> + struct netfs_inode *ctx)
> +{
> + int ret = -ENOBUFS;
> +
> + if (ctx->ops->begin_cache_operation) {
> + ret = ctx->ops->begin_cache_operation(rreq);
> + /* TODO: Get the zero point value from the cache */
> + }
> + return ret;
> +}
> +
> /**
> * netfs_readahead - Helper to manage a read request
> * @ractl: The description of the readahead request
> @@ -180,11 +196,9 @@ void netfs_readahead(struct readahead_control *ractl)
> if (IS_ERR(rreq))
> return;
>
> - if (ctx->ops->begin_cache_operation) {
> - ret = ctx->ops->begin_cache_operation(rreq);
> - if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> - goto cleanup_free;
> - }
> + ret = netfs_begin_cache_operation(rreq, ctx);
> + if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> + goto cleanup_free;
>
> netfs_stat(&netfs_n_rh_readahead);
> trace_netfs_read(rreq, readahead_pos(ractl), readahead_length(ractl),
> @@ -238,11 +252,9 @@ int netfs_read_folio(struct file *file, struct folio *folio)
> goto alloc_error;
> }
>
> - if (ctx->ops->begin_cache_operation) {
> - ret = ctx->ops->begin_cache_operation(rreq);
> - if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> - goto discard;
> - }
> + ret = netfs_begin_cache_operation(rreq, ctx);
> + if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> + goto discard;
>
> netfs_stat(&netfs_n_rh_readpage);
> trace_netfs_read(rreq, rreq->start, rreq->len, netfs_read_trace_readpage);
> @@ -390,11 +402,9 @@ int netfs_write_begin(struct netfs_inode *ctx,
> rreq->no_unlock_folio = folio_index(folio);
> __set_bit(NETFS_RREQ_NO_UNLOCK_FOLIO, &rreq->flags);
>
> - if (ctx->ops->begin_cache_operation) {
> - ret = ctx->ops->begin_cache_operation(rreq);
> - if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> - goto error_put;
> - }
> + ret = netfs_begin_cache_operation(rreq, ctx);
> + if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
> + goto error_put;
>
> netfs_stat(&netfs_n_rh_write_begin);
> trace_netfs_read(rreq, pos, len, netfs_read_trace_write_begin);
> diff --git a/include/linux/netfs.h b/include/linux/netfs.h
> index b447cb67f599..282511090ead 100644
> --- a/include/linux/netfs.h
> +++ b/include/linux/netfs.h
> @@ -129,6 +129,8 @@ struct netfs_inode {
> struct fscache_cookie *cache;
> #endif
> loff_t remote_i_size; /* Size of the remote file */
> + loff_t zero_point; /* Size after which we assume there's no data
> + * on the server */
While I understand the concept, I'm not yet sure I understand how this
new value will be used. It might be better to merge this patch in with
the patch that adds the first user of this data.
> };
>
> /*
> @@ -330,6 +332,7 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
> {
> ctx->ops = ops;
> ctx->remote_i_size = i_size_read(&ctx->inode);
> + ctx->zero_point = ctx->remote_i_size;
> #if IS_ENABLED(CONFIG_FSCACHE)
> ctx->cache = NULL;
> #endif
> @@ -345,6 +348,8 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
> static inline void netfs_resize_file(struct netfs_inode *ctx, loff_t new_i_size)
> {
> ctx->remote_i_size = new_i_size;
> + if (new_i_size < ctx->zero_point)
> + ctx->zero_point = new_i_size;
> }
>
> /**
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data
2023-10-13 15:56 ` [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data David Howells
2023-10-16 15:43 ` Jeff Layton
@ 2023-10-16 16:31 ` David Howells
1 sibling, 0 replies; 12+ messages in thread
From: David Howells @ 2023-10-16 16:31 UTC (permalink / raw)
To: Jeff Layton
Cc: dhowells, Steve French, Matthew Wilcox, Marc Dionne,
Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
Dominique Martinet, Ilya Dryomov, Christian Brauner, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm,
netdev, linux-kernel, linux-cachefs
Jeff Layton <jlayton@kernel.org> wrote:
> > (7) If stored data is culled from the local cache, we must set zero_point
> > above that if the data also got written to the server.
>
> When you say culled here, it sounds like you're just throwing out the
> dirty cache without writing the data back. That shouldn't be allowed
> though, so I must be misunderstanding what you mean here. Can you
> explain?
I meant fscache specifically. Too many caches - and some of them with the
same names!
> > (8) If dirty data is written back to the server, but not the local cache,
> > we must set zero_point above that.
>
> How do you write back without writing to the local cache? I'm guessing
> this means you're doing a non-buffered write?
I meant fscache. fscache can decline to honour a request to store data.
> > + if (size != i_size) {
> > + truncate_pagecache(&vnode->netfs.inode, size);
> > + netfs_resize_file(&vnode->netfs, size);
> > + fscache_resize_cookie(afs_vnode_cache(vnode), size);
> > + }
>
> Isn't this an existing bug? AFS is not setting remote_i_size in the
> setattr path currently? I think this probably ought to be done in a
> preliminary AFS patch.
It is being set. afs_apply_status() sets it. This is called by
afs_vnode_commit_status() which is called from afs_setattr_success(). The
value isn't updated until we get the return status from the server that
includes the new value.
> > + loff_t zero_point; /* Size after which we assume there's no data
> > + * on the server */
>
> While I understand the concept, I'm not yet sure I understand how this
> new value will be used. It might be better to merge this patch in with
> the patch that adds the first user of this data.
I'll consider it. At least it might make sense to move them adjacent to each
other in the series.
David
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 03/53] netfs: Note nonblockingness in the netfs_io_request struct
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
2023-10-13 15:56 ` [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests David Howells
2023-10-13 15:56 ` [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data David Howells
@ 2023-10-13 15:56 ` David Howells
2023-10-16 15:44 ` Jeff Layton
2023-10-13 15:56 ` [RFC PATCH 04/53] netfs: Allow the netfs to make the io (sub)request alloc larger David Howells
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: David Howells @ 2023-10-13 15:56 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Dominique Martinet,
Ilya Dryomov, Christian Brauner, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm, netdev,
linux-kernel, linux-cachefs
Allow O_NONBLOCK to be noted in the netfs_io_request struct. Also add a
flag, NETFS_RREQ_BLOCKED to record if we did block.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/netfs/objects.c | 2 ++
include/linux/netfs.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index 85f428fc52e6..e41f9fc9bdd2 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -37,6 +37,8 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
INIT_LIST_HEAD(&rreq->subrequests);
refcount_set(&rreq->ref, 1);
__set_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
+ if (file && file->f_flags & O_NONBLOCK)
+ __set_bit(NETFS_RREQ_NONBLOCK, &rreq->flags);
if (rreq->netfs_ops->init_request) {
ret = rreq->netfs_ops->init_request(rreq, file);
if (ret < 0) {
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 282511090ead..b92e982ac4a0 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -205,6 +205,8 @@ struct netfs_io_request {
#define NETFS_RREQ_DONT_UNLOCK_FOLIOS 3 /* Don't unlock the folios on completion */
#define NETFS_RREQ_FAILED 4 /* The request failed */
#define NETFS_RREQ_IN_PROGRESS 5 /* Unlocked when the request completes */
+#define NETFS_RREQ_NONBLOCK 6 /* Don't block if possible (O_NONBLOCK) */
+#define NETFS_RREQ_BLOCKED 7 /* We blocked */
const struct netfs_request_ops *netfs_ops;
};
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 03/53] netfs: Note nonblockingness in the netfs_io_request struct
2023-10-13 15:56 ` [RFC PATCH 03/53] netfs: Note nonblockingness in the netfs_io_request struct David Howells
@ 2023-10-16 15:44 ` Jeff Layton
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2023-10-16 15:44 UTC (permalink / raw)
To: David Howells, Steve French
Cc: Matthew Wilcox, Marc Dionne, Paulo Alcantara, Ronnie Sahlberg,
Shyam Prasad N, Tom Talpey, Dominique Martinet, Ilya Dryomov,
Christian Brauner, linux-afs, linux-cifs, linux-nfs, ceph-devel,
v9fs, linux-fsdevel, linux-mm, netdev, linux-kernel,
linux-cachefs
On Fri, 2023-10-13 at 16:56 +0100, David Howells wrote:
> Allow O_NONBLOCK to be noted in the netfs_io_request struct. Also add a
> flag, NETFS_RREQ_BLOCKED to record if we did block.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: linux-cachefs@redhat.com
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-mm@kvack.org
> ---
> fs/netfs/objects.c | 2 ++
> include/linux/netfs.h | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
> index 85f428fc52e6..e41f9fc9bdd2 100644
> --- a/fs/netfs/objects.c
> +++ b/fs/netfs/objects.c
> @@ -37,6 +37,8 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
> INIT_LIST_HEAD(&rreq->subrequests);
> refcount_set(&rreq->ref, 1);
> __set_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
> + if (file && file->f_flags & O_NONBLOCK)
> + __set_bit(NETFS_RREQ_NONBLOCK, &rreq->flags);
> if (rreq->netfs_ops->init_request) {
> ret = rreq->netfs_ops->init_request(rreq, file);
> if (ret < 0) {
> diff --git a/include/linux/netfs.h b/include/linux/netfs.h
> index 282511090ead..b92e982ac4a0 100644
> --- a/include/linux/netfs.h
> +++ b/include/linux/netfs.h
> @@ -205,6 +205,8 @@ struct netfs_io_request {
> #define NETFS_RREQ_DONT_UNLOCK_FOLIOS 3 /* Don't unlock the folios on completion */
> #define NETFS_RREQ_FAILED 4 /* The request failed */
> #define NETFS_RREQ_IN_PROGRESS 5 /* Unlocked when the request completes */
> +#define NETFS_RREQ_NONBLOCK 6 /* Don't block if possible (O_NONBLOCK) */
> +#define NETFS_RREQ_BLOCKED 7 /* We blocked */
> const struct netfs_request_ops *netfs_ops;
> };
>
>
I'd prefer to see this patch squashed in with the first patches that
actually check for these flags. I can't look at this patch alone and
tell how it'll be used.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 04/53] netfs: Allow the netfs to make the io (sub)request alloc larger
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
` (2 preceding siblings ...)
2023-10-13 15:56 ` [RFC PATCH 03/53] netfs: Note nonblockingness in the netfs_io_request struct David Howells
@ 2023-10-13 15:56 ` David Howells
2023-10-13 15:56 ` [RFC PATCH 05/53] netfs: Add a ->free_subrequest() op David Howells
2023-10-17 10:42 ` [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib Daire Byrne
5 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2023-10-13 15:56 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Dominique Martinet,
Ilya Dryomov, Christian Brauner, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm, netdev,
linux-kernel, linux-cachefs
Allow the network filesystem to specify extra space to be allocated on the
end of the io (sub)request. This allows cifs, for example, to use this
space rather than allocating its own cifs_readdata struct.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/netfs/objects.c | 7 +++++--
include/linux/netfs.h | 2 ++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index e41f9fc9bdd2..2f1865ff7cce 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -22,7 +22,8 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
struct netfs_io_request *rreq;
int ret;
- rreq = kzalloc(sizeof(struct netfs_io_request), GFP_KERNEL);
+ rreq = kzalloc(ctx->ops->io_request_size ?: sizeof(struct netfs_io_request),
+ GFP_KERNEL);
if (!rreq)
return ERR_PTR(-ENOMEM);
@@ -116,7 +117,9 @@ struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_request *rreq
{
struct netfs_io_subrequest *subreq;
- subreq = kzalloc(sizeof(struct netfs_io_subrequest), GFP_KERNEL);
+ subreq = kzalloc(rreq->netfs_ops->io_subrequest_size ?:
+ sizeof(struct netfs_io_subrequest),
+ GFP_KERNEL);
if (subreq) {
INIT_LIST_HEAD(&subreq->rreq_link);
refcount_set(&subreq->ref, 2);
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b92e982ac4a0..6942b8cf03dc 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -214,6 +214,8 @@ struct netfs_io_request {
* Operations the network filesystem can/must provide to the helpers.
*/
struct netfs_request_ops {
+ unsigned int io_request_size; /* Alloc size for netfs_io_request struct */
+ unsigned int io_subrequest_size; /* Alloc size for netfs_io_subrequest struct */
int (*init_request)(struct netfs_io_request *rreq, struct file *file);
void (*free_request)(struct netfs_io_request *rreq);
int (*begin_cache_operation)(struct netfs_io_request *rreq);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 05/53] netfs: Add a ->free_subrequest() op
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
` (3 preceding siblings ...)
2023-10-13 15:56 ` [RFC PATCH 04/53] netfs: Allow the netfs to make the io (sub)request alloc larger David Howells
@ 2023-10-13 15:56 ` David Howells
2023-10-17 10:42 ` [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib Daire Byrne
5 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2023-10-13 15:56 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Dominique Martinet,
Ilya Dryomov, Christian Brauner, linux-afs, linux-cifs,
linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm, netdev,
linux-kernel, linux-cachefs
Add a ->free_subrequest() op so that the netfs can clean up data attached
to a subrequest.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/netfs/objects.c | 2 ++
include/linux/netfs.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index 2f1865ff7cce..8e92b8401aaa 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -147,6 +147,8 @@ static void netfs_free_subrequest(struct netfs_io_subrequest *subreq,
struct netfs_io_request *rreq = subreq->rreq;
trace_netfs_sreq(subreq, netfs_sreq_trace_free);
+ if (rreq->netfs_ops->free_subrequest)
+ rreq->netfs_ops->free_subrequest(subreq);
kfree(subreq);
netfs_stat_d(&netfs_n_rh_sreq);
netfs_put_request(rreq, was_async, netfs_rreq_trace_put_subreq);
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 6942b8cf03dc..ed64d1034afa 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -218,6 +218,7 @@ struct netfs_request_ops {
unsigned int io_subrequest_size; /* Alloc size for netfs_io_subrequest struct */
int (*init_request)(struct netfs_io_request *rreq, struct file *file);
void (*free_request)(struct netfs_io_request *rreq);
+ void (*free_subrequest)(struct netfs_io_subrequest *rreq);
int (*begin_cache_operation)(struct netfs_io_request *rreq);
void (*expand_readahead)(struct netfs_io_request *rreq);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib
2023-10-13 15:56 [RFC PATCH 00/53] netfs, afs, cifs: Delegate high-level I/O to netfslib David Howells
` (4 preceding siblings ...)
2023-10-13 15:56 ` [RFC PATCH 05/53] netfs: Add a ->free_subrequest() op David Howells
@ 2023-10-17 10:42 ` Daire Byrne
5 siblings, 0 replies; 12+ messages in thread
From: Daire Byrne @ 2023-10-17 10:42 UTC (permalink / raw)
To: David Howells
Cc: Jeff Layton, Steve French, Matthew Wilcox, Marc Dionne,
Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
Dominique Martinet, Ilya Dryomov, Christian Brauner, linux-afs,
linux-cifs, linux-nfs, ceph-devel, v9fs, linux-fsdevel, linux-mm,
netdev, linux-kernel
On Fri, 13 Oct 2023 at 16:58, David Howells <dhowells@redhat.com> wrote:
>
> (2) Use of fscache is not yet tested. I'm not sure whether to allow a
> cache to be used with a write-through write.
Just adding a quick end user "thumbs up" for this potential feature.
We currently use fscache as the backend for "NFS re-export" servers to
extend our onprem storage to remote cloud compute (which works great).
But batch compute hosts (think VFX render farm) often chunk up stages
of work into multiple batch jobs such that they read data, write
results and then read the same data on different clients. Having the
ability to also cache the recent writes closer to the compute clients
(on the re-export server) would open up a lot of new workload
possibilities for us.
> (5) Write-through caching will generate and dispatch write subrequests as
> it gathers enough data to hit wsize and has whole pages that at least
> span that size. This needs to be a bit more flexible, allowing for a
> filesystem such as CIFS to have a variable wsize.
If I understand correctly, this is above and beyond the normal write
back cache and is more in tune with the wsize (of NFS, CIFS etc) for
each file? Again, our workloads are over longer latencies than are
normal (NFS over 200ms!) so this sounds like a nice optimisation when
dealing with slow stuttering file writes over high latency.
I can definitely volunteer for some of the fscache + NFS testing.
Cheers,
Daire
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 01/53] netfs: Add a procfile to list in-progress requests
2023-10-13 16:03 David Howells
@ 2023-10-13 16:03 ` David Howells
0 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2023-10-13 16:03 UTC (permalink / raw)
To: Jeff Layton, Steve French
Cc: David Howells, Matthew Wilcox, Marc Dionne, Paulo Alcantara,
Shyam Prasad N, Tom Talpey, Dominique Martinet, Ilya Dryomov,
Christian Brauner, linux-afs, linux-cifs, linux-nfs, ceph-devel,
v9fs, linux-fsdevel, linux-mm, netdev, linux-kernel,
linux-cachefs
Add a procfile, /proc/fs/netfs/requests, to list in-progress netfslib I/O
requests.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
cc: linux-fsdevel@vger.kernel.org
cc: linux-mm@kvack.org
---
fs/netfs/internal.h | 22 +++++++++++
fs/netfs/main.c | 91 +++++++++++++++++++++++++++++++++++++++++++
fs/netfs/objects.c | 4 +-
include/linux/netfs.h | 6 ++-
4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index 43fac1b14e40..1f067aa96c50 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -29,6 +29,28 @@ int netfs_begin_read(struct netfs_io_request *rreq, bool sync);
* main.c
*/
extern unsigned int netfs_debug;
+extern struct list_head netfs_io_requests;
+extern spinlock_t netfs_proc_lock;
+
+#ifdef CONFIG_PROC_FS
+static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq)
+{
+ spin_lock(&netfs_proc_lock);
+ list_add_tail_rcu(&rreq->proc_link, &netfs_io_requests);
+ spin_unlock(&netfs_proc_lock);
+}
+static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq)
+{
+ if (!list_empty(&rreq->proc_link)) {
+ spin_lock(&netfs_proc_lock);
+ list_del_rcu(&rreq->proc_link);
+ spin_unlock(&netfs_proc_lock);
+ }
+}
+#else
+static inline void netfs_proc_add_rreq(struct netfs_io_request *rreq) {}
+static inline void netfs_proc_del_rreq(struct netfs_io_request *rreq) {}
+#endif
/*
* objects.c
diff --git a/fs/netfs/main.c b/fs/netfs/main.c
index 068568702957..21f814eee6af 100644
--- a/fs/netfs/main.c
+++ b/fs/netfs/main.c
@@ -7,6 +7,8 @@
#include <linux/module.h>
#include <linux/export.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
#include "internal.h"
#define CREATE_TRACE_POINTS
#include <trace/events/netfs.h>
@@ -18,3 +20,92 @@ MODULE_LICENSE("GPL");
unsigned netfs_debug;
module_param_named(debug, netfs_debug, uint, S_IWUSR | S_IRUGO);
MODULE_PARM_DESC(netfs_debug, "Netfs support debugging mask");
+
+#ifdef CONFIG_PROC_FS
+LIST_HEAD(netfs_io_requests);
+DEFINE_SPINLOCK(netfs_proc_lock);
+
+static const char *netfs_origins[] = {
+ [NETFS_READAHEAD] = "RA",
+ [NETFS_READPAGE] = "RP",
+ [NETFS_READ_FOR_WRITE] = "RW",
+};
+
+/*
+ * Generate a list of I/O requests in /proc/fs/netfs/requests
+ */
+static int netfs_requests_seq_show(struct seq_file *m, void *v)
+{
+ struct netfs_io_request *rreq;
+
+ if (v == &netfs_io_requests) {
+ seq_puts(m,
+ "REQUEST OR REF FL ERR OPS COVERAGE\n"
+ "======== == === == ==== === =========\n"
+ );
+ return 0;
+ }
+
+ rreq = list_entry(v, struct netfs_io_request, proc_link);
+ seq_printf(m,
+ "%08x %s %3d %2lx %4d %3d @%04llx %zx/%zx",
+ rreq->debug_id,
+ netfs_origins[rreq->origin],
+ refcount_read(&rreq->ref),
+ rreq->flags,
+ rreq->error,
+ atomic_read(&rreq->nr_outstanding),
+ rreq->start, rreq->submitted, rreq->len);
+ seq_putc(m, '\n');
+ return 0;
+}
+
+static void *netfs_requests_seq_start(struct seq_file *m, loff_t *_pos)
+ __acquires(rcu)
+{
+ rcu_read_lock();
+ return seq_list_start_head(&netfs_io_requests, *_pos);
+}
+
+static void *netfs_requests_seq_next(struct seq_file *m, void *v, loff_t *_pos)
+{
+ return seq_list_next(v, &netfs_io_requests, _pos);
+}
+
+static void netfs_requests_seq_stop(struct seq_file *m, void *v)
+ __releases(rcu)
+{
+ rcu_read_unlock();
+}
+
+static const struct seq_operations netfs_requests_seq_ops = {
+ .start = netfs_requests_seq_start,
+ .next = netfs_requests_seq_next,
+ .stop = netfs_requests_seq_stop,
+ .show = netfs_requests_seq_show,
+};
+#endif /* CONFIG_PROC_FS */
+
+static int __init netfs_init(void)
+{
+ if (!proc_mkdir("fs/netfs", NULL))
+ goto error;
+
+ if (!proc_create_seq("fs/netfs/requests", S_IFREG | 0444, NULL,
+ &netfs_requests_seq_ops))
+ goto error_proc;
+
+ return 0;
+
+error_proc:
+ remove_proc_entry("fs/netfs", NULL);
+error:
+ return -ENOMEM;
+}
+fs_initcall(netfs_init);
+
+static void __exit netfs_exit(void)
+{
+ remove_proc_entry("fs/netfs", NULL);
+}
+module_exit(netfs_exit);
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index e17cdf53f6a7..85f428fc52e6 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -45,6 +45,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
}
}
+ netfs_proc_add_rreq(rreq);
netfs_stat(&netfs_n_rh_rreq);
return rreq;
}
@@ -76,12 +77,13 @@ static void netfs_free_request(struct work_struct *work)
container_of(work, struct netfs_io_request, work);
trace_netfs_rreq(rreq, netfs_rreq_trace_free);
+ netfs_proc_del_rreq(rreq);
netfs_clear_subrequests(rreq, false);
if (rreq->netfs_ops->free_request)
rreq->netfs_ops->free_request(rreq);
if (rreq->cache_resources.ops)
rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
- kfree(rreq);
+ kfree_rcu(rreq, rcu);
netfs_stat_d(&netfs_n_rh_rreq);
}
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b11a84f6c32b..b447cb67f599 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -175,10 +175,14 @@ enum netfs_io_origin {
* operations to a variety of data stores and then stitch the result together.
*/
struct netfs_io_request {
- struct work_struct work;
+ union {
+ struct work_struct work;
+ struct rcu_head rcu;
+ };
struct inode *inode; /* The file being accessed */
struct address_space *mapping; /* The mapping being accessed */
struct netfs_cache_resources cache_resources;
+ struct list_head proc_link; /* Link in netfs_iorequests */
struct list_head subrequests; /* Contributory I/O operations */
void *netfs_priv; /* Private data for the netfs */
unsigned int debug_id;
^ permalink raw reply related [flat|nested] 12+ messages in thread