Re: [PATCH v4 5/5] io_uring: add fgetxattr and getxattr support

[Pavel Begunkov <asml.silence@gmail.com>]

On 12/15/21 22:17, Stefan Roesch wrote:
> This adds support to io_uring for the fgetxattr and getxattr API.

Same comments as with 4/5, only one additional below

> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>   fs/io_uring.c                 | 150 ++++++++++++++++++++++++++++++++++
>   include/uapi/linux/io_uring.h |   2 +
>   2 files changed, 152 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index fc2239635342..c365944a8054 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
[...]
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index dbf473900da2..cd9160272308 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -145,7 +145,9 @@ enum {
>   	IORING_OP_MKDIRAT,
>   	IORING_OP_SYMLINKAT,
>   	IORING_OP_LINKAT,
> +	IORING_OP_FGETXATTR,

Even though it's just one-commit gap, it's not great changing uapi,
e.g. +1 to previous IORING_OP_FSETXATTR. It may actually make more
sense to keep xget and xfget closer together because you're reusing
code for them and compiler may _potentially_ better optimise it,
e.g. ifs in switches.

>   	IORING_OP_FSETXATTR,
> +	IORING_OP_GETXATTR,
>   	IORING_OP_SETXATTR,
>   
>   	/* this goes last, obviously */

-- 
Pavel Begunkov