Re: [PATCH for-next 04/12] io_uring: reschedule retargeting at shutdown of ring

[Dylan Yudaken <dylany@meta.com>]

On Mon, 2022-10-31 at 10:02 -0600, Jens Axboe wrote:
> On 10/31/22 7:41 AM, Dylan Yudaken wrote:
> > diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> > index 8d0d40713a63..40b37899e943 100644
> > --- a/io_uring/rsrc.c
> > +++ b/io_uring/rsrc.c
> > @@ -248,12 +248,20 @@ static unsigned int
> > io_rsrc_retarget_table(struct io_ring_ctx *ctx,
> >         return refs;
> >  }
> >  
> > -static void io_rsrc_retarget_schedule(struct io_ring_ctx *ctx)
> > +static void io_rsrc_retarget_schedule(struct io_ring_ctx *ctx,
> > bool delay)
> >         __must_hold(&ctx->uring_lock)
> >  {
> > -       percpu_ref_get(&ctx->refs);
> > -       mod_delayed_work(system_wq, &ctx->rsrc_retarget_work, 60 *
> > HZ);
> > -       ctx->rsrc_retarget_scheduled = true;
> > +       unsigned long del;
> > +
> > +       if (delay)
> > +               del = 60 * HZ;
> > +       else
> > +               del = 0;
> > +
> > +       if (likely(!mod_delayed_work(system_wq, &ctx-
> > >rsrc_retarget_work, del))) {
> > +               percpu_ref_get(&ctx->refs);
> > +               ctx->rsrc_retarget_scheduled = true;
> > +       }
> >  }
> 
> What happens for del == 0 and the work running ala:
> 
> CPU 0                           CPU 1
> mod_delayed_work(.., 0);
>                                 delayed_work runs
>                                         put ctx
> percpu_ref_get(ctx)

The work takes the lock before put(ctx), and CPU 0 only releases the
lock after calling get(ctx) so it should be ok.

> 
> Also I think that likely() needs to get dropped.
> 

It's not a big thing, but the only time it will be enqueued is on ring
shutdown if there is an outstanding enqueue. Other times it will not
get double enqueued as it is protected by the _scheduled bool (this is
important or else it will continually push back by 1 period and maybe
never run)