On Fri, Aug 23, 2024 at 05:27:44PM +0100, Mark Harmstone wrote:
> Move the various stack variables needed for encoded reads into struct
> btrfs_encoded_read_private, so that we can split it into several
> functions.
Moving local variables makes sense in some cases but I don't see reason
to move all of them.
> Signed-off-by: Mark Harmstone <maharmstone@xxxxxx>
> ---
> fs/btrfs/btrfs_inode.h | 20 ++++-
> fs/btrfs/inode.c | 170 +++++++++++++++++++++--------------------
> fs/btrfs/ioctl.c | 60 ++++++++-------
> 3 files changed, 135 insertions(+), 115 deletions(-)
>
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index affe70929234..5cd4308bd337 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -605,9 +605,23 @@ int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
> u64 file_offset, u64 disk_bytenr,
> u64 disk_io_size,
> struct page **pages);
> -ssize_t btrfs_encoded_read(struct file *file, loff_t offset,
> - struct iov_iter *iter,
> - struct btrfs_ioctl_encoded_io_args *encoded);
> +
> +struct btrfs_encoded_read_private {
> + wait_queue_head_t wait;
> + atomic_t pending;
> + blk_status_t status;
> + unsigned long nr_pages;
> + struct page **pages;
> + struct extent_state *cached_state;
The cached state is used as a local variable that is not reused by other
functions that also take the private structure, so what's the reason to
store it here?
> + size_t count;
> + struct iovec iovstack[UIO_FASTIOV];
> + struct iovec *iov;
Same, this is used in the leaf functions and not passed around.
> + struct iov_iter iter;
> + struct btrfs_ioctl_encoded_io_args args;
> + struct file *file;
> +};
As a coding pattern, the structure should store data that would be
otherwise passed as parameters or repeatedly derived from the
parameters.
