Re: [PATCH] btrfs: scrub: per-device bandwidth control

[Geert Uytterhoeven]

Hi Arnd,

On Thu, May 20, 2021 at 3:15 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Thu, May 20, 2021 at 9:43 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > On Tue, 18 May 2021, David Sterba wrote:
> > > --- a/fs/btrfs/scrub.c
> > > +++ b/fs/btrfs/scrub.c
> > > @@ -1988,6 +1993,60 @@ static void scrub_page_put(struct scrub_page *spage)
> > >       }
> > > }
> > >
> > > +/*
> > > + * Throttling of IO submission, bandwidth-limit based, the timeslice is 1
> > > + * second.  Limit can be set via /sys/fs/UUID/devinfo/devid/scrub_speed_max.
> > > + */
> > > +static void scrub_throttle(struct scrub_ctx *sctx)
> > > +{
> > > +     const int time_slice = 1000;
> > > +     struct scrub_bio *sbio;
> > > +     struct btrfs_device *device;
> > > +     s64 delta;
> > > +     ktime_t now;
> > > +     u32 div;
> > > +     u64 bwlimit;
> > > +
> > > +     sbio = sctx->bios[sctx->curr];
> > > +     device = sbio->dev;
> > > +     bwlimit = READ_ONCE(device->scrub_speed_max);
> > > +     if (bwlimit == 0)
> > > +             return;
> > > +
> > > +     /*
> > > +      * Slice is divided into intervals when the IO is submitted, adjust by
> > > +      * bwlimit and maximum of 64 intervals.
> > > +      */
> > > +     div = max_t(u32, 1, (u32)(bwlimit / (16 * 1024 * 1024)));
> > > +     div = min_t(u32, 64, div);
> > > +
> > > +     /* Start new epoch, set deadline */
> > > +     now = ktime_get();
> > > +     if (sctx->throttle_deadline == 0) {
> > > +             sctx->throttle_deadline = ktime_add_ms(now, time_slice / div);
> >
> > ERROR: modpost: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!
> >
> > div_u64(bwlimit, div)
>
> If 'time_slice' is in nanoseconds, the best interface to use
> is ktime_divns().

Actually this one is not a problem...

>
> > > +             sctx->throttle_sent = 0;
> > > +     }
> > > +
> > > +     /* Still in the time to send? */
> > > +     if (ktime_before(now, sctx->throttle_deadline)) {
> > > +             /* If current bio is within the limit, send it */
> > > +             sctx->throttle_sent += sbio->bio->bi_iter.bi_size;
> > > +             if (sctx->throttle_sent <= bwlimit / div)
> > > +                     return;
>
> Doesn't this also need to be changed?

... but this is.

Sorry. I added the annotation to the wrong line, after devising which
code caused the issue from looking at the generated assembly ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds