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

[David Sterba <dsterba@suse.cz>]

On Thu, May 20, 2021 at 09:43:10AM +0200, Geert Uytterhoeven wrote:
> > - values written to the file accept suffixes like K, M
> > - file is in the per-device directory /sys/fs/btrfs/FSID/devinfo/DEVID/scrub_speed_max
> > - 0 means use default priority of IO
> >
> > The scheduler is a simple deadline one and the accuracy is up to nearest
> > 128K.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> 
> Thanks for your patch, which is now commit b4a9f4bee31449bc ("btrfs:
> scrub: per-device bandwidth control") in linux-next.
> 
> noreply@ellerman.id.au reported the following failures for e.g.
> m68k/defconfig:
> 
> ERROR: modpost: "__udivdi3" [fs/btrfs/btrfs.ko] undefined!
> ERROR: modpost: "__divdi3" [fs/btrfs/btrfs.ko] undefined!

I'll fix it, thanks for the report.

> > +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)
> 
> > +		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;
> > +
> > +		/* We're over the limit, sleep until the rest of the slice */
> > +		delta = ktime_ms_delta(sctx->throttle_deadline, now);
> > +	} else {
> > +		/* New request after deadline, start new epoch */
> > +		delta = 0;
> > +	}
> > +
> > +	if (delta)
> > +		schedule_timeout_interruptible(delta * HZ / 1000);
> 
> ERROR: modpost: "__divdi3" [fs/btrfs/btrfs.ko] undefined!
> 
> I'm a bit surprised gcc doesn't emit code for the division by the
> constant 1000, but emits a call to __divdi3().  So this has to become
> div_u64(), too.
> 
> > +	/* Next call will start the deadline period */
> > +	sctx->throttle_deadline = 0;
> > +}
> 
> BTW, any chance you can start adding lore Link: tags to your commits, to
> make it easier to find the email thread to reply to when reporting a
> regression?

Well, no I'm not going to do that, sorry. It should be easy enough to
paste the patch subject to the search field on lore.k.org and click the
link leading to the mail, I do that all the time. Making sure that
patches have all the tags and information takes time already so I'm not
too keen to spend time on adding links.