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:   Tue, 25 Apr 2023 12:23:44 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     akpm@linux-foundation.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, maple-tree@lists.infradead.org
Subject: Re: [PATCH 6/9] maple_tree: Remove a confusing check
Message-ID: <20230425162344.akgvzb2x5n6zsojh@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>, akpm@linux-foundation.org,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        maple-tree@lists.infradead.org
References: <20230425110511.11680-1-zhangpeng.00@bytedance.com>
 <20230425110511.11680-7-zhangpeng.00@bytedance.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230425110511.11680-7-zhangpeng.00@bytedance.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?69TqQZrBTQYBnPU4WehYZjOPx5b3cf2P7cvCAFZK4vaPWkkmuyIULYFjubuU?=
 =?us-ascii?Q?GLYJYvWr3p2gEjbkLPGHMlfSuezbkfHRazJqNI64h+t6dzJpzevox4ZBOJGy?=
 =?us-ascii?Q?jqFhwGaTRhM7/wpPedhz3nbrAznmTLb7THRS+ZY0Q2L5kFnQbfAuBlmoJAX3?=
 =?us-ascii?Q?eYz77tw2+kENfVrCl2QHc5M6wvl60f3DONR6t6L7ohOQTOJB1z4OYM86rHIi?=
 =?us-ascii?Q?0fb7psnZShgzqyeq+gQw26+kebIPY9xRcUzS7fA1YRrr+F9OmlRcHZFYmjOj?=
 =?us-ascii?Q?BGMDQ2lrUFB9oIz4EMaOnzq49APrc4r9TKZR84v9SqnFTUcRaCognw3tZpyk?=
 =?us-ascii?Q?6bw6Gi5xB3o4/wyIizsNt3Tnj1vVHukiYkYXPodzCEG0zNtbdksfqjwlNLrJ?=
 =?us-ascii?Q?mlhSNHoxZKWglfYHsHJURZ7Znqtt4WeE1HaZdEQYj6CrTftZRAj/pruHaiXl?=
 =?us-ascii?Q?4GGed1eXMudHjQsQQWdsgtqUiDdMGTg5nghI/aqDG0Ad+I5XOR7yHGHFY68T?=
 =?us-ascii?Q?7XcSXu7kKn2tF4ateRPEkcNdfexR4V8kHBEVhF3+G/WIT8Qm17kYAjr2k/oc?=
 =?us-ascii?Q?t0FOqG63gmStiWIXI+p/0bW4uziE4lRikdGJ6AQuUO27BmrDp6SI6N9nNa1f?=
 =?us-ascii?Q?zImLNrisF8LS1q+ec9r34TEDurld1a4hY3eNQZDlmtGPwUeS+fzLY8gBLshR?=
 =?us-ascii?Q?9ab1MGVX21wgJJo7ygEv8CWuUPISx3EcUbN9wjg1aOu9oTLdkhbhdiV9z8dh?=
 =?us-ascii?Q?jSNnUg/l+42gXht5WCbwV7PsdSKQKxeQ/VhJoecRUb63rYItqeuljcxtdX7k?=
 =?us-ascii?Q?HuLiXr1d0MaDsmQYQt04dBQJ/H452InSMDi9/PBgr19g1PdGRknj4lVpc0k1?=
 =?us-ascii?Q?cY/R1y5ocn70ROqxHoR65CA5I29SqwZu/gMHczQdPPRLqdtAVbYrs4O8/hWL?=
 =?us-ascii?Q?Pp42ngNqPtDoGTRN3HBbDId1Y1D/QOcN7RSlGvGTQ6xVwZiK8A54jsOnaanJ?=
 =?us-ascii?Q?E6qczbPlhpo8wJIS7JQXfm4zQjUFJNfpnY/8gB3OHIxFgO9HJ62tG69RXErR?=
 =?us-ascii?Q?YnWxY4xZuitPAf1rR+hE9fgzyGtJvsocG4gf78P20yjUYdE8+o+O3XAaeG+E?=
 =?us-ascii?Q?lmKwJjnfRkoj3ud7mHME2QG0PMnv4HHYY8rg+Ip4VyH6XYWWD+AyUdMFY8bj?=
 =?us-ascii?Q?2oHSwKBuW4/a0okcAzqr0+KOznF9mANySFJgN6GDS3Gc20Dm8hBC3lNwGY1N?=
 =?us-ascii?Q?NELgRtuagaa++B4kOOQpTTSnibxt2BVbMJXJBg8n/W86Aspv/Z65o2J3G4p+?=
 =?us-ascii?Q?Hf3yn7dsTmt5TAoYeLRm9xfKT8zW9vGVcxsZCBg/4G9f5xImGH+qeolk3LeU?=
 =?us-ascii?Q?GpqqHPOxf/6K4lJRv7SV5989hNBncCYcfT07iYkItqUivoV5g82G/ROkoQtb?=
 =?us-ascii?Q?tDfi00n4GliXmhF4yu36826W02ucv5PcDetOoeH0pW4z0BppOJC6VlXJ2/pN?=
 =?us-ascii?Q?oWttWyk51smy08lMjH+jdgNrl7BS63ZKX0iWh80PkhDoKSMDLi/wkGPEkzfK?=
 =?us-ascii?Q?NdOIjku6O1k9xXc3KycmPNxiLiB/WUiCWAGzpqIGqif6NAMUabELqqStMEDV?=
 =?us-ascii?Q?Dw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?7AKCj9LAhIfW5ADDYzwbDpMDSh39lig5VQePIosNQjVGy5qBzgN+epkbfMEM?=
 =?us-ascii?Q?goGH27g5T6dg+zG+3aFouQOXhr0JTISUziW4Am5OJAt8D0M8pQTkYOb8/8Kp?=
 =?us-ascii?Q?9+XeT/EAn/S5TbFN1iApLcg86D0Q9Hz7Z2A05xSkcnrCcXEJxnAznnq/UUFK?=
 =?us-ascii?Q?O88FDrJ9LYDIJ1I8zb3ST/1wiq3i4QXdEzjsyie/OKCearfodLbOn1l4kzXv?=
 =?us-ascii?Q?dbDK33PB5I89QsSk5/vvmFICSR0StxQPFbJXxtuGpFCWQQ23//QCQLDCpti+?=
 =?us-ascii?Q?8Su2pL+S5vBp48KAqNbSLaCYVclUdI7K34rW3z2twM3suygZC1FDvrUFaHFQ?=
 =?us-ascii?Q?vIm90/ek4VOi2SRZ8VcGCDy4lOeGXzj+SQ9mRrLZmLPAorj+qzsC7LZWuK/j?=
 =?us-ascii?Q?mcjuu/DxqAe2ZC6Vl88/tCx/AT38jVJgFtn0QoykzDxh2/GHq+NUWws3+uqb?=
 =?us-ascii?Q?dLynGmj3Ekoi3lUzWFkykdiZU8yBqj9j/TWaOPPLcqucjqwiwl6CW9K05qCL?=
 =?us-ascii?Q?WhMvEB03auOQAjAQWSvJ6gXia2e5U5BvAPY/gjPtFq/LCacRYmW+vojcpA3n?=
 =?us-ascii?Q?S/wSOm0KaXtN3nRj4tzezuPPdjS8zwkmMWWEeDoz9dhdADW8nmn4GviXaI3U?=
 =?us-ascii?Q?AdpDmboyTS1r6JB32WDBNQ1aWDWViwFTEgrORvz7cQxdo5xY0txYvUahVlRn?=
 =?us-ascii?Q?C9u9mw7mLARy9IGYh3SdLFqYcTK8mLEOvpNrK+wK2leDffZS4bWwpsQru0MK?=
 =?us-ascii?Q?e0m0Iw20ZIQuozXoMlpNGCncxRHquMTu4gLBZdvnd7+0gq7RE3Lefr6wlfLN?=
 =?us-ascii?Q?FIIyGMctUOePSHcfgezOVmVkgEBVZIEFSc1m5AamO/8N8GranyOxBg3pYTkU?=
 =?us-ascii?Q?nJjP7nSX9L6YTbtnWpwsReMNWZ7p1Jk/U+1fgsrPfqeJ1iQqHpUD5R2bomo2?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ee188cda-a9e7-465b-d212-08db45a97233
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2023 16:23:47.3383
 (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: dmzPlHSFgP8ONPnvzFwb4ck+RGSHqENkXpKsCzz2w0+N9EN6lT91zCiMKXqggh2BYtLf4c/LiPlf1ApkYt/2/w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6142
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-25_07,2023-04-25_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=632
 adultscore=0 malwarescore=0 mlxscore=0 spamscore=0 bulkscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304250148
X-Proofpoint-GUID: SGOjbf50SyjhMe5SwlwoiALXLUrC_NkH
X-Proofpoint-ORIG-GUID: SGOjbf50SyjhMe5SwlwoiALXLUrC_NkH
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> [230425 07:05]:
> After this loop, we are at the last slot of a node. Our purpose is to
> find an entry that is not NULL, but the pivot is checked here, delete
> it, and change to mas_logical_pivot() to get the pivot. Finally, only
> check whether the entry is NULL.
> 
> Why is this confusing? If the pivot is equal to 0, but if the entry is
> not NULL at this time, it will return NULL because of the pivot, but it
> should not do this, the entry is valid.

Please avoid titles that don't say much at all.. and it's borderline
insulting. I could go into the reasons this was written this way, but
that would be a waste of time.. which is another reason to focus on what
you did in your commit message.

I've removed this function entirely in my patch set, so this isn't
needed.

> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
> ---
>  lib/maple_tree.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 7f4b2ce84ce61..83441ef2e1f57 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4742,17 +4742,13 @@ static inline void *mas_next_nentry(struct ma_state *mas,
>  		return NULL;
>  	}
>  
> -	pivot = mas_safe_pivot(mas, pivots, mas->offset, type);
> +	pivot = mas_logical_pivot(mas, pivots, mas->offset, type);
>  	entry = mas_slot(mas, slots, mas->offset);
>  	if (ma_dead_node(node))
>  		return NULL;
>  
> -	if (!pivot)
> -		return NULL;
> -
>  	if (!entry)
>  		return NULL;
> -
>  found:
>  	mas->last = pivot;
>  	return entry;
> -- 
> 2.20.1
>