From e519ce7a26b4c877d834e4234e8d24478448c0d3 Mon Sep 17 00:00:00 2001 From: Feng Tang Date: Wed, 20 Sep 2023 15:44:13 +0800 Subject: mm/slub: add sanity check for slub_min/max_order cmdline setup Currently there are 2 parameters could be setup from kernel cmdline: slub_min_order and slub_max_order. It's possible that the user configured slub_min_order is bigger than the default slub_max_order [1], which can still take effect, as calculate_oder() will use MAX_ORDER as a fallback to check against, but has some downsides: * the kernel message about SLUB will be strange in showing min/max orders: SLUB: HWalign=64, Order=9-3, MinObjects=0, CPUs=16, Nodes=1 * in calculate_order() called by each slab, the 2 loops of calc_slab_order() will all be meaningless due to slub_min_order is bigger than slub_max_order * prevent future code cleanup like in [2]. Fix it by adding some sanity check to enforce the min/max semantics. [1]. https://lore.kernel.org/lkml/21a0ba8b-bf05-0799-7c78-2a35f8c8d52a@os.amperecomputing.com/ [2]. https://lore.kernel.org/lkml/20230908145302.30320-7-vbabka@suse.cz/ Signed-off-by: Feng Tang Signed-off-by: Vlastimil Babka --- mm/slub.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mm/slub.c b/mm/slub.c index f7940048138c..b36e5eb0ccb7 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4711,6 +4711,9 @@ static int __init setup_slub_min_order(char *str) { get_option(&str, (int *)&slub_min_order); + if (slub_min_order > slub_max_order) + slub_max_order = slub_min_order; + return 1; } @@ -4721,6 +4724,9 @@ static int __init setup_slub_max_order(char *str) get_option(&str, (int *)&slub_max_order); slub_max_order = min_t(unsigned int, slub_max_order, MAX_ORDER); + if (slub_min_order > slub_max_order) + slub_min_order = slub_max_order; + return 1; } -- cgit v1.2.3-70-g09d2 From c7355d755698a01ff4187a0d2f6ad21ba233dc21 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Fri, 8 Sep 2023 09:57:13 +0200 Subject: mm/slub: simplify the last resort slab order calculation If calculate_order() can't fit even a single large object within slub_max_order, it will try using the smallest necessary order that may exceed slub_max_order but not MAX_ORDER. Currently this is done with a call to calc_slab_order() which is unnecessary. We can simply use get_order(size). No functional change. Signed-off-by: Vlastimil Babka Reviewed-by: Feng Tang Reviewed-and-tested-by: Jay Patel --- mm/slub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/slub.c b/mm/slub.c index b36e5eb0ccb7..0710adb5642a 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4193,7 +4193,7 @@ static inline int calculate_order(unsigned int size) /* * Doh this slab cannot be placed using slub_max_order. */ - order = calc_slab_order(size, 1, MAX_ORDER, 1); + order = get_order(size); if (order <= MAX_ORDER) return order; return -ENOSYS; -- cgit v1.2.3-70-g09d2 From 0fe2735d5e2e00601339aab3658e05f3707a1745 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Fri, 8 Sep 2023 10:53:26 +0200 Subject: mm/slub: remove min_objects loop from calculate_order() calculate_order() currently has two nested loops. The inner one that gradually modifies the acceptable waste from 1/16 up to 1/4, and the outer one that decreases min_objects down to 2. Upon closer inspection, the outer loop is unnecessary. Decreasing min_objects could have in theory two effects to make the inner loop and its call to calc_slab_order() succeed where a previous iteration with higher min_objects would not: - it could cause the min_objects-derived min_order to fit within slub_max_order. But min_objects is already pre-capped to max_objects that's derived from slub_max_order above the loops, so every iteration tries at least slub_max_order in calc_slab_order() - it could cause calc_slab_order() to be called with lower min_objects thus potentially lower min_order in its loop. This would make a difference if the lower order could cause the fractional waste test to succeed where a higher order has already failed with same fract_leftover in the previous iteration with a higher min_order. But that's not possible, because increasing the order can only result in lower (or same) fractional waste. If we increase the slab size 2 times, we will fit at least 2 times the number of objects (thus same fraction of waste), or it will allow us to fit one more object (lower fraction of waste). For more confidence I have tried adding a printk to notify when decreasing min_objects resulted in a success, and simulated calculations for a range of object sizes, nr_cpus and page_sizes. As expected, the printk never triggered. Thus remove the outer loop and adjust comments accordingly. There's almost no functional change except a weird corner case when slub_min_objects=1 on boot command line would cause the whole two nested loops to be skipped before this patch. Now it would try to find the best layout as usual, resulting in potentially higher orderthat minimizes waste. This is not wrong and will be further expanded by the next patch. Signed-off-by: Vlastimil Babka Reviewed-by: Feng Tang Reviewed-and-tested-by: Jay Patel --- mm/slub.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 0710adb5642a..c4b5f48149e8 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4141,14 +4141,6 @@ static inline int calculate_order(unsigned int size) unsigned int max_objects; unsigned int nr_cpus; - /* - * Attempt to find best configuration for a slab. This - * works by first attempting to generate a layout with - * the best configuration and backing off gradually. - * - * First we increase the acceptable waste in a slab. Then - * we reduce the minimum objects required in a slab. - */ min_objects = slub_min_objects; if (!min_objects) { /* @@ -4168,18 +4160,24 @@ static inline int calculate_order(unsigned int size) max_objects = order_objects(slub_max_order, size); min_objects = min(min_objects, max_objects); - while (min_objects > 1) { - unsigned int fraction; - - fraction = 16; - while (fraction >= 4) { - order = calc_slab_order(size, min_objects, - slub_max_order, fraction); - if (order <= slub_max_order) - return order; - fraction /= 2; - } - min_objects--; + /* + * Attempt to find best configuration for a slab. This works by first + * attempting to generate a layout with the best possible configuration + * and backing off gradually. + * + * We start with accepting at most 1/16 waste and try to find the + * smallest order from min_objects-derived/slub_min_order up to + * slub_max_order that will satisfy the constraint. Note that increasing + * the order can only result in same or less fractional waste, not more. + * + * If that fails, we increase the acceptable fraction of waste and try + * again. + */ + for (unsigned int fraction = 16; fraction >= 4; fraction /= 2) { + order = calc_slab_order(size, min_objects, slub_max_order, + fraction); + if (order <= slub_max_order) + return order; } /* -- cgit v1.2.3-70-g09d2 From 5886fc82b6e3166dd1ba876809888fc39028d626 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Fri, 8 Sep 2023 11:47:09 +0200 Subject: mm/slub: attempt to find layouts up to 1/2 waste in calculate_order() The main loop in calculate_order() currently tries to find an order with at most 1/4 waste. If that's impossible (for particular large object sizes), there's a fallback that will try to place one object within slab_max_order. If we expand the loop boundary to also allow up to 1/2 waste as the last resort, we can remove the fallback and simplify the code, as the loop will find an order for such sizes as well. Note we don't need to allow more than 1/2 waste as that will never happen - calc_slab_order() would calculate more objects to fit, reducing waste below 1/2. Successfully finding an order in the loop (compared to the fallback) will also have the benefit in trying to satisfy min_objects, because the fallback was passing 1. Thus the resulting slab orders might be larger (not because it would improve waste, but to reduce pressure on shared locks), which is one of the goals of calculate_order(). For example, with nr_cpus=1 and 4kB PAGE_SIZE, slub_max_order=3, before the patch we would get the following orders for these object sizes: 2056 to 10920 - order-3 as selected by the loop 10928 to 12280 - order-2 due to fallback, as <1/4 waste is not possible 12288 to 32768 - order-3 as <1/4 waste is again possible After the patch: 2056 to 32768 - order-3, because even in the range of 10928 to 12280 we try to satisfy the calculated min_objects. As a result the code is simpler and gives more consistent results. Signed-off-by: Vlastimil Babka Reviewed-by: Feng Tang Reviewed-and-tested-by: Jay Patel --- mm/slub.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index c4b5f48149e8..86141e5164ca 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4171,23 +4171,17 @@ static inline int calculate_order(unsigned int size) * the order can only result in same or less fractional waste, not more. * * If that fails, we increase the acceptable fraction of waste and try - * again. + * again. The last iteration with fraction of 1/2 would effectively + * accept any waste and give us the order determined by min_objects, as + * long as at least single object fits within slub_max_order. */ - for (unsigned int fraction = 16; fraction >= 4; fraction /= 2) { + for (unsigned int fraction = 16; fraction > 1; fraction /= 2) { order = calc_slab_order(size, min_objects, slub_max_order, fraction); if (order <= slub_max_order) return order; } - /* - * We were unable to place multiple objects in a slab. Now - * lets see if we can place a single object there. - */ - order = calc_slab_order(size, 1, slub_max_order, 1); - if (order <= slub_max_order) - return order; - /* * Doh this slab cannot be placed using slub_max_order. */ -- cgit v1.2.3-70-g09d2 From 90f055df112162fd9e093c16be1c21f38c35b907 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Fri, 8 Sep 2023 12:18:09 +0200 Subject: mm/slub: refactor calculate_order() and calc_slab_order() After the previous cleanups, we can now move some code from calc_slab_order() to calculate_order() so it's executed just once, and do some more cleanups. - move the min_order and MAX_OBJS_PER_PAGE evaluation to calculate_order(). - change calc_slab_order() parameter min_objects to min_order Also make MAX_OBJS_PER_PAGE check more robust by considering also min_objects in addition to slub_min_order. Otherwise this is not a functional change. Signed-off-by: Vlastimil Babka Reviewed-by: Feng Tang Reviewed-and-tested-by: Jay Patel --- mm/slub.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 86141e5164ca..63d281dfacdb 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4110,17 +4110,12 @@ static unsigned int slub_min_objects; * the smallest order which will fit the object. */ static inline unsigned int calc_slab_order(unsigned int size, - unsigned int min_objects, unsigned int max_order, + unsigned int min_order, unsigned int max_order, unsigned int fract_leftover) { - unsigned int min_order = slub_min_order; unsigned int order; - if (order_objects(min_order, size) > MAX_OBJS_PER_PAGE) - return get_order(size * MAX_OBJS_PER_PAGE) - 1; - - for (order = max(min_order, (unsigned int)get_order(min_objects * size)); - order <= max_order; order++) { + for (order = min_order; order <= max_order; order++) { unsigned int slab_size = (unsigned int)PAGE_SIZE << order; unsigned int rem; @@ -4139,7 +4134,7 @@ static inline int calculate_order(unsigned int size) unsigned int order; unsigned int min_objects; unsigned int max_objects; - unsigned int nr_cpus; + unsigned int min_order; min_objects = slub_min_objects; if (!min_objects) { @@ -4152,14 +4147,20 @@ static inline int calculate_order(unsigned int size) * order on systems that appear larger than they are, and too * low order on systems that appear smaller than they are. */ - nr_cpus = num_present_cpus(); + unsigned int nr_cpus = num_present_cpus(); if (nr_cpus <= 1) nr_cpus = nr_cpu_ids; min_objects = 4 * (fls(nr_cpus) + 1); } - max_objects = order_objects(slub_max_order, size); + /* min_objects can't be 0 because get_order(0) is undefined */ + max_objects = max(order_objects(slub_max_order, size), 1U); min_objects = min(min_objects, max_objects); + min_order = max_t(unsigned int, slub_min_order, + get_order(min_objects * size)); + if (order_objects(min_order, size) > MAX_OBJS_PER_PAGE) + return get_order(size * MAX_OBJS_PER_PAGE) - 1; + /* * Attempt to find best configuration for a slab. This works by first * attempting to generate a layout with the best possible configuration @@ -4176,7 +4177,7 @@ static inline int calculate_order(unsigned int size) * long as at least single object fits within slub_max_order. */ for (unsigned int fraction = 16; fraction > 1; fraction /= 2) { - order = calc_slab_order(size, min_objects, slub_max_order, + order = calc_slab_order(size, min_order, slub_max_order, fraction); if (order <= slub_max_order) return order; -- cgit v1.2.3-70-g09d2