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

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 18/05/2021 16:52, David Sterba wrote:
> Add sysfs interface to limit io during scrub. We relied on the ionice
> interface to do that, eg. the idle class let the system usable while
> scrub was running. This has changed when mq-deadline got widespread and
> did not implement the scheduling classes. That was a CFQ thing that got
> deleted. We've got numerous complaints from users about degraded
> performance.
> 
> Currently only BFQ supports that but it's not a common scheduler and we
> can't ask everybody to switch to it.
> 
> Alternatively the cgroup io limiting can be used but that also a
> non-trivial setup (v2 required, the controller must be enabled on the
> system). This can still be used if desired.
> 
> Other ideas that have been explored: piggy-back on ionice (that is set
> per-process and is accessible) and interpret the class and classdata as
> bandwidth limits, but this does not have enough flexibility as there are
> only 8 allowed and we'd have to map fixed limits to each value. Also
> adjusting the value would need to lookup the process that currently runs
> scrub on the given device, and the value is not sticky so would have to
> be adjusted each time scrub runs.
> 
> Running out of options, sysfs does not look that bad:
> 
> - it's accessible from scripts, or udev rules
> - the name is similar to what MD-RAID has
>   (/proc/sys/dev/raid/speed_limit_max or /sys/block/mdX/md/sync_speed_max)
> - the value is sticky at least for filesystem mount time
> - adjusting the value has immediate effect
> - sysfs is available in constrained environments (eg. system rescue)
> - the limit also applies to device replace
> 
> Sysfs:
> 
> - raw value is in bytes
> - 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>

This looks very good :)
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

I wonder if this interface would make sense for limiting balance bandwidth as well?