On 15/10/2020 09:53, Nathan Chancellor wrote: > On Wed, Oct 14, 2020 at 08:44:22PM +0100, Pavel Begunkov wrote: >> - 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); > > 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 I have no idea what have happened, but yeah, there should be "&", though without any additional negation. That's because deferred version is safer. Nathan, thanks for letting know! Jens, could you please fold in the change below. diff --git a/fs/io_uring.c b/fs/io_uring.c index 66c41d53a9d3..2c83c2688ec5 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1813,7 +1813,7 @@ 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) + if (link->flags & REQ_F_WORK_INITIALIZED) io_put_req_deferred(link, 2); else io_double_put_req(link); -- Pavel Begunkov
