Re: [PATCH v2 05/19] netfs: Split netfs_io_* object handling out

From: Jeff Layton
Date: Wed Mar 09 2022 - 10:44:35 EST


On Tue, 2022-03-08 at 23:26 +0000, David Howells wrote:
> Split netfs_io_* object handling out into a file that's going to contain
> object allocation, get and put routines.
>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: linux-cachefs@xxxxxxxxxx
>
> Link: https://lore.kernel.org/r/164622995118.3564931.6089530629052064470.stgit@xxxxxxxxxxxxxxxxxxxxxx/ # v1
> ---
>
> fs/netfs/Makefile | 6 ++
> fs/netfs/internal.h | 18 +++++++
> fs/netfs/objects.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/netfs/read_helper.c | 118 ----------------------------------------------
> 4 files changed, 147 insertions(+), 118 deletions(-)
> create mode 100644 fs/netfs/objects.c
>
> diff --git a/fs/netfs/Makefile b/fs/netfs/Makefile
> index c15bfc966d96..939fd00a1fc9 100644
> --- a/fs/netfs/Makefile
> +++ b/fs/netfs/Makefile
> @@ -1,5 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -netfs-y := read_helper.o stats.o
> +netfs-y := \
> + objects.o \
> + read_helper.o
> +
> +netfs-$(CONFIG_NETFS_STATS) += stats.o
>
> obj-$(CONFIG_NETFS_SUPPORT) := netfs.o
> diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
> index b7f2c4459f33..cf7a3ddb16a4 100644
> --- a/fs/netfs/internal.h
> +++ b/fs/netfs/internal.h
> @@ -5,17 +5,35 @@
> * Written by David Howells (dhowells@xxxxxxxxxx)
> */
>
> +#include <linux/netfs.h>
> +#include <trace/events/netfs.h>
> +
> #ifdef pr_fmt
> #undef pr_fmt
> #endif
>
> #define pr_fmt(fmt) "netfs: " fmt
>
> +/*
> + * objects.c
> + */
> +struct netfs_io_request *netfs_alloc_request(const struct netfs_request_ops *ops,
> + void *netfs_priv,
> + struct file *file);
> +void netfs_get_request(struct netfs_io_request *rreq);
> +void netfs_clear_subrequests(struct netfs_io_request *rreq, bool was_async);
> +void netfs_put_request(struct netfs_io_request *rreq, bool was_async);
> +struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_request *rreq);
> +void netfs_put_subrequest(struct netfs_io_subrequest *subreq, bool was_async);
> +void netfs_get_subrequest(struct netfs_io_subrequest *subreq);
> +
> /*
> * read_helper.c
> */
> extern unsigned int netfs_debug;
>
> +void netfs_rreq_work(struct work_struct *work);
> +
> /*
> * stats.c
> */
> diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
> new file mode 100644
> index 000000000000..f7383c28dc6e
> --- /dev/null
> +++ b/fs/netfs/objects.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Object lifetime handling and tracing.
> + *
> + * Copyright (C) 2022 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@xxxxxxxxxx)
> + */
> +
> +#include <linux/slab.h>
> +#include "internal.h"
> +
> +/*
> + * Allocate an I/O request and initialise it.
> + */
> +struct netfs_io_request *netfs_alloc_request(
> + const struct netfs_request_ops *ops, void *netfs_priv,
> + struct file *file)
> +{
> + static atomic_t debug_ids;
> + struct netfs_io_request *rreq;
> +
> + rreq = kzalloc(sizeof(struct netfs_io_request), GFP_KERNEL);
> + if (rreq) {
> + rreq->netfs_ops = ops;
> + rreq->netfs_priv = netfs_priv;
> + rreq->inode = file_inode(file);
> + rreq->i_size = i_size_read(rreq->inode);
> + rreq->debug_id = atomic_inc_return(&debug_ids);
> + INIT_LIST_HEAD(&rreq->subrequests);
> + INIT_WORK(&rreq->work, netfs_rreq_work);
> + refcount_set(&rreq->usage, 1);
> + __set_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
> + if (ops->init_request)
> + ops->init_request(rreq, file);
> + netfs_stat(&netfs_n_rh_rreq);
> + }
> +
> + return rreq;
> +}
> +
> +void netfs_get_request(struct netfs_io_request *rreq)
> +{
> + refcount_inc(&rreq->usage);
> +}
> +
> +void netfs_clear_subrequests(struct netfs_io_request *rreq, bool was_async)
> +{
> + struct netfs_io_subrequest *subreq;
> +
> + while (!list_empty(&rreq->subrequests)) {
> + subreq = list_first_entry(&rreq->subrequests,
> + struct netfs_io_subrequest, rreq_link);
> + list_del(&subreq->rreq_link);
> + netfs_put_subrequest(subreq, was_async);
> + }
> +}
> +
> +static void netfs_free_request(struct work_struct *work)
> +{
> + struct netfs_io_request *rreq =
> + container_of(work, struct netfs_io_request, work);
> + netfs_clear_subrequests(rreq, false);
> + if (rreq->netfs_priv)
> + rreq->netfs_ops->cleanup(rreq->mapping, rreq->netfs_priv);
> + trace_netfs_rreq(rreq, netfs_rreq_trace_free);
> + if (rreq->cache_resources.ops)
> + rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
> + kfree(rreq);
> + netfs_stat_d(&netfs_n_rh_rreq);
> +}
> +
> +void netfs_put_request(struct netfs_io_request *rreq, bool was_async)
> +{
> + if (refcount_dec_and_test(&rreq->usage)) {
> + if (was_async) {
> + rreq->work.func = netfs_free_request;
> + if (!queue_work(system_unbound_wq, &rreq->work))
> + BUG();
> + } else {
> + netfs_free_request(&rreq->work);
> + }
> + }
> +}
> +
> +/*
> + * Allocate and partially initialise an I/O request structure.
> + */
> +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);
> + if (subreq) {
> + INIT_LIST_HEAD(&subreq->rreq_link);
> + refcount_set(&subreq->usage, 2);
> + subreq->rreq = rreq;
> + netfs_get_request(rreq);
> + netfs_stat(&netfs_n_rh_sreq);
> + }
> +
> + return subreq;
> +}
> +
> +void netfs_get_subrequest(struct netfs_io_subrequest *subreq)
> +{
> + refcount_inc(&subreq->usage);
> +}
> +
> +static void __netfs_put_subrequest(struct netfs_io_subrequest *subreq,
> + bool was_async)
> +{
> + struct netfs_io_request *rreq = subreq->rreq;
> +
> + trace_netfs_sreq(subreq, netfs_sreq_trace_free);
> + kfree(subreq);
> + netfs_stat_d(&netfs_n_rh_sreq);
> + netfs_put_request(rreq, was_async);
> +}
> +
> +void netfs_put_subrequest(struct netfs_io_subrequest *subreq, bool was_async)
> +{
> + if (refcount_dec_and_test(&subreq->usage))
> + __netfs_put_subrequest(subreq, was_async);
> +}
> diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c
> index 26d54055b17e..ef23ef9889d5 100644
> --- a/fs/netfs/read_helper.c
> +++ b/fs/netfs/read_helper.c
> @@ -27,122 +27,6 @@ unsigned netfs_debug;
> module_param_named(debug, netfs_debug, uint, S_IWUSR | S_IRUGO);
> MODULE_PARM_DESC(netfs_debug, "Netfs support debugging mask");
>
> -static void netfs_rreq_work(struct work_struct *);
> -static void __netfs_put_subrequest(struct netfs_io_subrequest *, bool);
> -
> -static void netfs_put_subrequest(struct netfs_io_subrequest *subreq,
> - bool was_async)
> -{
> - if (refcount_dec_and_test(&subreq->usage))
> - __netfs_put_subrequest(subreq, was_async);
> -}
> -
> -static struct netfs_io_request *netfs_alloc_request(
> - const struct netfs_request_ops *ops, void *netfs_priv,
> - struct file *file)
> -{
> - static atomic_t debug_ids;
> - struct netfs_io_request *rreq;
> -
> - rreq = kzalloc(sizeof(struct netfs_io_request), GFP_KERNEL);
> - if (rreq) {
> - rreq->netfs_ops = ops;
> - rreq->netfs_priv = netfs_priv;
> - rreq->inode = file_inode(file);
> - rreq->i_size = i_size_read(rreq->inode);
> - rreq->debug_id = atomic_inc_return(&debug_ids);
> - INIT_LIST_HEAD(&rreq->subrequests);
> - INIT_WORK(&rreq->work, netfs_rreq_work);
> - refcount_set(&rreq->usage, 1);
> - __set_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
> - if (ops->init_request)
> - ops->init_request(rreq, file);
> - netfs_stat(&netfs_n_rh_rreq);
> - }
> -
> - return rreq;
> -}
> -
> -static void netfs_get_request(struct netfs_io_request *rreq)
> -{
> - refcount_inc(&rreq->usage);
> -}
> -
> -static void netfs_clear_subrequests(struct netfs_io_request *rreq, bool was_async)
> -{
> - struct netfs_io_subrequest *subreq;
> -
> - while (!list_empty(&rreq->subrequests)) {
> - subreq = list_first_entry(&rreq->subrequests,
> - struct netfs_io_subrequest, rreq_link);
> - list_del(&subreq->rreq_link);
> - netfs_put_subrequest(subreq, was_async);
> - }
> -}
> -
> -static void netfs_free_request(struct work_struct *work)
> -{
> - struct netfs_io_request *rreq =
> - container_of(work, struct netfs_io_request, work);
> - netfs_clear_subrequests(rreq, false);
> - if (rreq->netfs_priv)
> - rreq->netfs_ops->cleanup(rreq->mapping, rreq->netfs_priv);
> - trace_netfs_rreq(rreq, netfs_rreq_trace_free);
> - if (rreq->cache_resources.ops)
> - rreq->cache_resources.ops->end_operation(&rreq->cache_resources);
> - kfree(rreq);
> - netfs_stat_d(&netfs_n_rh_rreq);
> -}
> -
> -static void netfs_put_request(struct netfs_io_request *rreq, bool was_async)
> -{
> - if (refcount_dec_and_test(&rreq->usage)) {
> - if (was_async) {
> - rreq->work.func = netfs_free_request;
> - if (!queue_work(system_unbound_wq, &rreq->work))
> - BUG();
> - } else {
> - netfs_free_request(&rreq->work);
> - }
> - }
> -}
> -
> -/*
> - * Allocate and partially initialise an I/O request structure.
> - */
> -static 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);
> - if (subreq) {
> - INIT_LIST_HEAD(&subreq->rreq_link);
> - refcount_set(&subreq->usage, 2);
> - subreq->rreq = rreq;
> - netfs_get_request(rreq);
> - netfs_stat(&netfs_n_rh_sreq);
> - }
> -
> - return subreq;
> -}
> -
> -static void netfs_get_subrequest(struct netfs_io_subrequest *subreq)
> -{
> - refcount_inc(&subreq->usage);
> -}
> -
> -static void __netfs_put_subrequest(struct netfs_io_subrequest *subreq,
> - bool was_async)
> -{
> - struct netfs_io_request *rreq = subreq->rreq;
> -
> - trace_netfs_sreq(subreq, netfs_sreq_trace_free);
> - kfree(subreq);
> - netfs_stat_d(&netfs_n_rh_sreq);
> - netfs_put_request(rreq, was_async);
> -}
> -
> /*
> * Clear the unread part of an I/O request.
> */
> @@ -558,7 +442,7 @@ static void netfs_rreq_assess(struct netfs_io_request *rreq, bool was_async)
> netfs_rreq_completed(rreq, was_async);
> }
>
> -static void netfs_rreq_work(struct work_struct *work)
> +void netfs_rreq_work(struct work_struct *work)
> {
> struct netfs_io_request *rreq =
> container_of(work, struct netfs_io_request, work);
>
>

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>