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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 May 2021 15:26:29 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     David Sterba <dsterba@...e.com>,
        linux-btrfs <linux-btrfs@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] btrfs: scrub: per-device bandwidth control

Hi Arnd,

On Thu, May 20, 2021 at 3:15 PM Arnd Bergmann <arnd@...db.de> wrote:
> On Thu, May 20, 2021 at 9:43 AM Geert Uytterhoeven <geert@...ux-m68k.org> 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@...ux-m68k.org

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