Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 20 Sep 2023 21:36:11 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Vlastimil Babka <vbabka@suse.cz>
CC:     David Rientjes <rientjes@google.com>,
        Christoph Lameter <cl@linux.com>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Jay Patel <jaypatel@linux.ibm.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Pekka Enberg <penberg@kernel.org>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "patches@lists.linux.dev" <patches@lists.linux.dev>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/4] mm/slub: refactor calculate_order() and
 calc_slab_order()
Message-ID: <ZQr1SwNeNA+nTpzW@feng-clx>
References: <20230908145302.30320-6-vbabka@suse.cz>
 <20230908145302.30320-10-vbabka@suse.cz>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230908145302.30320-10-vbabka@suse.cz>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Rv6o3D3TszUBPfX3ofM6jbDIT7VEUdW/En6xsYrSAbYrvn0kYTC9UhXxpNj/?=
 =?us-ascii?Q?u6d4GFnxqYyGJaM3XTy9rt1proHa4c816GUubVTYg2F34O3/IZ4kixNCKWFz?=
 =?us-ascii?Q?W19BvU3PMF45xHmIv+nUw82xaL81r02Nvy78gOqc3jS/M2zWoGHPvqlmiLdd?=
 =?us-ascii?Q?KwLfl68r/BjNDjnY/lgCDdHmsLuzRP/F1E0y436pTtvSzNSodEEyU5floW8y?=
 =?us-ascii?Q?e9fKsRh6cefnQmPPL9nKf9Esi5Mj1illmBlotutL7J9IE+AjfaOFXYJh0LBm?=
 =?us-ascii?Q?2dDBl1ebkCulochodK8wx+gOaJ40gjd25oQxVwSGMU0x/00NEmwASXc/sfYw?=
 =?us-ascii?Q?cXjH0T79pXZ79m9Hdux+e5Mvn3QKZyBE5RQD5STD/n2FKZZ0dOGBBu+HufTC?=
 =?us-ascii?Q?iLrMaRTBJNqPGYQNk48jKeUC0FdR0TtbAU+2TnXBnciD+mpBG7jeacKTNaoy?=
 =?us-ascii?Q?0gUC0vHKE01uS7R/ENhWGPp0Skzb9SFdiqSKMh3NmV3l7gY9p7d01CrZogxh?=
 =?us-ascii?Q?ispnS9cFd5L5itQL91vcwRSk7NjrYtb5KQR7r01pRcNLEu6/WUPLlgLd+CkD?=
 =?us-ascii?Q?O0cdGYE+SQupDosBVuYxHNE1bcNKFJX8X+Dzd6HEAzduJyzXPk5WmG6vzbW6?=
 =?us-ascii?Q?jiGMZM6bIYjsk+f0QxSZVobvSOplaCjhUWJo7CD2OWoaQPhcVM1WlAxesp0D?=
 =?us-ascii?Q?LpIC1kndoM8CrcYsmebnMk17Bv00Y6pHfhXdiW9RnJfBDzh+hhjomsRJKUPK?=
 =?us-ascii?Q?QcYqCOQ7WuPrk8NGFAyu6Gwlp9pHHU0+9mnB0dC1hFd2KP8xiOy39jILcpFh?=
 =?us-ascii?Q?NAN4a97TB6h1ZIuGxH2S7w6Kg+amnE2O5CelGWUOA+7muH6mj17mJwry4xOg?=
 =?us-ascii?Q?7ptcn4a78UrBLYcgM2ZZFzQ3ImBvrpOVWLxvGCCV+uVBKIz/Clayq+GjVDZO?=
 =?us-ascii?Q?O9yqcyW+AqRB83r3CAijak29cSYi4xw/MJc+KVgoh60pakV06MBNgWvL0SLk?=
 =?us-ascii?Q?k5pCYWULDLhkkiFMuIhGfWnZhH9oYMubAlsC53RrChnMypl6nCr89DgrbUnc?=
 =?us-ascii?Q?0a0MbiJ+qyMQiQJ5ZAKJvex7bpoSOUD0UqesKYCL2Boybv/i6j6Vf06ZlyyJ?=
 =?us-ascii?Q?Bja3Nt/AMxYiA/yj0gerUvSNkNFAFIPbnwawV6V27poSzOxUidOdeMCSl1o6?=
 =?us-ascii?Q?1ybmK69RCb4UNZO71wp0vwmveNf5dHKegmOBMs29sKekxQ38uZQ6eEcffQpA?=
 =?us-ascii?Q?I/TEkYi/JQvg6tSXRtStqY0PDsJnjKb2sg9mY3YzQMt0t8yadBhLT10ennkA?=
 =?us-ascii?Q?W1J1i+jzgT321k2LO1wZ+NT+R3UbflG1eBXIllaR5yDWrSdSZL1xxWvV1+Vz?=
 =?us-ascii?Q?DbsPldJYVR1yVEAG/IvDEoVGpxvnIq2OnUoG1tTbP0MNB9JqDMer97zl3OqJ?=
 =?us-ascii?Q?F2RjwyChjy+uqPHs4aHdfVTs5k4IJgLY8E2v9h7dpyl71OMfFpR+R6OMZHd0?=
 =?us-ascii?Q?reGIMmx4tj6kw05cp6OtOc3qSL91ZorQHnv57zBrIgEUDErIfeb4BSqUtpk5?=
 =?us-ascii?Q?qBYMl0KUo4V8v3mYRiKeCyskwEa4va74y9S2Ia2v?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 872d16f0-32aa-4fa3-80c9-08dbb9dfe1d2
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Sep 2023 13:45:42.2397
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: I4m/eli1qwYa+5bhn3sHGZ4hF/PPZ22YbG0l9Et1byeplBhp5Yzezntb3cxQMZRu+e6OsrNEYcXuy/qnxSrB1w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB8041
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 20 Sep 2023 06:46:04 -0700 (PDT)

On Fri, Sep 08, 2023 at 10:53:07PM +0800, Vlastimil Babka wrote:
> 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
>   calc_slab_order().

Nit: here is to 'move ... to calculate_order()'?

I tried this patch series with normal boot on a desktop and one 2
socket server: patch 2/4 doesn't change order of any slab, and patch
3/4 does make the slab order of big objects more consistent.

Thanks for making the code much cleaner! And for the whole series, 

Reviewed-by: Feng Tang <feng.tang@intel.com>

> - 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 <vbabka@suse.cz>
> ---
>  mm/slub.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index f04eb029d85a..1c91f72c7239 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,7 +4147,7 @@ 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);
> @@ -4160,6 +4155,10 @@ static inline int calculate_order(unsigned int size)
>  	max_objects = order_objects(slub_max_order, size);
>  	min_objects = min(min_objects, max_objects);
>  
> +	min_order = max(slub_min_order, (unsigned int)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 and
> @@ -4176,7 +4175,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;
> -- 
> 2.42.0
> 
>