On Mon, Oct 21, 2024 at 05:05:20PM +0000, Mark Harmstone wrote:
> >> +static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd,
> >> + unsigned int issue_flags)
> >> +{
> >> + size_t copy_end_kernel = offsetofend(struct btrfs_ioctl_encoded_io_args,
> >> + flags);
> >> + size_t copy_end;
> >> + struct btrfs_ioctl_encoded_io_args args = {0};
> > = { 0 }
> >> + int ret;
> >> + u64 disk_bytenr, disk_io_size;
> >> + struct file *file = cmd->file;
> >> + struct btrfs_inode *inode = BTRFS_I(file->f_inode);
> >> + struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >> + struct extent_io_tree *io_tree = &inode->io_tree;
> >> + struct iovec iovstack[UIO_FASTIOV];
> >> + struct iovec *iov = iovstack;
> >> + struct iov_iter iter;
> >> + loff_t pos;
> >> + struct kiocb kiocb;
> >> + struct extent_state *cached_state = NULL;
> >> + u64 start, lockend;
> >
> > The stack consumption looks quite high.
>
> 696 bytes, compared to 672 in btrfs_ioctl_encoded_read.
> btrfs_ioctl_encoded write is pretty big too. Probably the easiest thing
> here would be to allocate btrfs_uring_priv early and pass that around, I
> think.
>
> Do you have a recommendation for what the maximum stack size of a
> function should be?
It depends from where the function is called. For ioctl callbacks, like
btrfs_ioctl_encoded_read it's the first function using kernel stack
leaving enough for any deep IO stacks (DM/NFS/iSCSI/...). If something
similar applies to the io_uring callbacks then it's probably fine.
Using a separate off-stack structure works but it's a penalty as it
needs the allcation. The io_uring is meant for high performance so if
the on-stack allocation is safe then keep it like that.
I've checked on a release config the stack consumption and the encoded
ioctl functions are not the worst:
tree-log.c:btrfs_sync_log 728 static
scrub.c:scrub_verify_one_metadata 552 dynamic,bounded
inode.c:print_data_reloc_error 544 dynamic,bounded
uuid-tree.c:btrfs_uuid_scan_kthread 520 static
tree-checker.c:check_root_item 504 static
file-item.c:btrfs_csum_one_bio 496 static
inode.c:btrfs_start_delalloc_roots 488 static
scrub.c:scrub_raid56_parity_stripe 464 dynamic,bounded
disk-io.c:write_dev_supers 464 static
ioctl.c:btrfs_ioctl_encoded_write 456 dynamic,bounded
ioctl.c:btrfs_ioctl_encoded_read 456 dynamic,bounded
