Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Apr 2023 17:07:08 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        maple-tree@lists.infradead.org,
        Wei Yang <richard.weiyang@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 02/34] maple_tree: Clean up mas_parent_enum()
Message-ID: <20230426210708.r2gpnzocxfulz5y5@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        maple-tree@lists.infradead.org,
        Wei Yang <richard.weiyang@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>
References: <20230425140955.3834476-1-Liam.Howlett@oracle.com>
 <20230425140955.3834476-3-Liam.Howlett@oracle.com>
 <75ebddd7-641f-0274-72c5-415007d60cdd@bytedance.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <75ebddd7-641f-0274-72c5-415007d60cdd@bytedance.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RVNyalhTd3pqL01ObGJTaHNmcE1QZnBFNlZuSHBRRC9uN2dyekpmM3NHYjk2?=
 =?utf-8?B?ek0yS3RGMVZKcEpja0NJS1NOOXlsRjQyMVZCNjQyMlVQQk5jU0tqaXNkUTVX?=
 =?utf-8?B?Q3ExKzhKSFlORmVWbUhzU3daOC9FK1BSWm1hZmhMSVpwek1HUHRiT2p0YTlt?=
 =?utf-8?B?NndRUE5sUTE2NitjNkFwKzJwT1hKQmNyOGZrUnJVNHNEMjlpdmVFckhCQnd0?=
 =?utf-8?B?Z1RxQ1dHN1ZzaXFQcTFzOWVsNENZcVFiRWJ1UGRaaERGT2g2cXB6dmNObldJ?=
 =?utf-8?B?eDVjSWIwTGdvTnVyK3JZTUNOWlN6KzBsaEU2aWIxMlFxbkhzMi9rZnZWLzR6?=
 =?utf-8?B?TGduRmJOM2tmK3ZKSkIwQjQ1aDE5TjRxb2EyY0I1a25MZUtzZlYrZENDTU4r?=
 =?utf-8?B?bkltVFlRd204Qkw3MlFQQkZkSHI1QlVwZ0NaRGxuTmg2ZitYVU5LZGZaS2gy?=
 =?utf-8?B?RWRCc2RXdUFSaURRM2dXVWVrLzRyZzFJWmh0bnlidCsweVlPMlFId0gzTmJp?=
 =?utf-8?B?SWEvcHp4SHBQUVVrZWpKNVFkcTZpOEkvVFp0SmltaU5zUlJmQUNUVUZudmxv?=
 =?utf-8?B?V0plMHZML0hKaGkrY0xtZHhLNndZNE51NnQvcW1OamRmd1QycnRuWmQrZ2NP?=
 =?utf-8?B?TW91dS9vSVBEN0tZb1A3cU5Cc0YzbGlzRUVqM2lDdDUrVmlyMzRsbmZtRS9V?=
 =?utf-8?B?TitMdEZYSzd1RzR1ZHJnYWpNS2lYOVVlbkxFTUNCaUg2eGRSWVRwQWlxV240?=
 =?utf-8?B?MGJEMzR6RTBhS0VmZTJMQnNabGt5TE1qRHNTbWNlSWVuUmx6ditCK21ZMG91?=
 =?utf-8?B?c2VDajB3UUZWbkhQdGRIMFExNWFreDhGQzRRdGJOcmwvdmovY3A0dVZGTThr?=
 =?utf-8?B?cHpaczV0MjBhNUh6Y3BRUEtQNE05MWlmRjA3NDMyWVUzRGNpMi9rcVRlM3h6?=
 =?utf-8?B?VkVtSVhrN29MUVpKaGQzTHV0U3BvUU9PeTVVRDExWkVFeC8zZnlTSWZRS3lr?=
 =?utf-8?B?Qm55eEZWY1ZGaVdndTRldzF5clZ1TDljZlpTOU9tT2lPVXVKc1RES05kRHM3?=
 =?utf-8?B?cHVJOHRMSzRsS0VDVHRFU2ZtcVNrbGZRWng2cHJKZXdtTTRZN1cydTlzQ2or?=
 =?utf-8?B?QXJnUC9xbnNKQlp5YTF4QjFVdTJYUUU5MkpHZlcvNFFHU0RicGdnZmIwU2VU?=
 =?utf-8?B?dnRGaE9DTDU3eXFhY1JIZkRLVkNNZnZMczVhWmNVeFh5bWM1RFlLS01jRy9N?=
 =?utf-8?B?STdLUXNYb2I2K1JRbjJ1UnFMWWJXSzYvbVVQTXZNblJTV1ZwTjZjTUZmeWQz?=
 =?utf-8?B?TmNVejNDTk9KV3E0L2VMbHJzZVJMNTFJNmNZYUh4UmZyYm0vQ0ZZNjFGUEVQ?=
 =?utf-8?B?aTEvMGhxZldGLzlQTVlCUjVBWkVVSkZiV0dwYkZTSVkrOG9BekJKcExzbGR5?=
 =?utf-8?B?S25CSmZnWEV5Q2FTWUtSVjZOTS9nUUg1ZE5CT1RxRDBPOGJZN3ZxYzNnVm9Q?=
 =?utf-8?B?aXA5M0FtZjJCWUdvL2E0S2QvSHhJNDdRczhvMjJYUE56U3VrNVl4NWN2YXo1?=
 =?utf-8?B?UkgyQVFaZHc4UTFaQVNxUlRtOE9nMm05bVZ0YUhUU2ZwbU9nQXRPTmF6T0Zu?=
 =?utf-8?B?dFlvMThzbSt4N09JYWVkbDBXVEVXa0YzV1BCNEVxeTVMM0orVm42S21KZlRz?=
 =?utf-8?B?Zk9pWGROUlJUelJqZE5DbmpSM3RNSFBPZ1BwUFFKS0NnRnVJSFF0NHVRUXRL?=
 =?utf-8?B?TWRZbG1WUFh0M3Y0T2t4OGpobVFteG1VTGNWdzNhZk82S0MvY1Y2UzRBQlc2?=
 =?utf-8?B?SHkxKzJGNkIvcS9PQlZKUko3bUsreGFJekFUa1NYQVdWRCt6elJvb3NCaEhm?=
 =?utf-8?B?WUdxWHVZcUtDUUtBdFRIUzRCcHpUSjZXYlZVOHlSSlRoQUNUa1Q1dVZTWHds?=
 =?utf-8?B?NXpEelhYY3ArMWhBVzJ2M0pEMnVXSnFwaklNMTNqVGdOZFhIZ01LRHF6d0VM?=
 =?utf-8?B?QWNSbVlpTHFMM0RjSDhHUkJzNG40bjVlVXduRCttbkp3VUpCNHZEV2dRcU1y?=
 =?utf-8?B?VmpBcEpRNkNRK1d2alJJdTFGb243WTdNbm9jcTRTZVFDNzdWa1NSMThDVlE2?=
 =?utf-8?B?RXVtOVFwRTRiWlE0UUs3TTlOZXNBRndyMHFZclpxVDhnTjVCcllBd0cyVTJU?=
 =?utf-8?B?aEE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?TG5nK25Mcmd4TERzdStDeFhDVkNtUzM5RVNWMTNib0FZRW5WL1NKNXhIQ2k5?=
 =?utf-8?B?UkZPeVVlMkpCQ0hmVmxtMEdxZmlBb2ZldW9nR01va1RuSWkzdVZMeEplSEhx?=
 =?utf-8?B?Y3pWa09kM0xBUGZyOVRzTlRFamFuZGRrWWpkQ0FOemFKVGxhN1BvUjhTK2cy?=
 =?utf-8?B?MjhaWXBsNjZzTU9kTkNwcDhpY0NZSDRGMGNRcmYwWVZmdmMzSkdFcld5dUJt?=
 =?utf-8?B?MzRvdVlzRExaVVl0dzllZGVadEIyYVd2WFBkakdYZ0RZTThkdGZkdjhaT3BS?=
 =?utf-8?B?dXdVZ1ZYVlBpZTJyM3RSbHJjVjdmeUd3VkZOQUFpQ0pXbCtGbG9VVFg3Ylp0?=
 =?utf-8?B?QjBNcllJS0RDOXZMaFc4UURobGE2UWhvTmIydkkzVUxkRmovY2ErejNUTUVv?=
 =?utf-8?B?ODVjVU81YVZYN0loclM5ZmtCZHZkcjJ6TFVPMC9rdVRDTHYvcU5lOGpRSWVL?=
 =?utf-8?B?MnR0VFhHLy9OMUFvaXJiSUZkWThmOVdMb3V4U3c0ZE9CYWRjam9CYmdickNX?=
 =?utf-8?B?MDRuTTV5aVZqb3h6R3YwUW1rdVJZNXlXRzd4M0x0RGtBYXpsVVBNVDgyRVRW?=
 =?utf-8?B?M2IxaGxqelZ4ZG8wRStrNkk1ZytlR0F0ektBVUo1WmsvNUI1SFUzUDBHUktR?=
 =?utf-8?B?RVltRFkxUXEvUEpGMDRjN2dpVFo0bU5EdzNOY3YvYXRWSEQ0QTF1anVMdkF2?=
 =?utf-8?B?TlVNWTREVWN6dGlUTlU1S2pJQzVERU9mcGovclJING5mbXJHTFA1eExJOWxB?=
 =?utf-8?B?cVphSHNlVDc4RGViUVUwMFVVT093Wmt1VzMrakpINEZ2N0kxdUhHN2l5bTlk?=
 =?utf-8?B?ek1sRy9lNThQNjVjemlpTTYvWUNHRmtDSlBqK3RHRUN2QVpMTG1zanhMQ2VQ?=
 =?utf-8?B?TkIyaG92YnpFRkdFVG1IeU5QbXBEV3hMeVJVci9FTlhvdUQ3WFdDSTMzbXN0?=
 =?utf-8?B?Vm9nZGVkYU9wVk1KYll3d0ZIRjQ5YmlFMHVMQVFBRDBCMnMzYjdxVzlTTTZa?=
 =?utf-8?B?ejFMWWt4aHBYb3ZGYk5VM3BmSEJ4ZXZ1bUtjNjZ3MEs1MWF6YzZsMHpPc2dP?=
 =?utf-8?B?SEJYVGxYczZlNVdDQzFtTUkyQzU2TE9NbDg4b3FtWnJKYXkxRzlKOHlsRlFL?=
 =?utf-8?B?OU0xVzhuV2hNSlllVmdMMXJQUi9vM0Y3dWYvV2dxN1JjcWNGZmJ5QkY3alZF?=
 =?utf-8?B?VGEvMFB6QVdYaFhhaUhMSWdkUUl5R0swZ2VnN0p6Q0lKbDE0RlFTQkZ2WWY1?=
 =?utf-8?B?SkFFY0QwdmNlYlNLWVp1VEZHNkw4Zm85bTB3T0VxU29IeTFkUT09?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 24bf1680-7fa7-4561-c69d-08db469a338e
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2023 21:07:10.8114
 (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: dxWgWB+LZdf0fuI9rIaSeq0JawXHRMqEPllGX5hgReAnHDhcbCj+bLWGHWTG/0shvDhDwG/6KKFPPKoWESVCdQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4510
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-04-26_10,2023-04-26_03,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 malwarescore=0 mlxscore=0 spamscore=0 adultscore=0 phishscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304260183
X-Proofpoint-ORIG-GUID: EYVCYRlzNs5LI-Ro4m6WeTVRMXAkeJ36
X-Proofpoint-GUID: EYVCYRlzNs5LI-Ro4m6WeTVRMXAkeJ36
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Peng Zhang <zhangpeng.00@bytedance.com> [230426 00:15]:
>=20
> =E5=9C=A8 2023/4/25 22:09, Liam R. Howlett =E5=86=99=E9=81=93:
> > From: "Liam R. Howlett" <Liam.Howlett@Oracle.com>
> >=20
> > mas_parent_enum() is a simple wrapper for mte_parent_enum() which is
> > only called from that wrapper.  Remove the wrapper and inline
> > mte_parent_enum() into mas_parent_enum().
> >=20
> > At the same time, clean up the bit masking of the root pointer since it
> > cannot be set by the time the bit masking occurs.  Change the check on
> > the root bit to a WARN_ON(), and fix the verification code to not
> > trigger the WARN_ON() before checking if the node is root.
> >=20
> > Reported-by: Wei Yang <richard.weiyang@gmail.com>
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> > ---
> >   lib/maple_tree.c | 28 +++++++++++-----------------
> >   1 file changed, 11 insertions(+), 17 deletions(-)
> >=20
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 9cf4fca42310c..ac0245dd88dad 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -428,25 +428,23 @@ static inline unsigned long mte_parent_slot_mask(=
unsigned long parent)
> >    * mas_parent_enum() - Return the maple_type of the parent from the s=
tored
> >    * parent type.
> >    * @mas: The maple state
> > - * @node: The maple_enode to extract the parent's enum
> > + * @enode: The maple_enode to extract the parent's enum
> >    * Return: The node->parent maple_type
> >    */
> >   static inline
> > -enum maple_type mte_parent_enum(struct maple_enode *p_enode,
> > -				struct maple_tree *mt)
> > +enum maple_type mas_parent_enum(struct ma_state *mas, struct maple_eno=
de *enode)
>=20
> Do you think it's better to rename this function to mas_parent_type()?
> The meaning of enum is not obvious and there is already a similar
> function mte_node_type().

Yes, thanks. I'll make that change in v2.

>=20
> >   {
> >   	unsigned long p_type;
> > -	p_type =3D (unsigned long)p_enode;
> > -	if (p_type & MAPLE_PARENT_ROOT)
> > -		return 0; /* Validated in the caller. */
> > +	p_type =3D (unsigned long)mte_to_node(enode)->parent;
> > +	if (WARN_ON(p_type & MAPLE_PARENT_ROOT))
> > +		return 0;
> >   	p_type &=3D MAPLE_NODE_MASK;
> > -	p_type =3D p_type & ~(MAPLE_PARENT_ROOT | mte_parent_slot_mask(p_type=
));
> > -
> > +	p_type &=3D ~mte_parent_slot_mask(p_type);
> >   	switch (p_type) {
> >   	case MAPLE_PARENT_RANGE64: /* or MAPLE_PARENT_ARANGE64 */
> > -		if (mt_is_alloc(mt))
> > +		if (mt_is_alloc(mas->tree))
> >   			return maple_arange_64;
> >   		return maple_range_64;
> >   	}
> > @@ -454,12 +452,6 @@ enum maple_type mte_parent_enum(struct maple_enode=
 *p_enode,
> >   	return 0;
> >   }
> > -static inline
> > -enum maple_type mas_parent_enum(struct ma_state *mas, struct maple_eno=
de *enode)
> > -{
> > -	return mte_parent_enum(ma_enode_ptr(mte_to_node(enode)->parent), mas-=
>tree);
> > -}
> > -
> >   /*
> >    * mte_set_parent() - Set the parent node and encode the slot
> >    * @enode: The encoded maple node.
> > @@ -7008,14 +7000,16 @@ static void mas_validate_parent_slot(struct ma_=
state *mas)
> >   {
> >   	struct maple_node *parent;
> >   	struct maple_enode *node;
> > -	enum maple_type p_type =3D mas_parent_enum(mas, mas->node);
> > -	unsigned char p_slot =3D mte_parent_slot(mas->node);
> > +	enum maple_type p_type;
> > +	unsigned char p_slot;
> >   	void __rcu **slots;
> >   	int i;
> >   	if (mte_is_root(mas->node))
> >   		return;
> > +	p_slot =3D mte_parent_slot(mas->node);
> > +	p_type =3D mas_parent_enum(mas, mas->node);
> >   	parent =3D mte_parent(mas->node);
> >   	slots =3D ma_slots(parent, p_type);
> >   	MT_BUG_ON(mas->tree, mas_mn(mas) =3D=3D parent);