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;
Message-ID: <31d24c29-5b7c-328c-b830-276acab43203@oracle.com>
Date:   Wed, 13 Dec 2023 13:03:29 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH V2] maple_tree: do not preallocate nodes for slot stores
Content-Language: en-US
To:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        maple-tree@lists.infradead.org
Cc:     akpm@linux-foundation.org, willy@infradead.org,
        liam.howlett@oracle.com, zhangpeng.00@bytedance.com,
        stable@vger.kernel.org
References: <20231213205058.386589-1-sidhartha.kumar@oracle.com>
From:   Sidhartha Kumar <sidhartha.kumar@oracle.com>
In-Reply-To: <20231213205058.386589-1-sidhartha.kumar@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aUd1Y0JoQ3JoT25sUGcwNGFnV2FkQ2E2S1RmUHhwOEtRRldPQWg0L3hVdkZj?=
 =?utf-8?B?MForb3krOUp1ekh4cFgzdGZsZGVCTlFIb2JQcWx2d2lOaXRpVUd1L1o3ZmFV?=
 =?utf-8?B?YWRuUU5XS3FsQ3FtV2ZxTDhIdFduZFVCcURRaHJON0FYYkVYeW1sY0UrdWJp?=
 =?utf-8?B?RGdXdS9WSytNSHU5dVErZkY2cm9HVzA0VzBPcy9HTi9tTUJTQkRkV1lHV2Nv?=
 =?utf-8?B?S0gvY1A3eUVBbGR4S2xxQTB5Uzl6aG5QWEFyWTl5QnBzaTdkeThseUNjSzBM?=
 =?utf-8?B?bjl6WkErZGxhUFhFeDFNd3BjZUtSV0t1ZkxicDRzeVJ2d2xCdGd5U3BnT08r?=
 =?utf-8?B?dVFOWi9IRUhiRzlheE4rTzFpdzVOVHFwKzQvVGhvM2dRdmZuYWFYQklpVGVD?=
 =?utf-8?B?YmdTM3JyTFVUT2dsUjZVQmkyRDJiRWYvaUdXUncrN3hZeGo1Y1JleUt5WnZ6?=
 =?utf-8?B?WEt6ZWhJL3VrZGZTUEl3dVZ0THJZQ2tDamhtNGZtVE9HMnk2cU1yQkxKVDU4?=
 =?utf-8?B?K21sL0JoUTJLRDFwMlZRUVg3dkN4YVB0Zk1ONGM4bFFBK1VZV1ZZdWxlSjZa?=
 =?utf-8?B?bWszdkUzb0UxQVowN1Jra2FrZnNSSUZMSEZUc09zWWZXVkE2VGcyU3MwYXZR?=
 =?utf-8?B?UVJaZGtuc0x1UXZKanhGcDJYT0RwMTBnUW9UWTRTdU1RTkU1NjU5UEN3VmVx?=
 =?utf-8?B?RGQwZ2pYVGhreE1HQzBUd2ZuVFVnZENBeW9XTmlHTVJhV1BmOTNOR2k1NWlY?=
 =?utf-8?B?RTB5c1JMbGdHQTJ5WkVUWGVYajVldjAwV1pyVUpIRzdyaHlqRWhEWElIRUFF?=
 =?utf-8?B?WFpmN21kdW90eHNHUnFkaEU3enBOWG5nbkpVL0xYd3hKcmdUQ2JvREhUNFFt?=
 =?utf-8?B?dDc2R1N1eXVMSm9RSXA5ekZReTgvK3RmcVF3WkhKZ2E2L3VnODJzN3N0M3cv?=
 =?utf-8?B?Y1pWdDFEQzQvdWh0bkNoQ2VvOUFSMkRZbjJLZ1RnMmplRm9nL29aMlVYQ2Np?=
 =?utf-8?B?aVBySS9kTEx2L2ZwaEJDOVl0TEdIaWgrK1VEeTZpZnZPQ3UxbVpGUmpIclVp?=
 =?utf-8?B?RWI2WFJRY3haUjNKRmozWmZsbi8xU1dlbDV3S2xSOVpBVnpIalRLYWlrNkJw?=
 =?utf-8?B?UUNyaWNUaW5sZkJnMHNGT0VXSXZFVmRaV0J1Q1NSVTRXVXdWcmxFZWxRWXpj?=
 =?utf-8?B?YUNoMGJWNER5WkFaRzdTaGh4eTBwOG4rNmh2aUxkUTU2Z3JRWEVDWUxBWWhR?=
 =?utf-8?B?WitySVV3YVllZHp0Zkl0NUEzUXhKUXZmWlVocEtaNWQ3SXBGMEl6K2lkOHlN?=
 =?utf-8?B?TUltVGlSQ2oxc082VTh2Y1JoSWFBUlg0cnluampnZG5VbWJDazVxZk1MOWI0?=
 =?utf-8?B?bDVXOGdiZ1lGdVYvbjVsd0s4OUttSEF1N25idm9Pb0UyWjhyOWhYT0VWSWZG?=
 =?utf-8?B?OTk4bytKL1NIZVFPUkVkQklPMDZCbk40ZTIwNXd1dG9XelRPQ3lXa2JEdjcw?=
 =?utf-8?B?RE9CalZZWklQMXlSUURBUC83aFQ2dm94bHhpZElEa1Q5Nnh3V3U2SEZFZldl?=
 =?utf-8?B?NFIzOUZvQnhrS0xpbDN1dTN1ZERqU041UTRpTnJxOHFzSmF1U0ZSWUNEUlFG?=
 =?utf-8?B?RElObkZXbVlPNERLQWlxNDNiL2dJOUZNbmNiY3dsS0phV05PNTY3cytmVlhs?=
 =?utf-8?B?ZTh2YmNBM2g5dXZnYmovb3JhL3M0c3pvT2lMZVBmKzZoT1hkQ0F6c0FXOEZK?=
 =?utf-8?B?WXZZbW1rcVJlU1MwZVRMS0t3eFV6TkpUSDF6Q1BUREJpNHpmMnBHdHZrRkZK?=
 =?utf-8?B?OUN4S0hNWUdmZGExQlA3bzJmRmZSaHlYMU56c3NvUzJVQ2dGUFhEZ2FScy9h?=
 =?utf-8?B?dzliSklYWFpvdU15eHhHTUpHc0ZMak13RG5KUk4xRTVxd3Q1OVk4TGtlTFdp?=
 =?utf-8?B?cWVhdTZjVjh3bSsvVnBSN21HZ1g2alBMYWZaa2lVZnJrMGVlSHhmbHQrY0tM?=
 =?utf-8?B?ekNQRFF0MzJYeGtYRHhmcGtuTHhrL3ZrYnR6bDlHdFFnS3FPQUkxKzhNNGc3?=
 =?utf-8?B?bXBvODQ3UW9NWVI1dU1FZEVMYTJCb3dxNVoydXZqOW1MTHVHKzd4ZkRKTnBF?=
 =?utf-8?B?SzBYWE10bkpNUWRHelM0Z1NVbGptcG5WTVBhUi82dFBweHdRSFVwMUVyU09Q?=
 =?utf-8?Q?REPnZ2qk1AbAcvGWwOuR9qg=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?N2hzY2JPUGM2QnZ3VzZIZVNWcUwzNEJHZTdPcWx4Qm5QVThDVUNhQ2dqZDBY?=
 =?utf-8?B?amhML1BZdjl1RmVaSFk4QmQyTkcrSzBKQTNzWGswc29vQ1J2S3BNSnBuZlB5?=
 =?utf-8?B?WVRvc2N2ZjdQSFVkc002NTFsdDVBQ0xVdEg3SldTTTdhcmp4MTRqd0xtNkFJ?=
 =?utf-8?B?dGFhRDF2UytDQnlRckk3RHRwVjFMM3E1eW1KYkg0YWcvT0J6a0d3N3NTNVBW?=
 =?utf-8?B?U0htbWlFWjREYUVtcExjYVAvMnM3Vjhtam8xaThqWnRtTnc5Z3ZmaWh4RGhT?=
 =?utf-8?B?V3h3UWNjemhXbENiMjVLZGpTOEpnSm8rUHp3NVdxYTFKN0VrV2VWUVUrZXlh?=
 =?utf-8?B?N1M2Q0dHSVdwQ0FWWXhSR2w1MC9jY3NZa1V4TVFjaXVRVDJQeEsrL25ScytO?=
 =?utf-8?B?MmdUZjFva1pydFo1dDlEcWVFSEJ6QlVENVprS0FoK0JEWFRtemZCN25MZUhp?=
 =?utf-8?B?UUNkaW02U0tCV0U2ZjBqQmpOd2Q3RVJIRmRWem1XREh6RVRoMHhVUE84d3E0?=
 =?utf-8?B?ZHI5R1g3L2E1L05qWGROTXhOYkpwRHZockdIM0IwakNsck8xSGNOZE91RjVu?=
 =?utf-8?B?MTdXVGFFREhGeTd1SkZKVStrYUNYeUIrNVlZckJCOEJZZDhVazZIQVBmMFZs?=
 =?utf-8?B?RXJ1bkVRNThHbkpQbmdlendub0NGNTJwYW1hQnJPNVZkbTN3a3VWR0phcS8x?=
 =?utf-8?B?b21Dd3JDZnpVQkJCcFNVb1h0dmJmZWgwTHRGVm5zc3dDSU1JNFBXVEtIOVNO?=
 =?utf-8?B?UkM2K1ZJamFyMTAvNWdoQ2VwZTRZUEk1VW1LUXN3N2NiZWU2TUY5VUZwak9u?=
 =?utf-8?B?cDM0LzJjMnVlSHAxSU9jMjNCajlvZ0lCemdMaFptdGlySm9BVVhONlRpSm1V?=
 =?utf-8?B?RmVVVlltWkpCakhNSFpzNHhPNWlSSi92WEdZbVdETDBJcjloZXdkNlFWZHV3?=
 =?utf-8?B?bWVJWmJHVWNMbEEzU1FFcnd2ZmhqaGgwdFBvVEhaV3VTL2ovTGJOVzFWMyt3?=
 =?utf-8?B?T21pSDJ3VTVKeXIvTGVzak1pbjhZL25CYUVFNGRtWEk4OGlRcTlYL1U1M1pa?=
 =?utf-8?B?eFozU0dOS3FzbDVGK3g1eG5FeUYyMm1lOVQ3ZUZNK2cxOUpIekFuc1AwWE5l?=
 =?utf-8?B?bHd4S1hoSDZoRkVkbFhGdWRxZ1IrczlOcU5HL3RDZ2VnQmZSM2NUcWlJYy84?=
 =?utf-8?B?Zlp4OVFDSTBNL0pqNFhpK2JVV3Z5RlRNaXkrbkdzYlFSeEloQkNkaVhhMEVN?=
 =?utf-8?B?VWFBeFNNOTk2Rmg3T0toUGpQN2tLMElycE41bUE4a2tuckNiYVJMemxpc1BM?=
 =?utf-8?Q?iBRPx84uov+R0=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95845e24-9721-4524-f18f-08dbfc1efbcf
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB5113.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 21:03:40.8995
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ZwtHkmutcUWj3nHoHttMFR3NRj8olLafbaPDgLeqxyswwkzEMMR2XpfzYfyysltgQP00z3fIR6huQr0lioeqflewkIPBLvaBTfb6UYBx/EI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR10MB6290
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-13_14,2023-12-13_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 spamscore=0
 mlxscore=0 adultscore=0 phishscore=0 malwarescore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312130149
