Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> writes:
> On Mon, 24 Oct 2022 12:05:56 -0700 Stefan Roesch <shr@xxxxxxxxxxxx> wrote:
>
>> This 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.
>>
>> ...
>>
>> --- a/include/linux/backing-dev.h
>> +++ b/include/linux/backing-dev.h
>> @@ -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);
>>
>> /*
>> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
>> index 8b8936603783..21d7c1880ea8 100644
>> --- a/mm/page-writeback.c
>> +++ b/mm/page-writeback.c
>> @@ -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;
>> +}
>
> 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)
>
>> +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?
