mm/block: add bdi sysfs knobs
Subsystems (4)
v3 cover mm/block: add bdi sysfs knobs
At meta network block devices (nbd) are used to implement remote block storage. In testing and during production it has been observed that these network block devices can consume a huge portion of the dirty writeback cache and writeback can take a considerable time.
To be able to give stricter limits, I'm proposing the following changes:
1) introduce strictlimit knob
Currently the max_ratio knob exists to limit the dirty_memory. However this knob only applies once (dirty_ratio + dirty_background_ratio) / 2 has been reached.
With the BDI_CAP_STRICTLIMIT flag, the max_ratio can be applied without reaching that limit. This change exposes that knob.
This knob can also be useful for NFS, fuse filesystems and USB devices.
2) Use part of 1000 internal calculation
The max_ratio is based on percentage. With the current machine sizes percentage values can be very high (1% of a 256GB main memory is already 2.5GB). This change uses part of 1000 instead of percentages for the internal calculations.
3) Introduce two new sysfs knobs: min_bytes and max_bytes.
Currently all calculations are based on ratio, but for a user it often more convenient to specify a limit in bytes. The new knobs will not store bytes values, instead they will translate the byte value to a corresponding ratio. As the internal values are now part of 1000, the ratio is closer to the specified value. However the value should be more seen as an approximation as it can fluctuate over time.
Changes:
V3:
- change signature of function bdi_ratio_from_pages to take an unsigned long parameter
- use div64_u64 function for division to support 32 bit platforms
- Refreshed to 6.1-rc2
V2:
- Refreshed to 6.1-rc1
- Use part of 1000, instead of part of 10000
- Reformat cover letter
*** BLURB HERE ***
Stefan Roesch (14):
mm: add bdi_set_strict_limit() function
mm: add knob /sys/class/bdi/<bdi>/strict_limit
mm: document /sys/class/bdi/<bdi>/strict_limit knob
mm: use part per 1000 for bdi ratios.
mm: add bdi_get_max_bytes() function
mm: split off __bdi_set_max_ratio() function
mm: add bdi_set_max_bytes() function.
mm: add knob /sys/class/bdi/<bdi>/max_bytes
mm: document /sys/class/bdi/<bdi>/max_bytes knob
mm: add bdi_get_min_bytes() function.
mm: split off __bdi_set_min_ratio() function
mm: add bdi_set_min_bytes() function
mm: add /sys/class/bdi/<bdi>/min_bytes knob
mm: document /sys/class/bdi/<bdi>/min_bytes knob
Documentation/ABI/testing/sysfs-class-bdi | 40 +++++++ include/linux/backing-dev.h | 8 ++ mm/backing-dev.c | 93 +++++++++++++++- mm/page-writeback.c | 127 ++++++++++++++++++++-- 4 files changed, 254 insertions(+), 14 deletions(-)
base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
Signature · 1 line
2.30.2
RFC PATCH v3 1/14 mm: add bdi_set_strict_limit() function
This adds the bdi_set_strict_limit function to be able to set/unset the BDI_CAP_STRICTLIMIT flag.
--- include/linux/backing-dev.h | 1 + mm/page-writeback.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+)
include/linux/backing-dev.h +1 -0
@@ -104,6 +104,7 @@ static inline unsigned long wb_stat_error(void)int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);/* * Flags in backing_dev_info::capability
mm/page-writeback.c +16 -0
@@ -698,6 +698,22 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) }EXPORT_SYMBOL (bdi_set_max_ratio); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit) +{ +if (strict_limit >1 ) +return -EINVAL ; + +spin_lock_bh (&bdi_lock); +if (strict_limit) + bdi->capabilities |=BDI_CAP_STRICTLIMIT ; +else + bdi->capabilities &= ~BDI_CAP_STRICTLIMIT ; +spin_unlock_bh (&bdi_lock); + +return 0 ; +} +EXPORT_SYMBOL_GPL (bdi_set_strict_limit); +static unsigned long dirty_freerun_ceiling (unsigned long thresh,unsigned long bg_thresh) {
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 01/14] mm: add bdi_set_strict_limit() function
Quoted 16 lines · On Mon, 24 Oct 2022 12:05:50 -0700 Stefan Roesch <[email protected]> wrote:
This adds the
bdi_set_strict_limitfunction to be able to set/unset theBDI_CAP_STRICTLIMITflag....
mm/page-writeback.c +5
-0@@ -698,6 +698,22 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) }EXPORT_SYMBOL (bdi_set_max_ratio); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit) +{ +if (strict_limit >1 ) +return -EINVAL ; +
mm/page-writeback.c int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) · line 715
Quoted 10 lines
mm/page-writeback.c +10
-0+spin_lock_bh (&bdi_lock); +if (strict_limit) + bdi->capabilities |=BDI_CAP_STRICTLIMIT ; +else + bdi->capabilities &= ~BDI_CAP_STRICTLIMIT ; +spin_unlock_bh (&bdi_lock); + +return 0 ; +} +EXPORT_SYMBOL_GPL (bdi_set_strict_limit);
I don't believe the export is needed?
Re: [RFC PATCH v3 01/14] mm: add bdi_set_strict_limit() function
Quoted 22 lines · Andrew Morton
Quoted 20 lines · On Mon, 24 Oct 2022 12:05:50 -0700 Stefan Roesch <[email protected]> wrote:
This adds the
bdi_set_strict_limitfunction to be able to set/unset theBDI_CAP_STRICTLIMITflag....
mm/page-writeback.c +9
-0@@ -698,6 +698,22 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) }EXPORT_SYMBOL (bdi_set_max_ratio); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit) +{ +if (strict_limit >1 ) +return -EINVAL ; + +spin_lock_bh (&bdi_lock); +if (strict_limit) + bdi->capabilities |=BDI_CAP_STRICTLIMIT ; +else
Quoted 8 lines
mm/page-writeback.c int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) · line 715
Quoted 6 lines
mm/page-writeback.c +6
-0+ bdi->capabilities &= ~BDI_CAP_STRICTLIMIT ; +spin_unlock_bh (&bdi_lock); + +return 0 ; +} +EXPORT_SYMBOL_GPL (bdi_set_strict_limit);I don't believe the export is needed?
No, the export is not needed, the next version will remove the export.
RFC PATCH v3 2/14 mm: add knob /sys/class/bdi/<bdi>/strict_limit
In reply to Stefan RoeschAdd a new knob to /sys/class/bdi/<bdi>/strict_limit. This new knob allows to set/unset the flag BDI_CAP_STRICTLIMIT in the bdi capabilities.
--- mm/backing-dev.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
mm/backing-dev.c +29 -0
@@ -209,11 +209,40 @@ static ssize_t stable_pages_required_show(struct device *dev, }static DEVICE_ATTR_RO (stable_pages_required); +static ssize_t strict_limit_store (struct device *dev, +struct device_attribute *attr,const char *buf,size_t count) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); +unsigned int strict_limit; +ssize_t ret; + + ret =kstrtouint (buf,10 , &strict_limit); +if (ret <0 ) +return ret; + + ret =bdi_set_strict_limit (bdi, strict_limit); +if (!ret) + ret = count; + +return ret; +} + +static ssize_t strict_limit_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%d\n" , + !!(bdi->capabilities &BDI_CAP_STRICTLIMIT )); +} +static DEVICE_ATTR_RW (strict_limit); +static struct attribute *bdi_dev_attrs[] = { &dev_attr_read_ahead_kb.attr, &dev_attr_min_ratio.attr, &dev_attr_max_ratio.attr, &dev_attr_stable_pages_required.attr, + &dev_attr_strict_limit.attr,NULL , };ATTRIBUTE_GROUPS (bdi_dev);
Signature · 1 line
2.30.2
RFC PATCH v3 3/14 mm: document /sys/class/bdi/<bdi>/strict_limit knob
In reply to Stefan RoeschThis documents the new /sys/class/bdi/<bdi>/strict_limit knob.
--- Documentation/ABI/testing/sysfs-class-bdi | 11 +++++++++++ 1 file changed, 11 insertions(+)
Documentation/ABI/testing/sysfs-class-bdi +11 -0
@@ -55,6 +55,17 @@ Description: mount that is prone to get stuck, or a FUSE mount which cannot be trusted to play fair. + (read-write) +What: /sys/class/bdi/<bdi>/strict_limit +Date: October 2022 +Contact: Stefan Roesch <[email protected]> +Description: + Forces per-BDI checks for the share of given device in the write-back + cache even before the global background dirty limit is reached. This + is useful in situations where the global limit is much higher than + affordable for given relatively slow (or untrusted) device. Turning + strictlimit on has no visible effect if max_ratio is equal to 100%. + (read-write) What: /sys/class/bdi/<bdi>/stable_pages_required Date: January 2008
Signature · 1 line
2.30.2
RFC PATCH v3 4/14 mm: use part per 1000 for bdi ratios.
In reply to Stefan RoeschTo get finer granularity for ratio calculations use part per 1000 instead of percentiles. This is especially important if we want to automatically convert byte values to ratios. Otherwise the values that are actually used can be quite different. This is also important for machines with more main memory (1% of 256GB is already 2.5GB).
--- include/linux/backing-dev.h | 3 +++ mm/backing-dev.c | 6 +++--- mm/page-writeback.c | 15 +++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-)
include/linux/backing-dev.h +3 -0
@@ -102,6 +102,9 @@ static inline unsigned long wb_stat_error(void)#endif } +/* BDI ratio is expressed as part per 1000 for finer granularity. */ +#define BDI_RATIO_SCALE 10 +int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);
mm/backing-dev.c +3 -3
@@ -178,7 +178,7 @@ static ssize_t min_ratio_store(struct device *dev,return ret; } -BDI_SHOW(min_ratio, bdi->min_ratio) +BDI_SHOW(min_ratio, bdi->min_ratio / BDI_RATIO_SCALE)static ssize_t max_ratio_store (struct device *dev,struct device_attribute *attr,const char *buf,size_t count)
@@ -197,7 +197,7 @@ static ssize_t max_ratio_store(struct device *dev,return ret; } -BDI_SHOW(max_ratio, bdi->max_ratio) +BDI_SHOW(max_ratio, bdi->max_ratio / BDI_RATIO_SCALE) static ssize_t stable_pages_required_show(struct device *dev, struct device_attribute *attr,
@@ -809,7 +809,7 @@ int bdi_init(struct backing_dev_info *bdi)kref_init (&bdi->refcnt); bdi->min_ratio =0 ; - bdi->max_ratio =100 ; + bdi->max_ratio =100 *BDI_RATIO_SCALE ; bdi->max_prop_frac =FPROP_FRAC_BASE ;INIT_LIST_HEAD (&bdi->bdi_list);INIT_LIST_HEAD (&bdi->wb_list);
mm/page-writeback.c +9 -6
@@ -197,7 +197,7 @@ static void wb_min_max_ratio(struct bdi_writeback *wb, min *= this_bw; min =div64_ul (min, tot_bw); } -if (max <100 ) { +if (max <100 *BDI_RATIO_SCALE ) { max *= this_bw; max =div64_ul (max, tot_bw); }
@@ -655,6 +655,8 @@ int bdi_set_min_ratio(struct backing_dev_info *bdi, unsigned int min_ratio)unsigned int delta;int ret =0 ; + min_ratio *=BDI_RATIO_SCALE ; +spin_lock_bh (&bdi_lock);if (min_ratio > bdi->max_ratio) { ret = -EINVAL ;
@@ -665,7 +667,7 @@ int bdi_set_min_ratio(struct backing_dev_info *bdi, unsigned int min_ratio) bdi->min_ratio = min_ratio; }else { delta = min_ratio - bdi->min_ratio; -if (bdi_min_ratio + delta <100 ) { +if (bdi_min_ratio + delta <100 *BDI_RATIO_SCALE ) { bdi_min_ratio += delta; bdi->min_ratio = min_ratio; }else {
@@ -684,6 +686,7 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio)if (max_ratio >100 )return -EINVAL ; + max_ratio *=BDI_RATIO_SCALE ;spin_lock_bh (&bdi_lock);if (bdi->min_ratio > max_ratio) {
@@ -776,15 +779,15 @@ static unsigned long __wb_calc_thresh(struct dirty_throttle_control *dtc)fprop_fraction_percpu (&dom->completions, dtc->wb_completions, &numerator, &denominator); - wb_thresh = (thresh * (100 - bdi_min_ratio)) /100 ; + wb_thresh = (thresh * (100 *BDI_RATIO_SCALE - bdi_min_ratio)) / (100 *BDI_RATIO_SCALE ); wb_thresh *= numerator; wb_thresh =div64_ul (wb_thresh, denominator);wb_min_max_ratio (dtc->wb, &wb_min_ratio, &wb_max_ratio); - wb_thresh += (thresh * wb_min_ratio) /100 ; -if (wb_thresh > (thresh * wb_max_ratio) /100 ) - wb_thresh = thresh * wb_max_ratio /100 ; + wb_thresh += (thresh * wb_min_ratio) / (100 *BDI_RATIO_SCALE ); +if (wb_thresh > (thresh * wb_max_ratio) / (100 *BDI_RATIO_SCALE )) + wb_thresh = thresh * wb_max_ratio / (100 *BDI_RATIO_SCALE );return wb_thresh; }
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 04/14] mm: use part per 1000 for bdi ratios.
Quoted 8 lines · On Mon, 24 Oct 2022 12:05:53 -0700 Stefan Roesch <[email protected]> wrote:
To get finer granularity for ratio calculations use part per 1000 instead of percentiles. This is especially important if we want to automatically convert byte values to ratios. Otherwise the values that are actually used can be quite different. This is also important for machines with more main memory (1% of 256GB is already 2.5GB).
...
This changes an existing userspace interface, doesn't it? /sys/class/bdi/<bdi>/min_ratio. Can't do that!
We could add a new interace to the same thing, I guess. /sys/class/bdi/<bdi>/min_ratio_fine or whatever.
We might want to go for more than 100->1000, too. Otherwise in a few years we'll be adding /sys/class/bdi/<bdi>/min_ratio_even_finer.
Also, this patch forgot to updateDocumentation/ABI/testing/sysfs-class-bdi.
Re: [RFC PATCH v3 04/14] mm: use part per 1000 for bdi ratios.
Quoted 8 lines · Andrew Morton
Quoted 6 lines · On Mon, 24 Oct 2022 12:05:53 -0700 Stefan Roesch <[email protected]> wrote:
To get finer granularity for ratio calculations use part per 1000 instead of percentiles. This is especially important if we want to automatically convert byte values to ratios. Otherwise the values that are actually used can be quite different. This is also important for machines with more main memory (1% of 256GB is already 2.5GB).
Quoted 6 lines
Quoted 2 lines
...
This changes an existing userspace interface, doesn't it? /sys/class/bdi/<bdi>/min_ratio. Can't do that!
It does not change the user interface. It maintains the percent values in the min_ratio and max_ratio knobs.
For instance:
-BDI_SHOW(min_ratio, bdi->min_ratio) +BDI_SHOW(min_ratio, bdi->min_ratio / BDI_RATIO_SCALE)
Quoted 6 lines
We could add a new interace to the same thing, I guess. /sys/class/bdi/<bdi>/min_ratio_fine or whatever.
We might want to go for more than 100->1000, too. Otherwise in a few years we'll be adding /sys/class/bdi/<bdi>/min_ratio_even_finer.
The next version of the patch series will also add min_ratio_fine and max_ratio_fine. This makes sure that also ratio values can be specified with a finer granularity.
Quoted 2 lines
Also, this patch forgot to update
Documentation/ABI/testing/sysfs-class-bdi.
The exisiting user behavior is not changed, only the internal calculation.
RFC PATCH v3 5/14 mm: add bdi_get_max_bytes() function
In reply to Stefan RoeschThis adds a function to return the specified value for max_bytes. It converts the stored max_ratio of the bdi to the corresponding bytes value. It introduces the bdi_get_bytes helper function to do the conversion. This is an approximation as it is based on the value that is returned by global_dirty_limits(), which can change. The helper function will also be used by the min_bytes bdi knob.
--- include/linux/backing-dev.h | 1 + mm/page-writeback.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+)
include/linux/backing-dev.h +1 -0
@@ -105,6 +105,7 @@ static inline unsigned long wb_stat_error(void)/* BDI ratio is expressed as part per 1000 for finer granularity. */ #define BDI_RATIO_SCALE 10 +unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);
mm/page-writeback.c +18 -0
@@ -650,6 +650,18 @@ void wb_domain_exit(struct wb_domain *dom)*/ static unsigned int bdi_min_ratio; +static unsigned long long bdi_get_bytes (unsigned int ratio) +{ +unsigned long background_thresh; +unsigned long dirty_thresh; +unsigned long long bytes; + +global_dirty_limits (&background_thresh, &dirty_thresh); + bytes = (dirty_thresh *PAGE_SIZE * ratio) /BDI_RATIO_SCALE /100 ; + +return bytes; +} +int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio) {unsigned int delta;
@@ -701,6 +713,12 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio) }EXPORT_SYMBOL (bdi_set_max_ratio); +unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi) +{ +return bdi_get_bytes (bdi->max_ratio); +} +EXPORT_SYMBOL_GPL (bdi_get_max_bytes); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit) {if (strict_limit >1 )
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 05/14] mm: add bdi_get_max_bytes() function
Quoted 10 lines · On Mon, 24 Oct 2022 12:05:54 -0700 Stefan Roesch <[email protected]> wrote:
This adds a function to return the specified value for
max_bytes. It converts the storedmax_ratioof the bdi to the corresponding bytes value. It introduces thebdi_get_byteshelper function to do the conversion. This is an approximation as it is based on the value that is returned byglobal_dirty_limits(), which can change. The helper function will also be used by themin_bytesbdi knob.
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 108
Quoted 4 lines
include/linux/backing-dev.h +1
-0@@ -105,6 +105,7 @@ static inline unsigned long wb_stat_error(void)/* BDI ratio is expressed as part per 1000 for finer granularity. */ #define BDI_RATIO_SCALE 10 +unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);
We don't use unsigned long long much. If you want a 64-bit unsigned, use u64?
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 109
Quoted 1 line
include/linux/backing-dev.h +1
-0+EXPORT_SYMBOL_GPL (bdi_get_max_bytes);
Is this symbol to be used by modules?
Re: [RFC PATCH v3 05/14] mm: add bdi_get_max_bytes() function
Quoted 15 lines · Andrew Morton
Quoted 13 lines · On Mon, 24 Oct 2022 12:05:54 -0700 Stefan Roesch <[email protected]> wrote:
This adds a function to return the specified value for
max_bytes. It converts the storedmax_ratioof the bdi to the corresponding bytes value. It introduces thebdi_get_byteshelper function to do the conversion. This is an approximation as it is based on the value that is returned byglobal_dirty_limits(), which can change. The helper function will also be used by themin_bytesbdi knob.include/linux/backing-dev.h +0
-0@@ -105,6 +105,7 @@ static inline unsigned long wb_stat_error(void)/* BDI ratio is expressed as part per 1000 for finer granularity. */ #define BDI_RATIO_SCALE 10
Quoted 5 lines
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 108
Quoted 1 line
include/linux/backing-dev.h +1
-0+unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);We don't use unsigned long long much. If you want a 64-bit unsigned, use u64?
The next version will use u64.
Quoted 3 lines
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 109
Quoted 1 line
include/linux/backing-dev.h +1
-0+EXPORT_SYMBOL_GPL (bdi_get_max_bytes);Is this symbol to be used by modules?
The next version will remove the export.
RFC PATCH v3 6/14 mm: split off __bdi_set_max_ratio() function
In reply to Stefan RoeschThis splits off the __bdi_set_max_ratio() function from the bdi_set_max_ratio() function. The __bdi_set_max_ratio() function will also be called from the bdi_set_max_bytes() function, which will be introduced in the next patch.
--- mm/page-writeback.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
mm/page-writeback.c +9 -5
@@ -692,14 +692,10 @@ int bdi_set_min_ratio(struct backing_dev_info *bdi, unsigned int min_ratio)return ret; } -int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned max_ratio) +static int __bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio) {int ret =0 ; -if (max_ratio >100 ) -return -EINVAL ; - max_ratio *=BDI_RATIO_SCALE ; -spin_lock_bh (&bdi_lock);if (bdi->min_ratio > max_ratio) { ret = -EINVAL ;
@@ -711,6 +707,14 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned max_ratio)return ret; } + +int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio) +{ +if (max_ratio >100 ) +return -EINVAL ; + +return __bdi_set_max_ratio (bdi, max_ratio *BDI_RATIO_SCALE ); +}EXPORT_SYMBOL (bdi_set_max_ratio);unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi)
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 06/14] mm: split off __bdi_set_max_ratio() function
Quoted 1 line · On Mon, 24 Oct 2022 12:05:55 -0700 Stefan Roesch <[email protected]> wrote:
This splits off the
__bdi_set_max_ratio()function from the
nit: it saves a few electrons to use "foo()" in place of "the foo() function". No information is lost in this substitution.
RFC PATCH v3 7/14 mm: add bdi_set_max_bytes() function.
In reply to Stefan RoeschThis introduces the bdi_set_max_bytes() function. The max_bytes function does not store the max_bytes value. Instead it converts the max_bytes value into the corresponding ratio value.
--- include/linux/backing-dev.h | 1 + mm/page-writeback.c | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)
include/linux/backing-dev.h +1 -0
@@ -108,6 +108,7 @@ static inline unsigned long wb_stat_error(void)unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio); +int bdi_set_max_bytes (struct backing_dev_info *bdi,unsigned long long max_bytes);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);/*
mm/page-writeback.c +38 -0
@@ -13,6 +13,7 @@*/ #include <linux/kernel.h> +#include <linux/math64.h> #include <linux/export.h> #include <linux/spinlock.h> #include <linux/fs.h>
@@ -650,6 +651,28 @@ void wb_domain_exit(struct wb_domain *dom)*/ static unsigned int bdi_min_ratio; +static int bdi_check_pages_limit (unsigned long pages) +{ +unsigned long max_dirty_pages =global_dirtyable_memory (); + +if (pages > max_dirty_pages /2 ) +return -EINVAL ; + +return 0 ; +} + +static unsigned long bdi_ratio_from_pages (unsigned long pages) +{ +unsigned long background_thresh; +unsigned long dirty_thresh; +unsigned long ratio; + +global_dirty_limits (&background_thresh, &dirty_thresh); + ratio =div64_u64 (pages *100ULL *BDI_RATIO_SCALE , dirty_thresh); + +return ratio; +} +static unsigned long long bdi_get_bytes (unsigned int ratio) {unsigned long background_thresh;
@@ -723,6 +746,21 @@ unsigned long long bdi_get_max_bytes(struct backing_dev_info *bdi) }EXPORT_SYMBOL_GPL (bdi_get_max_bytes); +int bdi_set_max_bytes (struct backing_dev_info *bdi,unsigned long long max_bytes) +{ +int ret; +unsigned long pages = max_bytes >>PAGE_SHIFT ; +unsigned long max_ratio; + + ret =bdi_check_pages_limit (pages); +if (ret) +return ret; + + max_ratio =bdi_ratio_from_pages (pages); +return __bdi_set_max_ratio (bdi, max_ratio); +} +EXPORT_SYMBOL_GPL (bdi_set_max_bytes); +int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit) {if (strict_limit >1 )
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 07/14] mm: add bdi_set_max_bytes() function.
Quoted 7 lines · On Mon, 24 Oct 2022 12:05:56 -0700 Stefan Roesch <[email protected]> wrote:
This introduces the
bdi_set_max_bytes()function. Themax_bytesfunction does not store themax_bytesvalue. Instead it converts themax_bytesvalue into the corresponding ratio value....
+EXPORT_SYMBOL_GPL(bdi_set_max_bytes);
Is this needed?
Re: [RFC PATCH v3 07/14] mm: add bdi_set_max_bytes() function.
Quoted 11 lines · Andrew Morton
Quoted 7 lines · On Mon, 24 Oct 2022 12:05:56 -0700 Stefan Roesch <[email protected]> wrote:
This introduces the
bdi_set_max_bytes()function. Themax_bytesfunction does not store themax_bytesvalue. Instead it converts themax_bytesvalue into the corresponding ratio value....
+EXPORT_SYMBOL_GPL(bdi_set_max_bytes);
Is this needed?
I removed the export in the next version.
Andrew Morton
maintainerRe: [RFC PATCH v3 07/14] mm: add bdi_set_max_bytes() function.
In reply to Stefan RoeschQuoted 31 lines · On Mon, 24 Oct 2022 12:05:56 -0700 Stefan Roesch <[email protected]> wrote:
This introduces the
bdi_set_max_bytes()function. Themax_bytesfunction does not store themax_bytesvalue. Instead it converts themax_bytesvalue into the corresponding ratio value....
include/linux/backing-dev.h +1
-0@@ -108,6 +108,7 @@ static inline unsigned long wb_stat_error(void)unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio); +int bdi_set_max_bytes (struct backing_dev_info *bdi,unsigned long long max_bytes);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);/* mm/page-writeback.c +1
-0@@ -13,6 +13,7 @@*/ #include <linux/kernel.h> +#include <linux/math64.h> #include <linux/export.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -650,6 +651,28 @@ void wb_domain_exit(struct wb_domain *dom)*/ static unsigned int bdi_min_ratio;
mm/page-writeback.c void wb_domain_exit(struct wb_domain *dom) · line 662
Quoted 10 lines
mm/page-writeback.c +9
-0+static int bdi_check_pages_limit (unsigned long pages) +{ +unsigned long max_dirty_pages =global_dirtyable_memory (); + +if (pages > max_dirty_pages /2 ) +return -EINVAL ; + +return 0 ; +}
Some code comments are needed here. Explain what it does and why it does it. The "/ 2" seems utterly arbitray - explain why this value was chosen? Why is it better than "/ 3"?
mm/page-writeback.c void wb_domain_exit(struct wb_domain *dom) · line 671
Quoted 8 lines
mm/page-writeback.c +8
-0+static unsigned long bdi_ratio_from_pages (unsigned long pages) +{ +unsigned long background_thresh; +unsigned long dirty_thresh; +unsigned long ratio; + +global_dirty_limits (&background_thresh, &dirty_thresh); + ratio =div64_u64 (pages *100ULL *BDI_RATIO_SCALE , dirty_thresh);
`unsigned long' is 32-bit on 32-bit machines, which makes this code a bit odd. Should everything here be u64?
Re: [RFC PATCH v3 07/14] mm: add bdi_set_max_bytes() function.
Quoted 34 lines · Andrew Morton
Quoted 32 lines · On Mon, 24 Oct 2022 12:05:56 -0700 Stefan Roesch <[email protected]> wrote:
This introduces the
bdi_set_max_bytes()function. Themax_bytesfunction does not store themax_bytesvalue. Instead it converts themax_bytesvalue into the corresponding ratio value....
include/linux/backing-dev.h +1
-0@@ -108,6 +108,7 @@ static inline unsigned long wb_stat_error(void)unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio); +int bdi_set_max_bytes (struct backing_dev_info *bdi,unsigned long long max_bytes);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);/* mm/page-writeback.c +1
-0@@ -13,6 +13,7 @@*/ #include <linux/kernel.h> +#include <linux/math64.h> #include <linux/export.h> #include <linux/spinlock.h> #include <linux/fs.h> @@ -650,6 +651,28 @@ void wb_domain_exit(struct wb_domain *dom)*/ static unsigned int bdi_min_ratio;
Quoted 15 lines
mm/page-writeback.c void wb_domain_exit(struct wb_domain *dom) · line 662
Quoted 9 lines
mm/page-writeback.c +9
-0+static int bdi_check_pages_limit (unsigned long pages) +{ +unsigned long max_dirty_pages =global_dirtyable_memory (); + +if (pages > max_dirty_pages /2 ) +return -EINVAL ; + +return 0 ; +}Some code comments are needed here. Explain what it does and why it does it. The "/ 2" seems utterly arbitray - explain why this value was chosen? Why is it better than "/ 3"?
I changed the check to
(pages > max_dirty_pages)
Quoted 12 lines
mm/page-writeback.c void wb_domain_exit(struct wb_domain *dom) · line 671
Quoted 9 lines
mm/page-writeback.c +8
-0+static unsigned long bdi_ratio_from_pages (unsigned long pages) +{ +unsigned long background_thresh; +unsigned long dirty_thresh; +unsigned long ratio; + +global_dirty_limits (&background_thresh, &dirty_thresh); + ratio =div64_u64 (pages *100ULL *BDI_RATIO_SCALE , dirty_thresh);`unsigned long' is 32-bit on 32-bit machines, which makes this code a bit odd. Should everything here be u64?
The function global_dirty_limits() uses unsigned long pointers for its arguments. unsigned long looks like a better fit. Any thoughts?
RFC PATCH v3 8/14 mm: add knob /sys/class/bdi/<bdi>/max_bytes
In reply to Stefan RoeschThis adds the new knob max_bytes to specify a dirty memory limit for the corresponding bdi. The specified bytes value is converted to a ratio.
--- mm/backing-dev.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
mm/backing-dev.c +29 -0
@@ -199,6 +199,34 @@ static ssize_t max_ratio_store(struct device *dev, } BDI_SHOW(max_ratio, bdi->max_ratio / BDI_RATIO_SCALE) +static ssize_t max_bytes_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%llu\n" ,bdi_get_max_bytes (bdi)); +} + +static ssize_t max_bytes_store (struct device *dev, +struct device_attribute *attr,const char *buf,size_t count) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); +unsigned long long bytes; +ssize_t ret; + + ret =kstrtoull (buf,10 , &bytes); +if (ret <0 ) +return ret; + + ret =bdi_set_max_bytes (bdi, bytes); +if (!ret) + ret = count; + +return ret; +} +DEVICE_ATTR_RW (max_bytes); +static ssize_t stable_pages_required_show (struct device *dev,struct device_attribute *attr,char *buf)
@@ -241,6 +269,7 @@ static struct attribute *bdi_dev_attrs[] = { &dev_attr_read_ahead_kb.attr, &dev_attr_min_ratio.attr, &dev_attr_max_ratio.attr, + &dev_attr_max_bytes.attr, &dev_attr_stable_pages_required.attr, &dev_attr_strict_limit.attr, NULL,
Signature · 1 line
2.30.2
RFC PATCH v3 9/14 mm: document /sys/class/bdi/<bdi>/max_bytes knob
In reply to Stefan RoeschThis documents the new /sys/class/bdi/<bdi>/max_bytes knob.
--- Documentation/ABI/testing/sysfs-class-bdi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Documentation/ABI/testing/sysfs-class-bdi +14 -0
@@ -56,6 +56,20 @@ Description: be trusted to play fair. (read-write) + +What: /sys/class/bdi/<bdi>/max_bytes +Date: October 2022 +Contact: Stefan Roesch <[email protected]> +Description: + Allows limiting a particular device to use not more than the + given 'max_bytes' of the write-back cache. This is useful in + situations where we want to avoid one device taking all or + most of the write-back cache. For example in case of an NFS + mount that is prone to get stuck, a FUSE mount which cannot be + trusted to play fair, or a nbd device. + + (read-write) + What: /sys/class/bdi/<bdi>/strict_limit Date: October 2022 Contact: Stefan Roesch <[email protected]>
Signature · 1 line
2.30.2
RFC PATCH v3 10/14 mm: add bdi_get_min_bytes() function.
In reply to Stefan RoeschThis adds a function to return the specified value for min_bytes. It converts the stored min_ratio of the bdi to the corresponding bytes value. This is an approximation as it is based on the value that is returned by global_dirty_limits(), which can change. The returned value can be different than the value when the min_bytes value was set.
--- include/linux/backing-dev.h | 1 + mm/page-writeback.c | 6 ++++++ 2 files changed, 7 insertions(+)
include/linux/backing-dev.h +1 -0
@@ -105,6 +105,7 @@ static inline unsigned long wb_stat_error(void)/* BDI ratio is expressed as part per 1000 for finer granularity. */ #define BDI_RATIO_SCALE 10 +unsigned long long bdi_get_min_bytes (struct backing_dev_info *bdi);unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio);
mm/page-writeback.c +6 -0
@@ -740,6 +740,12 @@ int bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned int max_ratio) }EXPORT_SYMBOL (bdi_set_max_ratio); +unsigned long long bdi_get_min_bytes (struct backing_dev_info *bdi) +{ +return bdi_get_bytes (bdi->min_ratio); +} +EXPORT_SYMBOL_GPL (bdi_get_min_bytes); +unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi) {return bdi_get_bytes (bdi->max_ratio);
Signature · 1 line
2.30.2
Andrew Morton
maintainerRe: [RFC PATCH v3 10/14] mm: add bdi_get_min_bytes() function.
Quoted 11 lines · On Mon, 24 Oct 2022 12:05:59 -0700 Stefan Roesch <[email protected]> wrote:
This adds a function to return the specified value for
min_bytes. It converts the storedmin_ratioof the bdi to the corresponding bytes value. This is an approximation as it is based on the value that is returned byglobal_dirty_limits(), which can change. The returned value can be different than the value when themin_bytesvalue was set....
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 108
Quoted 4 lines
include/linux/backing-dev.h +1
-0@@ -105,6 +105,7 @@ static inline unsigned long wb_stat_error(void)/* BDI ratio is expressed as part per 1000 for finer granularity. */ #define BDI_RATIO_SCALE 10 +unsigned long long bdi_get_min_bytes (struct backing_dev_info *bdi);
u64?
include/linux/backing-dev.h static inline unsigned long wb_stat_error(void) · line 109
Quoted 1 line
include/linux/backing-dev.h +1
-0+EXPORT_SYMBOL_GPL (bdi_get_min_bytes);
Needed?
RFC PATCH v3 11/14 mm: split off __bdi_set_min_ratio() function
In reply to Stefan RoeschThis splits off the __bdi_set_min_ratio() function from the bdi_set_min_ratio() function. The __bdi_set_min_ratio() function will also be called from the bdi_set_min_bytes() function, which will be introduced in the next patch.
--- mm/page-writeback.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
mm/page-writeback.c +6 -1
@@ -685,7 +685,7 @@ static unsigned long long bdi_get_bytes(unsigned int ratio)return bytes; } -int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio) +static int __bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio) {unsigned int delta;int ret =0 ;
@@ -731,6 +731,11 @@ static int __bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned int max_rareturn ret; } +int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio) +{ +return __bdi_set_min_ratio (bdi, min_ratio *BDI_RATIO_SCALE ); +} +int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio) {if (max_ratio >100 )
Signature · 1 line
2.30.2
RFC PATCH v3 12/14 mm: add bdi_set_min_bytes() function
In reply to Stefan RoeschThis introduces the bdi_set_min_bytes() function. The min_bytes function does not store the min_bytes value. Instead it converts the min_bytes value into the corresponding ratio value.
--- include/linux/backing-dev.h | 1 + mm/page-writeback.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)
include/linux/backing-dev.h +1 -0
@@ -109,6 +109,7 @@ unsigned long long bdi_get_min_bytes(struct backing_dev_info *bdi);unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi);int bdi_set_min_ratio (struct backing_dev_info *bdi,unsigned int min_ratio);int bdi_set_max_ratio (struct backing_dev_info *bdi,unsigned int max_ratio); +int bdi_set_min_bytes (struct backing_dev_info *bdi,unsigned long long min_bytes);int bdi_set_max_bytes (struct backing_dev_info *bdi,unsigned long long max_bytes);int bdi_set_strict_limit (struct backing_dev_info *bdi,unsigned int strict_limit);
mm/page-writeback.c +15 -0
@@ -751,6 +751,21 @@ unsigned long long bdi_get_min_bytes(struct backing_dev_info *bdi) }EXPORT_SYMBOL_GPL (bdi_get_min_bytes); +int bdi_set_min_bytes (struct backing_dev_info *bdi,unsigned long long min_bytes) +{ +int ret; +unsigned long pages = min_bytes >>PAGE_SHIFT ; +unsigned long min_ratio; + + ret =bdi_check_pages_limit (pages); +if (ret) +return ret; + + min_ratio =bdi_ratio_from_pages (pages); +return __bdi_set_min_ratio (bdi, min_ratio); +} +EXPORT_SYMBOL_GPL (bdi_set_min_bytes); +unsigned long long bdi_get_max_bytes (struct backing_dev_info *bdi) {return bdi_get_bytes (bdi->max_ratio);
Signature · 1 line
2.30.2
RFC PATCH v3 13/14 mm: add /sys/class/bdi/<bdi>/min_bytes knob
In reply to Stefan Roeschbdi has two existing knobs to limit the amount of dirty memory:min_ratio and max_ratio. However the granularity of the knobs is limited and often it is more convenient to specify limits in terms of bytes. This change adds the min_bytes knob.
It does not store the min_bytes value, instead it converts the max_bytes value to a ratio. The value is therefore more an approximation than an absolute value.
It also maintains the sum over all the bdi min_ratio values stored in the variable bdi_min_ratio.
--- mm/backing-dev.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
mm/backing-dev.c +29 -0
@@ -199,6 +199,34 @@ static ssize_t max_ratio_store(struct device *dev, } BDI_SHOW(max_ratio, bdi->max_ratio / BDI_RATIO_SCALE) +static ssize_t min_bytes_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); + +return sysfs_emit (buf,"%llu\n" ,bdi_get_min_bytes (bdi)); +} + +static ssize_t min_bytes_store (struct device *dev, +struct device_attribute *attr,const char *buf,size_t count) +{ +struct backing_dev_info *bdi =dev_get_drvdata (dev); +unsigned long long bytes; +ssize_t ret; + + ret =kstrtoull (buf,10 , &bytes); +if (ret <0 ) +return ret; + + ret =bdi_set_min_bytes (bdi, bytes); +if (!ret) + ret = count; + +return ret; +} +DEVICE_ATTR_RW (min_bytes); +static ssize_t max_bytes_show (struct device *dev,struct device_attribute *attr,char *buf)
@@ -269,6 +297,7 @@ static struct attribute *bdi_dev_attrs[] = { &dev_attr_read_ahead_kb.attr, &dev_attr_min_ratio.attr, &dev_attr_max_ratio.attr, + &dev_attr_min_bytes.attr, &dev_attr_max_bytes.attr, &dev_attr_stable_pages_required.attr, &dev_attr_strict_limit.attr,
Signature · 1 line
2.30.2
RFC PATCH v3 14/14 mm: document /sys/class/bdi/<bdi>/min_bytes knob
In reply to Stefan RoeschThis documents the new /sys/class/bdi/<bdi>/min_bytes knob.
--- Documentation/ABI/testing/sysfs-class-bdi | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
Documentation/ABI/testing/sysfs-class-bdi +15 -0
@@ -57,6 +57,21 @@ Description: (read-write) +What: /sys/class/bdi/<bdi>/min_bytes +Date: October 2022 +Contact: Stefan Roesch <[email protected]> +Description: + Under normal circumstances each device is given a part of the + total write-back cache that relates to its current average + writeout speed in relation to the other devices. + + The 'min_bytes' parameter allows assigning a minimum + percentage of the write-back cache to a particular device + expressed in bytes. + For example, this is useful for providing a minimum QoS. + + (read-write) + What: /sys/class/bdi/<bdi>/max_bytes Date: October 2022 Contact: Stefan Roesch <[email protected]>
Signature · 1 line
2.30.2
Jens Axboe
maintainerQuoted 24 lines · Stefan Roesch
At meta network block devices (nbd) are used to implement remote block storage. In testing and during production it has been observed that these network block devices can consume a huge portion of the dirty writeback cache and writeback can take a considerable time.
To be able to give stricter limits, I'm proposing the following changes:
1) introduce strictlimit knob
Currently the
max_ratioknob exists to limit thedirty_memory. However this knob only applies once (dirty_ratio+dirty_background_ratio) / 2 has been reached.
With theBDI_CAP_STRICTLIMITflag, themax_ratiocan be applied without reaching that limit. This change exposes that knob.This knob can also be useful for NFS, fuse filesystems and USB devices.
2) Use part of 1000 internal calculation
The
max_ratiois based on percentage. With the current machine sizes percentage values can be very high (1% of a 256GB main memory is already 2.5GB). This change uses part of 1000 instead of percentages for the internal calculations.
Quoted 8 lines
3) Introduce two new sysfs knobs:
min_bytesandmax_bytes.Currently all calculations are based on ratio, but for a user it often more convenient to specify a limit in bytes. The new knobs will not store bytes values, instead they will translate the byte value to a corresponding ratio. As the internal values are now part of 1000, the ratio is closer to the specified value. However the value should be more seen as an approximation as it can fluctuate over time.
Anyone have any concerns or input on this series? Would be nice to get it moving forward, as we do have a need for it at Meta. Outside of that, would be applicable for end-user use cases as well on the distro side.
Signature · 1 line
Jens Axboe