X-Proofpoint-ORIG-GUID: 0fgnIOtxuJUNfu-1gRI4GZEZAgliJyAX
X-Proofpoint-GUID: 0fgnIOtxuJUNfu-1gRI4GZEZAgliJyAX
X-Spam-Status: No, score=-1.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	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, 13 Dec 2023 13:04:56 -0800 (PST)

On 12/13/23 12:50 PM, Sidhartha Kumar wrote:
> mas_preallocate() defaults to requesting 1 node for preallocation and then
> ,depending on the type of store, will update the request variable. There
> isn't a check for a slot store type, so slot stores are preallocating the
> default 1 node. Slot stores do not require any additional nodes, so add a
> check for the slot store case that will bypass node_count_gfp(). Update
> the tests to reflect that slot stores do not require allocations.
> 
> User visible effects of this bug include increased memory usage from the
> unneeded node that was allocated.
> 
> Fixes: 0b8bb544b1a7 ("maple_tree: update mas_preallocate() testing")
> Cc: <stable@vger.kernel.org> # 6.6+
> Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
> v1->v2:
> 	fix coding style per Matthew and Andrew
> 	use wr_mas->node_end to fix build error
> 

When this is merged to mm-unstable could the following fixlet be applied to be 
compatible with Liam's series[1]:

