Re: [PATCH 1/1] io_uring: add lockdep checks for io_handle_tw_list

[Jens Axboe <axboe@kernel.dk>]

On 4/1/25 11:19 AM, Pavel Begunkov wrote:
> On 4/1/25 17:13, Jens Axboe wrote:
>> On 4/1/25 9:46 AM, Pavel Begunkov wrote:
>>> Add a lockdep check to io_handle_tw_list() verifying that the context is
>>> locked and no task work drops it by accident.
>>
>> I think we'd want a bit more of a "why" explanation here, but I can add
>> that while committing.
>>
>>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>>> index 6df996d01ccf..13e0b48d1aac 100644
>>> --- a/io_uring/io_uring.c
>>> +++ b/io_uring/io_uring.c
>>> @@ -1054,6 +1054,10 @@ struct llist_node *io_handle_tw_list(struct llist_node *node,
>>>               mutex_lock(&ctx->uring_lock);
>>>               percpu_ref_get(&ctx->refs);
>>>           }
>>> +
>>> +        lockdep_assert(req->ctx == ctx);
>>> +        lockdep_assert_held(&ctx->uring_lock);
>>> +
>>>           INDIRECT_CALL_2(req->io_task_work.func,
>>>                   io_poll_task_func, io_req_rw_complete,
>>>                   req, ts);
>>
>> If the assumption is that some previous tw messed things up, might not
>> be a bad idea to include dumping of that if one of the above lockdep
>> asserts fail? Preferably in such a way that code generation is the same
>> when lockdep isn't set...
> 
> We can move it after the tw run where it still has the request
> (but doesn't own it).

Yep let's do that, but it'd still be nice to dump what func is if it
ends up triggering.

-- 
Jens Axboe