Re: [PATCH 0/7] Allocate structures on stack instead of kmalloc()

[David Sterba <dsterba@suse.cz>]

On Tue, Jul 27, 2021 at 04:17:24PM -0500, Goldwyn Rodrigues wrote:
> Here are some structs which can be converted to allocation on stack in order
> to save on post-checks on kmalloc() and kfree() each of them.
> 
> Sizes of the structures are also in the commit message in case you feel they
> are too bit to be allocated on stack.

Reducing the potential error failures is good and may slightly increase
performance in case the system is low on memory and allocator would have
to do some work.

We must also try to reduce stack usage, but for the ioctls it should be
safe as it's run from the process context and not from a deep call
chain. Where it could be a problem, if the call chaing becomes deep
under btrfs, ie. in any other intermediate layer like NFS, DM, block
layer and drivers.

So, I'd limit the size of on-stack variables to something like 128, that
accounts for a few nested function calls with a few varaibles (eg. 4
functions with 4 pointers each).. This is a normal pattern anywhere
else.

> There are two more structs in ioctl.c which can be converted, but
> I was not sure of them:
> 
> 1. In create_snapshot(), pending_snapshot can be converted. pending_snapshot
>    is used in the transaction.

That one is 144 bytes, arguably still ok.

> 2. In btrfs_ioctl_set_received_subvol_32, args64 can be converted, but args32
>    cannot. All Pointers associated with memdup_user() can also be converted
>    by using copy_from_user() instead. This would include many more structs.

size of btrfs_ioctl_received_subvol_args_32 is 192,
and btrfs_ioctl_received_subvol_args is 200 bytes, both in the same
function.

I'd leave this one as is, it's not something critical where performance
matters.

So, overall I'll apply the series without the two commented.