On Thu, Aug 31, 2023 at 11:59:42AM -0600, Jens Axboe wrote:
> On 8/31/23 1:42 AM, Ming Lei wrote:
> > @@ -3313,11 +3323,12 @@ __cold void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd)
> > atomic_inc(&tctx->in_cancel);
> > do {
> > bool loop = false;
> > + bool wq_cancelled;
>
> Minor nit, but io_uring generally uses US spelling, so this should be
> wq_canceled.
OK.
>
> >
> > io_uring_drop_tctx_refs(current);
> > /* read completions before cancelations */
> > inflight = tctx_inflight(tctx, !cancel_all);
> > - if (!inflight)
> > + if (!inflight && !tctx->io_wq)
> > break;
>
> Not sure I follow this one, is it just for checking of io_wq was ever
> setup? How could it not be?
Here if we have io_wq, all requests in io_wq have to be canceled no
matter if inflight is zero or not because tctx_inflight(tctx, true)
doesn't track FIXED_FILE IOs, but there still can be such IOs in io_wq.
>
> > - if (loop) {
> > + if (!wq_cancelled || (inflight && loop)) {
> > cond_resched();
> > continue;
> > }
>
> And this one is a bit puzzling to me too - if we didn't cancel anything
> but don't have anything inflight, why are we looping? Should it be
> something ala:
>
> if (inflight && (!wq_cancelled || loop)) {
There can be FIXED_FILE IOs in io_wq even though inflight is zero.
Thanks,
Ming
