| Message ID | ac03a3ccafe6c5a1c5cf1883e9b88dddfb34fcfe.1680723651.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: adjust async discard tuning | expand |
On Wed, Apr 5, 2023 at 4:00 PM Boris Burkov <boris@bur.io> wrote: > > Currently, a limit of 0 results in a hardcoded metering over 6 hours. > Since the default is a set limit, I suspect no one truly depends on this > rather arbitrary setting. Repurpose it for an arguably more useful > "unlimited" mode, where the delay is 0. > > Note that if block groups are too new, or go fully empty, there is still > a delay associated with those conditions. Those delays implement > heuristics for not trimming a region we are relatively likely to fully > overwrite soon. > For my own clarity, this means that there's a basic heuristic of not activating right away all the time? If so, that seems sound. > Signed-off-by: Boris Burkov <boris@bur.io> > --- > fs/btrfs/discard.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c > index 0bc526f5fcd9..c48abc817ed2 100644 > --- a/fs/btrfs/discard.c > +++ b/fs/btrfs/discard.c > @@ -56,8 +56,6 @@ > #define BTRFS_DISCARD_DELAY (120ULL * NSEC_PER_SEC) > #define BTRFS_DISCARD_UNUSED_DELAY (10ULL * NSEC_PER_SEC) > > -/* Target completion latency of discarding all discardable extents */ > -#define BTRFS_DISCARD_TARGET_MSEC (6 * 60 * 60UL * MSEC_PER_SEC) > #define BTRFS_DISCARD_MIN_DELAY_MSEC (1UL) > #define BTRFS_DISCARD_MAX_DELAY_MSEC (1000UL) > #define BTRFS_DISCARD_MAX_IOPS (1000U) > @@ -577,6 +575,7 @@ void btrfs_discard_calc_delay(struct btrfs_discard_ctl *discard_ctl) > s32 discardable_extents; > s64 discardable_bytes; > u32 iops_limit; > + unsigned long min_delay = BTRFS_DISCARD_MIN_DELAY_MSEC; > unsigned long delay; > > discardable_extents = atomic_read(&discard_ctl->discardable_extents); > @@ -607,13 +606,16 @@ void btrfs_discard_calc_delay(struct btrfs_discard_ctl *discard_ctl) > } > > iops_limit = READ_ONCE(discard_ctl->iops_limit); > - if (iops_limit) > + > + if (iops_limit) { > delay = MSEC_PER_SEC / iops_limit; > - else > - delay = BTRFS_DISCARD_TARGET_MSEC / discardable_extents; > + } else { > + /* unset iops_limit means go as fast as possible, so allow a delay of 0 */ > + delay = 0; > + min_delay = 0; > + } > > - delay = clamp(delay, BTRFS_DISCARD_MIN_DELAY_MSEC, > - BTRFS_DISCARD_MAX_DELAY_MSEC); > + delay = clamp(delay, min_delay, BTRFS_DISCARD_MAX_DELAY_MSEC); > discard_ctl->delay_ms = delay; > > spin_unlock(&discard_ctl->lock); > -- > 2.40.0 > LGTM. Reviewed-by: Neal Gompa <neal@gompa.dev>
diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c index 0bc526f5fcd9..c48abc817ed2 100644 --- a/fs/btrfs/discard.c +++ b/fs/btrfs/discard.c @@ -56,8 +56,6 @@ #define BTRFS_DISCARD_DELAY (120ULL * NSEC_PER_SEC) #define BTRFS_DISCARD_UNUSED_DELAY (10ULL * NSEC_PER_SEC) -/* Target completion latency of discarding all discardable extents */ -#define BTRFS_DISCARD_TARGET_MSEC (6 * 60 * 60UL * MSEC_PER_SEC) #define BTRFS_DISCARD_MIN_DELAY_MSEC (1UL) #define BTRFS_DISCARD_MAX_DELAY_MSEC (1000UL) #define BTRFS_DISCARD_MAX_IOPS (1000U) @@ -577,6 +575,7 @@ void btrfs_discard_calc_delay(struct btrfs_discard_ctl *discard_ctl) s32 discardable_extents; s64 discardable_bytes; u32 iops_limit; + unsigned long min_delay = BTRFS_DISCARD_MIN_DELAY_MSEC; unsigned long delay; discardable_extents = atomic_read(&discard_ctl->discardable_extents); @@ -607,13 +606,16 @@ void btrfs_discard_calc_delay(struct btrfs_discard_ctl *discard_ctl) } iops_limit = READ_ONCE(discard_ctl->iops_limit); - if (iops_limit) + + if (iops_limit) { delay = MSEC_PER_SEC / iops_limit; - else - delay = BTRFS_DISCARD_TARGET_MSEC / discardable_extents; + } else { + /* unset iops_limit means go as fast as possible, so allow a delay of 0 */ + delay = 0; + min_delay = 0; + } - delay = clamp(delay, BTRFS_DISCARD_MIN_DELAY_MSEC, - BTRFS_DISCARD_MAX_DELAY_MSEC); + delay = clamp(delay, min_delay, BTRFS_DISCARD_MAX_DELAY_MSEC); discard_ctl->delay_ms = delay; spin_unlock(&discard_ctl->lock);
Currently, a limit of 0 results in a hardcoded metering over 6 hours. Since the default is a set limit, I suspect no one truly depends on this rather arbitrary setting. Repurpose it for an arguably more useful "unlimited" mode, where the delay is 0. Note that if block groups are too new, or go fully empty, there is still a delay associated with those conditions. Those delays implement heuristics for not trimming a region we are relatively likely to fully overwrite soon. Signed-off-by: Boris Burkov <boris@bur.io> --- fs/btrfs/discard.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)