[1]: 
https://lore.kernel.org/all/20231101171629.3612299-5-Liam.Howlett@oracle.com/T/#mc0e5000f6de822182bf7579c230030c5ec4ec1a7

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 1130803b9d3f7..c9a970ea20dd5 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5477,7 +5477,7 @@ int mas_preallocate(struct ma_state *mas, void *entry, 
gfp_t gfp)
         node_size = mas_wr_new_end(&wr_mas);

         /* Slot store, does not require additional nodes */
-       if (node_size == wr_mas.node_end) {
+       if (node_size == mas->end) {
                 /* reuse node */
                 if (!mt_in_rcu(mas->tree))
                         return 0;

> 
>   lib/maple_tree.c                 | 11 +++++++++++
>   tools/testing/radix-tree/maple.c |  2 +-
>   2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index bb24d84a4922f..684689457d77f 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -5501,6 +5501,17 @@ int mas_preallocate(struct ma_state *mas, void *entry, gfp_t gfp)
>   
>   	mas_wr_end_piv(&wr_mas);
>   	node_size = mas_wr_new_end(&wr_mas);
> +
> +	/* Slot store, does not require additional nodes */
> +	if (node_size == wr_mas.node_end) {
> +		/* reuse node */
> +		if (!mt_in_rcu(mas->tree))
> +			return 0;
> +		/* shifting boundary */
> +		if (wr_mas.offset_end - mas->offset == 1)
> +			return 0;
> +	}
> +
>   	if (node_size >= mt_slots[wr_mas.type]) {
>   		/* Split, worst case for now. */
>   		request = 1 + mas_mt_height(mas) * 2;
> diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c
> index e5da1cad70baf..76a8990bb14e8 100644
> --- a/tools/testing/radix-tree/maple.c
> +++ b/tools/testing/radix-tree/maple.c
> @@ -35538,7 +35538,7 @@ static noinline void __init check_prealloc(struct maple_tree *mt)
>   	MT_BUG_ON(mt, mas_preallocate(&mas, ptr, GFP_KERNEL) != 0);
>   	allocated = mas_allocated(&mas);
>   	height = mas_mt_height(&mas);
> -	MT_BUG_ON(mt, allocated != 1);
> +	MT_BUG_ON(mt, allocated != 0);
>   	mas_store_prealloc(&mas, ptr);
>   	MT_BUG_ON(mt, mas_allocated(&mas) != 0);
>