Received-SPF: pass (google.com: domain of linux-kernel+bounces-213445-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 13 Jun 2024 10:40:28 -0400
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc: linux-kernel@vger.kernel.org, maple-tree@lists.infradead.org,
        linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org
Subject: Re: [PATCH v2 10/16] maple_tree: convert mas_insert() to preallocate
 nodes
Message-ID: <2xqcyocypx4bpwbovxjxplc76dtns5t4qk4cc3yrejjtuzzdh3@periymnnezyg>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@oracle.com>, 
	Sidhartha Kumar <sidhartha.kumar@oracle.com>, linux-kernel@vger.kernel.org, maple-tree@lists.infradead.org, 
	linux-mm@kvack.org, akpm@linux-foundation.org, willy@infradead.org
References: <20240607185257.963768-1-sidhartha.kumar@oracle.com>
 <20240607185257.963768-11-sidhartha.kumar@oracle.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240607185257.963768-11-sidhartha.kumar@oracle.com>
User-Agent: NeoMutt/20231103
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?QL0Sa25d39/DpHMbV9ajVE05w63ZbXeJlKp6lrXo8h0uaCBWLOhkv+baxBS6?=
 =?us-ascii?Q?7JnaVQRt54fdz/TczHj0iPmupMadckD8WQ8p6FSD0UmC/6lYRPeJVEIb1EzW?=
 =?us-ascii?Q?57IrPNjBw5seP/T0+sSHt1eILI9C5tPdpG1GzUDXMU7MxLriyhbmTmdv/hqr?=
 =?us-ascii?Q?29toCB6uYWudUFWQ65j0mMewksMM6p58/rwxNyvA+jUawZ/cuD4HiznqjbF2?=
 =?us-ascii?Q?9crbn9aF+rCyBcAxuq8eY4Rhs3Y1hjjhARLrzUUVhjaR7ZjKqdWWCecLh0vx?=
 =?us-ascii?Q?JfrmDLM62wjhgV5k/KMjN/GtSyxxXb3AkJdLdnbqE4yhTntW8cSccjPG03Rj?=
 =?us-ascii?Q?bXHYeI2ppjPoyJOAd1jGTFeLSjjmKRgrb+h+AqBcMdLDR0FVWlvJ4VSYDp0m?=
 =?us-ascii?Q?k/RSsZP7skIIjU+bCcunRyOE06iKv//bJqjk3J2Kvih9bcJYeMb2RERU858s?=
 =?us-ascii?Q?SFn3rckXfbbKyHb5PrQiaGMQ+EDzsyS0+1/8PECz0diHepE51lS2cZy7ZG98?=
 =?us-ascii?Q?VcxLigknlzjpckJ1IZrKEGR6JDufgeZTCKHWk6zMLJbFZ6QqptACqua6ei9K?=
 =?us-ascii?Q?RHonJtdZJr5MlM26S2dWeqAwAAGAivoafdUQ/yVvzq70b/Oe3SYoctc5Nv+h?=
 =?us-ascii?Q?Up3zdZjxJPR8p8/ldKflBx8FT1LOS59BdRqTBIoZ1ey5SqObNO+RVuqFASGy?=
 =?us-ascii?Q?EYtWEP3nHd+MQeBUvq8Qvqo4htjKCI+uZ5QecXytDEzwYsAt+3HQP6TlnWYW?=
 =?us-ascii?Q?dsZ2kR7vp0LwusI8IkgJIy6c5t+YRTHPG1GCztrMlFgvzDjxJ4ebW4Z/K9CC?=
 =?us-ascii?Q?VrMFMwsrYOUKg4qLoqEz7Sy6s/3ZjAKtxlSKLHBmmCiFzXXMKLBRh9lpvno5?=
 =?us-ascii?Q?QH6JYA++3WTpXVif+cqzRFey0DBEQYZFm5W422j4aZ0RTfFSksVBIKkD9neC?=
 =?us-ascii?Q?mBahYGFnP5n/40osryQS1B96cpkBn0lLzhrs4TACXWOApExRQ3Y2u4laIz+f?=
 =?us-ascii?Q?p6qiTOFA+xdmJkjy0K2CTvWHWKUN/dAmuFf4m8kGl+zGHQgvDIpdHK7PLTx7?=
 =?us-ascii?Q?kIwORPV2CrOuq2dVssH0aEWoGOnOSGFXXpEactHrR6lM4SDLLYo6Crqi1urZ?=
 =?us-ascii?Q?ucfnPDSuDFeinIm8VHCeOhQ31QX+V4KreEwnRkHND6cKlB0EAKMC+50+8L5k?=
 =?us-ascii?Q?Z8iMsnfbNiMEoBJBlpryGemTisZiGC8GnPP+AJeo1Up/UF7yr5RczJUuocKg?=
 =?us-ascii?Q?Kde2RLcdIc0u7RY7eaTQBZy0wWnVnFjcM3E24NaMXAhpTuEBucp8IiKgL9rq?=
 =?us-ascii?Q?QlUUhHYz832Ja8GyKX2SAsAE3BJxiQ8qGla/IBC4M3GI7LK8b4jDUaiOOskt?=
 =?us-ascii?Q?kBb+8YQRqiTiL/WN97wsOUtWUqScU25TZmDg3HKOWHTO2oUQIeXsSP7RcV8z?=
 =?us-ascii?Q?BZvKq3OVGkS2Obwa6QmjUJIJsvau1DozLt/nP90o7GeWqjFimUesgNGbR12b?=
 =?us-ascii?Q?h0F434+LsI3FQMFG3iTv1d8kPbVhZIU/bLx5mCakWu13BT45MwdNWgCYkCKO?=
 =?us-ascii?Q?WIOPmkWlPRCLMhNgMJHBQAfJp8fsr6GGM96xnZ1hGr3nn9hOPprE0KX5Wu1X?=
 =?us-ascii?Q?5w=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	BE4py/VO97N4wFXoPL9Pn7/5AuLXepd/SYPGwl/CRGMsuuNDv2hkibR4XrVQdRg04k96XvFeI4VUnqTWi3XSjPNzmt5buGkIcMwlZ3uKbL2hy7Eo9+ZsSpsK02BCFHCkmVEowPGm1v5m7u/XL27ufJ/vTpX+ChZE3mvr34Gn+omDW9esNnevwQjg9Smr7Oat7BDrp0/IQ/qSrcgRsUIodVEvT3SLLKbkm+hPwCO28dg/7zqXmAKy+wSE+dhlxsq1CxNOvMAGFXQgS616c4ODeY/MURr4hzL/TrNR0ZK9FtobVUp+mNSopTsaAgCXx9MygV2x3zcrEHCo80ZGPeMI7Rs/gCJu+iptJ7dO0pyIZP9lnQbBtCLG/L00xLqb7KVMPqYpqnT07PT4cGpN5wtwJds8utkFBauEEGYa8sO+E0tu3ArpRaO3vn/NjjCqJ8FPjv1GBjBwWcwbwRHiH4wTfYxbmTBJJ3hZcLzVl8CEKFnWTgVDvx0ZhKUV1U7qkWDOt9cKrj7Yw/9ZOYlHe5F6DRcEeorPCikhsA0lMqXREw6lqNAgOrtD+AFk/I9Scj7xnZ2OIeWpQiiA6Q+tokhnxhAPgIwFMPmvEHe9OdKnhEY=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 22402690-fd5e-4b4b-4bb3-08dc8bb6c656
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jun 2024 14:40:30.8904
 (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: 2rUthGr4zqbDR2ts1hkDrV1spQ4Z4ucgATDaO5GLNbG8mI7gw6t6agf6yOiLrKZ+tZZNXKitphG0hWhfoIrLgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA3PR10MB7994
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-13_08,2024-06-13_02,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 adultscore=0
 phishscore=0 suspectscore=0 malwarescore=0 mlxscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2405010000 definitions=main-2406130107
X-Proofpoint-ORIG-GUID: QHXH_HZaFS_fptrxKEcjz-bIn0mHUd8E
X-Proofpoint-GUID: QHXH_HZaFS_fptrxKEcjz-bIn0mHUd8E

* Sidhartha Kumar <sidhartha.kumar@oracle.com> [240607 14:53]:
> By setting the store type in mas_insert(), we no longer need to use
> mas_wr_modify() to determine the correct store function to use. Instead,
> set the store type and call mas_wr_store_entry(). Also, pass in the
> requested gfp flags to mas_insert() so they can be passed to the call to
> mas_wr_preallocate().

This can be done without passing in the gfp flags as it was done before
by using the same flags as before (GFP_KERNEL).  However the only user
already has the gfp flags to use, so this makes sense.

We may need to add a _gfp() version in the future, but most users want
to mas_store() vs mas_insert(), so I'm fine with this change.

> 
> Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
>  lib/maple_tree.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 2c42e99c400c..c37bfac4f622 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4442,11 +4442,12 @@ static inline void mas_wr_preallocate(struct ma_wr_state *wr_mas, void *entry, g
>   * mas_insert() - Internal call to insert a value
>   * @mas: The maple state
>   * @entry: The entry to store
> + * @gfp: The GFP_FLAGS to use for allocations
>   *
>   * Return: %NULL or the contents that already exists at the requested index
>   * otherwise.  The maple state needs to be checked for error conditions.
>   */
> -static inline void *mas_insert(struct ma_state *mas, void *entry)
> +static inline void *mas_insert(struct ma_state *mas, void *entry, gfp_t gfp)
>  {
>  	MA_WR_STATE(wr_mas, mas, entry);
>  
> @@ -4468,26 +4469,24 @@ static inline void *mas_insert(struct ma_state *mas, void *entry)
>  	if (wr_mas.content)
>  		goto exists;
>  
> -	if (mas_is_none(mas) || mas_is_ptr(mas)) {
> -		mas_store_root(mas, entry);
> +	mas_wr_preallocate(&wr_mas, entry, gfp);
> +	if (mas_is_err(mas))
>  		return NULL;
> -	}
>  
>  	/* spanning writes always overwrite something */
> -	if (!mas_wr_walk(&wr_mas))
> +	if (mas->store_type == wr_spanning_store)
>  		goto exists;
>  
>  	/* At this point, we are at the leaf node that needs to be altered. */
> -	wr_mas.offset_end = mas->offset;
> -	wr_mas.end_piv = wr_mas.r_max;
> -
> -	if (wr_mas.content || (mas->last > wr_mas.r_max))
> -		goto exists;
> +	if (mas->store_type != wr_new_root && mas->store_type != wr_store_root) {
> +		wr_mas.offset_end = mas->offset;
> +		wr_mas.end_piv = wr_mas.r_max;
>  
> -	if (!entry)
> -		return NULL;
> +		if (wr_mas.content || (mas->last > wr_mas.r_max))
> +			goto exists;
> +	}
>  
> -	mas_wr_modify(&wr_mas);
> +	mas_wr_store_entry(&wr_mas);
>  	return wr_mas.content;
>  
>  exists:
> @@ -4532,7 +4531,7 @@ int mas_alloc_cyclic(struct ma_state *mas, unsigned long *startp,
>  		return ret;
>  
>  	do {
> -		mas_insert(mas, entry);
> +		mas_insert(mas, entry, gfp);
>  	} while (mas_nomem(mas, gfp));
>  	if (mas_is_err(mas))
>  		return xa_err(mas->node);
> @@ -6536,7 +6535,7 @@ int mtree_insert_range(struct maple_tree *mt, unsigned long first,
>  
>  	mtree_lock(mt);
>  retry:
> -	mas_insert(&ms, entry);
> +	mas_insert(&ms, entry, gfp);
>  	if (mas_nomem(&ms, gfp))
>  		goto retry;
>  
> @@ -6585,7 +6584,7 @@ int mtree_alloc_range(struct maple_tree *mt, unsigned long *startp,
>  	if (ret)
>  		goto unlock;
>  
> -	mas_insert(&mas, entry);
> +	mas_insert(&mas, entry, gfp);
>  	/*
>  	 * mas_nomem() may release the lock, causing the allocated area
>  	 * to be unavailable, so try to allocate a free area again.
> @@ -6667,7 +6666,7 @@ int mtree_alloc_rrange(struct maple_tree *mt, unsigned long *startp,
>  	if (ret)
>  		goto unlock;
>  
> -	mas_insert(&mas, entry);
> +	mas_insert(&mas, entry, gfp);
>  	/*
>  	 * mas_nomem() may release the lock, causing the allocated area
>  	 * to be unavailable, so try to allocate a free area again.
> -- 
> 2.45.2
>