[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-aio
Subject:    Re: [PATCH] aio: io_cancel: don't always return -EINVAL
From:       Jeff Moyer <jmoyer () redhat ! com>
Date:       2020-01-23 16:26:53
Message-ID: x49o8uuxho2.fsf () segfault ! boston ! devel ! redhat ! com
[Download RAW message or body]

mwilck@suse.com writes:

> From: Martin Wilck <mwilck@suse.com>
>
> Since 0460fef2a921 ("aio: use cancellation list lazily"), io_cancel()
> basically always returns -EINVAL, because only such iocbs that have
> a ki_cancel attribute are members of the active list, and hardly any
> iocbs have this attribute. This is confusing, because according to
> the man page -EINVAL means that the passed-in aio context is invalid.
>
> Change this to -EAGAIN, which according to the man page means "The iocb
> specified was not cancelled". Using -EAGAIN might wrongly suggest to users
> that repeating the system call might succeed, but it seems to be more
> fitting than -EINVAL.
>
> While at it, fix the comments for io_cancel.

The fix looks good, but please add a "Fixes:" tag.  I'm not sure how the
kerneldoc will render that return block, but hopefully you checked and
are happy with the output?

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

> ---
>  fs/aio.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 01e0fb9..3dc4f79 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1972,22 +1972,31 @@ COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
>  }
>  #endif
>  
> -/* sys_io_cancel:
> - *	Attempts to cancel an iocb previously passed to io_submit.  If
> - *	the operation is successfully cancelled, the resulting event is
> - *	copied into the memory pointed to by result without being placed
> - *	into the completion queue and 0 is returned.  May fail with
> - *	-EFAULT if any of the data structures pointed to are invalid.
> - *	May fail with -EINVAL if aio_context specified by ctx_id is
> - *	invalid.  May fail with -EAGAIN if the iocb specified was not
> - *	cancelled.  Will fail with -ENOSYS if not implemented.
> +/**
> + * sys_io_cancel() - attempt to cancel an async I/O iocb
> + * @ctx_id:	aio context
> + * @iocb:	the iocb to cancel
> + * @result:	unused
> + *
> + * Attempt to cancel an iocb previously passed to io_submit. Actual
> + * cancellation requires that the I/O backend has set the ki_cancel
> + * attribute. As of v5.5, cancellation is only implemented by the USB gadget
> + * code, and for users of IOCB_CMD_POLL. The result field is unused, user
> + * space needs to wait for iocb completion to examine the iocb status.
> + * This function never returns 0.
> + *
> + * Return:	-EINVAL or -EFAULT if invalid parameters are passed.
> + *		-EAGAIN if the iocb can't be cancelled because ki_cancel
> + *		is unset.
> + *		-EINPROGRESS, otherwise.
>   */
>  SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
>  		struct io_event __user *, result)
>  {
>  	struct kioctx *ctx;
>  	struct aio_kiocb *kiocb;
> -	int ret = -EINVAL;
> +	/* Return code for iocbs not found on active_reqs list */
> +	int ret = -EAGAIN;
>  	u32 key;
>  	u64 obj = (u64)(unsigned long)iocb;


--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
[prev in list] [next in list] [prev in thread] [next in thread]