On Wed, Oct 14, 2020 at 08:44:22PM +0100, Pavel Begunkov wrote:
> Optimise put handling in __io_fail_links() after removing
> REQ_F_COMP_LOCKED.
>
> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> ---
> fs/io_uring.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index f61af4d487fd..271a016e8507 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1816,7 +1816,16 @@ static void __io_fail_links(struct io_kiocb *req)
> trace_io_uring_fail_link(req, link);
>
> io_cqring_fill_event(link, -ECANCELED);
> - io_put_req_deferred(link, 2);
> +
> + /*
> + * It's ok to free under spinlock as they're not linked anymore,
> + * but avoid REQ_F_WORK_INITIALIZED because it may deadlock on
> + * work.fs->lock.
> + */
> + if (link->flags | REQ_F_WORK_INITIALIZED)
> + io_put_req_deferred(link, 2);
> + else
> + io_double_put_req(link);
> }
>
> io_commit_cqring(ctx);
> --
> 2.24.0
>
This part of commit 9c357fed168a ("io_uring: fix REQ_F_COMP_LOCKED by
killing it") causes a clang warning:
fs/io_uring.c:1816:19: warning: bitwise or with non-zero value always
evaluates to true [-Wtautological-bitwise-compare]
if (link->flags | REQ_F_WORK_INITIALIZED)
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
According to the comment, was it intended for that to be a bitwise AND
then negated to check for the absence of it? If so, wouldn't it be
clearer to flip the condition so that a negation is not necessary like
below? I can send a formal patch if my analysis is correct but if not,
feel free to fix it yourself and add
Reported-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 66c41d53a9d3..28b1a0fede3e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1813,10 +1813,10 @@ static void __io_fail_links(struct io_kiocb *req)
* but avoid REQ_F_WORK_INITIALIZED because it may deadlock on
* work.fs->lock.
*/
- if (link->flags | REQ_F_WORK_INITIALIZED)
- io_put_req_deferred(link, 2);
- else
+ if (link->flags & REQ_F_WORK_INITIALIZED)
io_double_put_req(link);
+ else
+ io_put_req_deferred(link, 2);
}
io_commit_cqring(ctx);
