On 11/22/24 10:07 AM, Pavel Begunkov wrote:
> On 11/22/24 16:12, Jens Axboe wrote:
> ...
>> static inline void io_req_local_work_add(struct io_kiocb *req,
>> struct io_ring_ctx *ctx,
>> - unsigned flags)
>> + unsigned tw_flags)
>> {
>> - unsigned nr_wait, nr_tw, nr_tw_prev;
>> - struct llist_node *head;
>> + unsigned nr_tw, nr_tw_prev, nr_wait;
>> + unsigned long flags;
>> /* See comment above IO_CQ_WAKE_INIT */
>> BUILD_BUG_ON(IO_CQ_WAKE_FORCE <= IORING_MAX_CQ_ENTRIES);
>> /*
>> - * We don't know how many reuqests is there in the link and whether
>> - * they can even be queued lazily, fall back to non-lazy.
>> + * We don't know how many requests are in the link and whether they can
>> + * even be queued lazily, fall back to non-lazy.
>> */
>> if (req->flags & (REQ_F_LINK | REQ_F_HARDLINK))
>> - flags &= ~IOU_F_TWQ_LAZY_WAKE;
>> + tw_flags &= ~IOU_F_TWQ_LAZY_WAKE;
>> - guard(rcu)();
>
> protects against ctx->task deallocation, see a comment in
> io_ring_exit_work() -> synchronize_rcu()
Yeah that's just an editing mistake.
>> + spin_lock_irqsave(&ctx->work_lock, flags);
>> + wq_list_add_tail(&req->io_task_work.work_node, &ctx->work_list);
>> + nr_tw_prev = ctx->work_items++;
>
> Is there a good reason why it changes the semantics of
> what's stored across adds? It was assigning a corrected
> nr_tw, this one will start heavily spamming with wake_up()
> in some cases.
Not sure I follow, how so? nr_tw_prev will be the previous count, just
like before. Except we won't need to dig into the list to find it, we
have it readily available. nr_tw will be the current code, or force wake
if needed. As before.
--
Jens Axboe
