Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe <axboe@kernel.dk>]

On 10/1/24 7:22 PM, Al Viro wrote:
> diff --git a/io_uring/xattr.c b/io_uring/xattr.c
> index 71d9e2569a2f..7f6bbfd846b9 100644
> --- a/io_uring/xattr.c
> +++ b/io_uring/xattr.c
>  int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
>  {
> +	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
>  	int ret;
>  
>  	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
>  
> -	ret = __io_setxattr(req, issue_flags, &req->file->f_path);
> +	ret = file_setxattr(req->file, &ix->ctx);
>  	io_xattr_finish(req, ret);
>  	return IOU_OK;

This and ... ->

> -retry:
> -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
> -	if (!ret) {
> -		ret = __io_setxattr(req, issue_flags, &path);
> -		path_put(&path);
> -		if (retry_estale(ret, lookup_flags)) {
> -			lookup_flags |= LOOKUP_REVAL;
> -			goto retry;
> -		}
> -	}
> -
> +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
>  	io_xattr_finish(req, ret);
>  	return IOU_OK;

this looks like it needs an ix->filename = NULL, as
filename_{s,g}xattr() drops the reference. The previous internal helper
did not, and hence the cleanup always did it. But should work fine if
->filename is just zeroed.

Otherwise looks good. I've skimmed the other patches and didn't see
anything odd, I'll take a closer look tomorrow.

-- 
Jens Axboe