Date:   Mon, 23 Jan 2023 17:35:21 -0500
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Wei Yang <richard.weiyang@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] maple_tree: should get pivots boundary by type
Message-ID: <20230123223521.4eacmsmftmwddjfb@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Wei Yang <richard.weiyang@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
References: <20221112234308.23823-1-richard.weiyang@gmail.com>
 <20230123134341.2514c269a07ad9e7989921c6@linux-foundation.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230123134341.2514c269a07ad9e7989921c6@linux-foundation.org>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?vlLLZwcHn0MRvyM/gpF+yUSvg4+2ewx2enYwypmT/u0UxHqXDg4TVFY2jnUs?=
 =?us-ascii?Q?kUlCdbBaozU113Szw1xoZaYQnLrbrYmne2BCixNJZCYMGf7+05wOD7a5pXV3?=
 =?us-ascii?Q?orUlzvALHr5qlSc+QWe5hP6GJM931TgvypHGjwtgKstiIOZsHKYW0Xn85Q1Z?=
 =?us-ascii?Q?4NwHEzgWvZO5MsVDMY/rcRmqTyYkm24mN06YYvtk5DWUqCn7FBDjzpLh06Ya?=
 =?us-ascii?Q?l5zouX2oGHmknVB/a4zqxCrvRYjjekE1EYuI+PgTvsBK+zy7lh+f7tS/uAsH?=
 =?us-ascii?Q?9PdaN0HqFJknm6bCq1tdeR0omOnqWPC4sPWSDngEvBuKdtto9liX7X8xczuF?=
 =?us-ascii?Q?cqgJQYSOGrInC7w5VarVr8l6sejvTr2rkeDVkI78tKRY6cc0wbrQvzgbqCoR?=
 =?us-ascii?Q?3j+i3Yd5zzi9ttKJ6JXiTyF8ZC5jHjV8O+PqA8UlbXwmYfNj9OAmzeQpTbPn?=
 =?us-ascii?Q?xLya3jC9O1wXgJCo8F51AX8R0JYaj/ebND1XqN9mU4Z8s/JchBWHknxAdIvG?=
 =?us-ascii?Q?MT/kuNXi4sg0tUvBOyG2JdxpsZYFS5fPrfHgtNCeEq81+sN6poNqU9HdIoJi?=
 =?us-ascii?Q?dXe3l9J2e2/ydaK+2P1rInlrAjllwEmHZs2BDjOit1ZnmGadYFgDn2aZcRCu?=
 =?us-ascii?Q?B6CEglX9eOQxQ16Xi8HMsvFNHSeI+JwVbH8pgwGdIG8k57eGoJqXEvimKAl5?=
 =?us-ascii?Q?NXaQmreVwQEGu5BHeYDWuLe1dce7bnsaOlPL/24OQEtNPAqDRpZglVYnZBgA?=
 =?us-ascii?Q?SpoxcqjL5YeMFAbctG9FgoU7whfwbYNqkER9poO5C6c0iHeFrALlsZuIIs3V?=
 =?us-ascii?Q?T07PwuaQYGYleuVsCkbtDcoCqusysyfOE/F+U/zmmwptURj1KFEOAijI1veK?=
 =?us-ascii?Q?q+aowM3yOPuSau+MoKtEx2PHUpwP9W3Xwtcthks2rpjk9sxP3H9voPlR8Glo?=
 =?us-ascii?Q?Xk9tK5XFJhKRxoMFXGNDYDC+g6nSJAon7X/fef3A+5yK62YFviTpVRHLxIOP?=
 =?us-ascii?Q?0L3tI1lnxZDFsEJc3vYO9RECZNs6bjfwoAKMB40v+/2OexHBBBkN0J+j11SJ?=
 =?us-ascii?Q?aY/wkox+3UQjpq5qqAy18NzmiEmTcaZ5n5YKLxxLH2YCoVIrXsk36yVt5VTq?=
 =?us-ascii?Q?OFSyYLHZtd7HlFmdLUi5maemkMi+/TrGmfKw/kNul0uB2ZEUsrxtUwi1jtBm?=
 =?us-ascii?Q?UwTSk+fDQY167yo+a2ALlvJXovx9InUaJ+Qkh7oTgiDsFt2uQt18/fHHQHI0?=
 =?us-ascii?Q?5ew5UxFjmZVLy/Z+7EemF+J2nGizaTDuHwaMUgVgIxWyNSu1HZbtKnY3uCN2?=
 =?us-ascii?Q?jNfDbQlhVakOg0WU4S27OIwos6eKRokgez3FbpVZO3cM1Tr/oTen3WX7FILO?=
 =?us-ascii?Q?dihVWekYfaWZKkrBbPIcNfxlIGeh5kTX35uBJruGKNh6JF7RkXP2JQVJzWDf?=
 =?us-ascii?Q?RhEzabvxY53OPZX6u3uioX/4NOTmT8WWWGv1FzN29cnhJTcKUx7ntZmlz+Dn?=
 =?us-ascii?Q?4FKjKTNL9WZ1SWkkt7l5hOTSHcD7DctABtv47r2QcOrJQs1ekiXx2m/p/ZXo?=
 =?us-ascii?Q?1dBV9A4bzGR0pNdD5DmPOD6IwometlHBqgwzOPDAyCmQhjYrRvzsfOrYYE5v?=
 =?us-ascii?Q?wA=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: YIbAR8xZmZMl/kWMC7MqYa/v10hkVEd4vAnTwSRPkjizB0JlqBZ2T0lJleZf33jD+gLlF76i+Eerujgekl00xiqqZ8Ukb58i4GkFx9OO+yra5g71okycm5H0OFHSjO9xPMY2r4mW5VtnGEN4qxuQylD+G8VDk+mnhiYO7dL+CAzieMxdNkLF3ez1dWm/U/eSVrt5W8XKzVJNXO/ymuuxoNZNFsIftxDG4vbwj/NhU4Nn8NO8EGG4VOUIo/rn1boUgBWwUoPaECxqMl2SAqTCUzPQqe7+pj+aLbnEb6SI9TjxD70ZlAft3xgwIzzNKyHVTtSE90xnLWAVFctAmOy3zCdeLOakHgaW+ZlV+xaBZjHvQCUgqZT62XQupqPsoK1U8JA8makKEqneacy+IrxCRL1Soief4RjrgJAa/K9+H9B5/HM/R4iR0NP7/9M0C7BkjFE3z+YwIAc6ptzej4Gjxk6KavvqWujWI1AgR9NL4QDlCSf2WYvYMboca+m/pZx/7QYsDu76NWXu9QBVzOX//yJEHn/ZF8dgjleVZzjdr5NY7e/yzCoq7kWSdTZZitvZCvqTePuwLqVQ5TW/1Zaz62lYowFN1ynfPGh0mp0iCpn+4DowGlxvdDVKJhcG4MkEiDU1yJSh/knoBRsYCuRYRVfKTncBK6TNk4WUs6ODmjbbv1Q5zdMPyj3JXd9QlzaqOQ5m4ZL93WuSCzxPNlPnbvUCfDABLqGX5FQTl3to2AWSqXTsO4woivHENlsEgrSWXEezPg8DbqkHeJxjNuzn26qJp86X5JArrTvNt1R86Rqt4MJJ72SB1JMXnDyKkEro3lFgkKZL+ThQNA2apPTBvpWdmpzL1yMNxzPIjjw4JIyrDz1iNODiF7OGWnTAhy32
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 79d99cfd-275e-49b4-a2df-08dafd921e11
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2023 22:35:24.0058
 (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: nAkQqxfKH5fDPFZO7Qhw7C/Rv7SEB0KhN4KdY3lnwSZe+yx8wml6YIT1cdiO+685IuQS2bViCeoh+hLQkV2hlw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB7055
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-23_12,2023-01-23_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0
 bulkscore=0 mlxscore=0 mlxlogscore=832 adultscore=0 spamscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301230214
X-Proofpoint-GUID: ZvG7x89ChuQckAFL7dAAmsnUn7QABA5-
X-Proofpoint-ORIG-GUID: ZvG7x89ChuQckAFL7dAAmsnUn7QABA5-
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Andrew Morton <akpm@linux-foundation.org> [230123 16:43]:
> On Sat, 12 Nov 2022 23:43:08 +0000 Wei Yang <richard.weiyang@gmail.com> wrote:
> 
> > We should get pivots boundary by type.
> > 
> 
> When fixing a bug, please always fully describe the runtime effects of
> that bug.
> 
> I see from earlier review that the bug could result in overindexing
> mt_pivots[], but Liam says this code isn't presently used, but
> mas_alloc() calls mte_pivot(), so I'm all confused.

Correct.  The function mas_alloc() is... very much not well named.  It
is called from mtree_alloc_range() which will put a temporary value into
the tree so that you may store over that value later without allocating
memory later.  It's about allocating a range, not memory. Perhaps
mas_reserve() would have been better, in retrospect.

Anyways, it's not used by the mm code right now, but it's certainly a
bug.

> 
> 
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -669,12 +669,13 @@ static inline unsigned long mte_pivot(const struct maple_enode *mn,
> >  				 unsigned char piv)
> >  {
> >  	struct maple_node *node = mte_to_node(mn);
> > +	enum maple_type type = mte_node_type(mn);
> >  
> > -	if (piv >= mt_pivots[piv]) {
> > +	if (piv >= mt_pivots[type]) {
> >  		WARN_ON(1);
> >  		return 0;
> >  	}
> > -	switch (mte_node_type(mn)) {
> > +	switch (type) {
> >  	case maple_arange_64:
> >  		return node->ma64.pivot[piv];
> >  	case maple_range_64:
> > -- 
> > 2.33.1
> >