Re: [PATCH v5 3/5] fs: split off do_getxattr from getxattr

[Jens Axboe <axboe@kernel.dk>]

On 12/21/21 2:59 PM, Stefan Roesch wrote:
> 
> 
> On 12/21/21 11:18 AM, Linus Torvalds wrote:
>> On Tue, Dec 21, 2021 at 11:15 AM Stefan Roesch <shr@fb.com> wrote:
>>>
>>> Linus, if we remove the constness, then we either need to cast away the constness (the system call
>>> is defined as const) or change the definition of the system call.
>>
>> You could also do it as
>>
>>         union {
>>                 const void __user *setxattr_value;
>>                 void __user *getxattr_value;
>>         };
>>
> 
> Pavel brought up a very good point. By adding the kname array into the
> xarray_ctx we increase the size of io_xattr structure too much. In
> addition this will also increase the size of the io_kiocb structure.
> The original solution did not increase the size.
> 
> Per opcode we limit the storage space to 64 bytes. However the array
> itself requires 256 bytes.

Just to expand on that a bit - part of struct io_kiocb is per-command
data, and we try pretty hard to keep that at 64-bytes as that's the
largest one we currently have. If we add the array to the io_xattr
structure, then that will increase the whole io_kiocb from 224 bytes to
more than twice that.

So there are really two options here:

1) The xattr_ctx structure goes into the async data that a command has
   to allocate for deferred execution. This isn't a _huge_ deal as we
   have to defer the xattr commands for now anyway, as the VFS doesn't
   support a nonblocking version of that yet. But it would still be nice
   not to have to do that.

2) We keep the original interface that Stefan proposed, leaving the
   xattr_ctx bits embedded as they fit fine like that.

#2 would be a bit more efficient, but I don't feel that strongly about
it for this particular case.

Comments?

-- 
Jens Axboe