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:   Mon, 15 May 2023 13:29:19 -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 08/10] maple_tree: Add comments and some minor cleanups
 to mas_wr_append()
Message-ID: <20230515172919.47rkp77cbtwy7pvn@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: <20230515131757.60035-1-zhangpeng.00@bytedance.com>
 <20230515131757.60035-9-zhangpeng.00@bytedance.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230515131757.60035-9-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?UM9NuCCa+PqGIYScmLd7Z1zXxkZ/BHn28FW/Tjt5cPEGJfUeaTrzULPtPbEH?=
 =?us-ascii?Q?dylzdsTF10mPBkJECf9LKaiczDYpf3tbpt4lt1Tqn8n7mgiHHmTBQSKY2loI?=
 =?us-ascii?Q?CKE+ipNrg9j9Zvg/FH5OwDrWMNGavZik0RTSlemdPDTsG8GzObqpu0OE92Ia?=
 =?us-ascii?Q?WMzBZ7TIu6cOsrdX9EBfle92ZCrsD8JWYfG2INNrifqk90wON8MRKqEE2P2E?=
 =?us-ascii?Q?JZsFke4uKEAlIhzBkW/p7CPMD2nirC1C4pTwXbDikgaJJ9WXMZCo8rPkMWbq?=
 =?us-ascii?Q?nRNd0TVyr3n5AbudbRnXqdiTePDDA9mGPxQltQHgBBI0i7DMkWkAsNPra3io?=
 =?us-ascii?Q?gkpqI6m1ZjzBKE1Fi8oxQbuLJqyW7hRef9S52tMJz/9Q3GeysovOkssi0nqJ?=
 =?us-ascii?Q?Tls8gqn1jkiKryiSKWAC/U3/1vCI7rETC4Zlh8trsi82MApJq+kEDmmurrwS?=
 =?us-ascii?Q?zBdAm5jY9wiZ3GmQV24MDOHD9lB2xFZkTE6BBiV4u3kurDo32C/9Um72LMSz?=
 =?us-ascii?Q?fYHucDhu5+SRj1kK6QhSdONdMMMxw5Ee0KY+GTorL9dDuLSegrImV4OiOfS0?=
 =?us-ascii?Q?pxAlbuIwz0P/oNjTwQJQ/EyisqkzP3BJF5MvqvABUJYCjcZAbWSeP2p1og6e?=
 =?us-ascii?Q?BZwY6sT8IRin3XbV/ipEy8ln3F5EzB4/H0p98P2uxh/2k9UvuCQnB85sNcev?=
 =?us-ascii?Q?i0ugqgK0FskMAQyBxEk5FFyVOqminxSdHJTzld2dhy2QOM5RaHkvr9hdsazd?=
 =?us-ascii?Q?I7nLQBWyGS2XhtnYxC71BceSd1IUCX3q7kSWp4t6oVo1tpGsh+PdqlEMNjti?=
 =?us-ascii?Q?fvnc9pR2wdkYsN3ZFREYEsHFkFUj/MU/Y7+5AIH2rldW47MveBGRlZNjZtlm?=
 =?us-ascii?Q?E0yXCTL2AgZcGN0iZehRHRtiHwNxRpQVDM0/sUmwXY45k0OXIytBio0WLw3B?=
 =?us-ascii?Q?C2RwUbJTyOCD6jLlXQAPaf54V0WSR2SBAm+lm0frd6QnXYrA+x2IpOFhgGwA?=
 =?us-ascii?Q?6fDTdj7izfCttJSgavpp2lNLKt0olxEAZDndqzjWldPClr5QSzutfss+JNKq?=
 =?us-ascii?Q?WGMp1nO/80VOUi+ZNSzlAMhnXKHRQqRVS9AhCkUV1378UAB5iAH1yIquqJOK?=
 =?us-ascii?Q?YAnmMpIs2hWvJSAK84EUO8CYXYN8zCPvLOHONTHatdx3eUYkYlp00I33i58x?=
 =?us-ascii?Q?EOJ7aWWdC7yHTSUiSQUVc8u5eqvpL7YbimDzKtlh/hHHPZN13PqGTh35Cze5?=
 =?us-ascii?Q?eDu9KmdqK0EcNj+rwwa0p2VvTIe7AZyIFQVjuY1OwN3o1AcDBm0uBkTVYAMd?=
 =?us-ascii?Q?b9kkqHxi0pVyLJCPcBt6QTIGkaCLNrznZ9KrHK5M3qv3waNe7zm4xIwPORlc?=
 =?us-ascii?Q?QcOyv52bA7kNJ+gJmVXwf1I/Mznb/vXQjDgSa+4/cmaMb5ecyBaEc3dpkdho?=
 =?us-ascii?Q?YiIzDWrpXkp+flxDXNY6o/F6K3wGKFHg8HuEwYs2Jth0yzmFgKDaCD+L8TeS?=
 =?us-ascii?Q?im3379qts9lJzRSMu0riNxUXv0R8MuTbWchSVSKMOb3aymPMyWy7wKiGbt8n?=
 =?us-ascii?Q?F+7Py74s10yyKE5yGBUczewjkQFBtynA7XDxqrz1Uplgwq50Xa6tNss9O1bj?=
 =?us-ascii?Q?Yw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?YZwKB3SV1WeCVq1/Fmx01Nm24rBESLj1YhGXTq5f4wGYhyeFbh0S6OUzaUBY?=
 =?us-ascii?Q?ha0YGc70cP2q6R9GzRSYOM4+iZP+Wv40TuZphLoOC6dp5+ESVOeKZYnWg9Ud?=
 =?us-ascii?Q?DqxVC28GkMB5vqSc2QQFfr9H6s7BKgWa3wR/+vE+3PyfsJOpdR8fawBi8fQH?=
 =?us-ascii?Q?XS0lMcXE1tzr7fk5ZhEfE0XpY9J4Q27d4GyjyyzA1KkRGAnlQwdxmD98qIPV?=
 =?us-ascii?Q?Cs9VOM8wYWFDscblpb//CwfLgxuaDOOHUNTfqTofu4T9snBqXOKjqp6qBOFa?=
 =?us-ascii?Q?OPDg5ut+qgXU5lHLNW3w99Ui8feTr3e2aNv6sxQL/L4wdbZ9JWSc0RZy6CfG?=
 =?us-ascii?Q?6sXg8KEhl8UzS2wlDQwgXN1mKHPjnx8CwHW/AwvM1/ke/DNJfrsrv2rT7L20?=
 =?us-ascii?Q?WXPRGayQwvM/IQHDT6IXLQ7BQZInPaa6FKdkaNKtciFzFS7PDGlcByUdpESO?=
 =?us-ascii?Q?irVkZvXoR+eknv+boKmD95VVUo4OITKxsar6e61khFk2Sk2iHtfPLv3mo/e4?=
 =?us-ascii?Q?/PTe6rp4R1ELeXbf81WvdMlupI+IQfBg/m5h3LHkYcdg/nx/8Kqv7yCVCdw7?=
 =?us-ascii?Q?cmuYOW1IHxrI0Jjiy4UFWwS3lKcOM0HpUquALNUIeGrBvdNUkyqF14dxHuta?=
 =?us-ascii?Q?zSv+RC8j0rBn+ozv+eJFvWEZStiKoP90X8sfbt1/ox+n5Lk1Hr35pkXU87bQ?=
 =?us-ascii?Q?7Nv3osro/HCjKFeGdonHHf8soGd2Utp9gavprRx+mFPGfQ0z7dGp6RXPcDeJ?=
 =?us-ascii?Q?3gPY/z2f55G/NpzUzG8x3ZI1SGmCcsdAA+yVPKBOsw7ic1GS1l+pJVd/puWL?=
 =?us-ascii?Q?zH7I7ajRq8GqSEIq2gzmjPo96aT5SeSNT+0CugB5RwT9wLZUhFV5//+LiGPW?=
 =?us-ascii?Q?4lYwqPhDdjnTLTmv9VUjE7uKz3B7fUFgns0NWQFZQADo/YI1I6horsgNebkV?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7fb4ee73-09c6-43ba-2713-08db5569ebb1
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 May 2023 17:29:21.8639
 (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: gWE7qnO5/mF522YGdng9YUehwzPjJVTBr6Aq5nNAS4oIVq7be2/YxFeTp1iKl28OEI9Ivv9LIT4QEDQCgngFFQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR10MB7810
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-15_16,2023-05-05_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 adultscore=0 bulkscore=0
 mlxscore=0 phishscore=0 malwarescore=0 spamscore=0 mlxlogscore=839
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2305150147
X-Proofpoint-ORIG-GUID: DpI-OhVwvEwvQPF3NRjWM0QgD3b-Fo7v
X-Proofpoint-GUID: DpI-OhVwvEwvQPF3NRjWM0QgD3b-Fo7v
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> [230515 09:18]:
> Add comment for mas_wr_append(), move mas_update_gap() into
> mas_wr_append(), and other cleanups to make mas_wr_modify() cleaner.

Sorry, no.

I won't accept cases in the comments and referencing it in the code.
I'm not making any vma_adjust() or vma_merge()-like comment blocks.

Please change Case 1/2 to "Append to start of range"/"Append to end of
range"

This path leads to chaos.

> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
> ---
>  lib/maple_tree.c | 52 ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 20082ef8c396c..538e49feafbe4 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4315,6 +4315,31 @@ static inline unsigned char mas_wr_new_end(struct ma_wr_state *wr_mas)
>  	return new_end;
>  }
>  
> +/*
> + * mas_wr_append: Attempt to append
> + * @wr_mas: the maple write state
> + *
> + * Return: True if appended, false otherwise
> + *
> + * Case 1:
> + *                       r_min   r_max/end_piv
> + *                 +-------+-------+
> + * original range: |       |offset |
> + *                 +-------+-------+
> + *                             +---+
> + *      overwrite:             |   |
> + *                             +---+
> + *                           index last
> + * Case 2:
> + *                       r_min   r_max/end_piv
> + *                 +-------+-------+
> + * original range: |       |offset |
> + *                 +-------+-------+
> + *                         +---+
> + *      overwrite:         |   |
> + *                         +---+
> + *                       index last
> + */
>  static inline bool mas_wr_append(struct ma_wr_state *wr_mas)
>  {
>  	unsigned char end = wr_mas->node_end;
> @@ -4322,7 +4347,11 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas)
>  	struct ma_state *mas = wr_mas->mas;
>  	unsigned char node_pivots = mt_pivots[wr_mas->type];
>  
> +	if (!(mas->offset == wr_mas->node_end))
> +		return false;
> +
>  	if ((mas->index != wr_mas->r_min) && (mas->last == wr_mas->r_max)) {
> +		/* Case 1 */
>  		if (new_end < node_pivots)
>  			wr_mas->pivots[new_end] = wr_mas->pivots[end];
>  
> @@ -4330,13 +4359,10 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas)
>  			ma_set_meta(wr_mas->node, maple_leaf_64, 0, new_end);
>  
>  		rcu_assign_pointer(wr_mas->slots[new_end], wr_mas->entry);
> -		mas->offset = new_end;
>  		wr_mas->pivots[end] = mas->index - 1;
> -
> -		return true;
> -	}
> -
> -	if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) {
> +		mas->offset = new_end;
> +	} else if ((mas->index == wr_mas->r_min) && (mas->last < wr_mas->r_max)) {
> +		/* Case 2 */
>  		if (new_end < node_pivots)
>  			wr_mas->pivots[new_end] = wr_mas->pivots[end];
>  
> @@ -4346,10 +4372,13 @@ static inline bool mas_wr_append(struct ma_wr_state *wr_mas)
>  
>  		wr_mas->pivots[end] = mas->last;
>  		rcu_assign_pointer(wr_mas->slots[end], wr_mas->entry);
> -		return true;
> +	} else {
> +		return false;
>  	}
>  
> -	return false;
> +	if (!wr_mas->content || !wr_mas->entry)
> +		mas_update_gap(mas);
> +	return  true;
>  }
>  
>  /*
> @@ -4385,12 +4414,9 @@ static inline void mas_wr_modify(struct ma_wr_state *wr_mas)
>  	if (new_end >= mt_slots[wr_mas->type])
>  		goto slow_path;
>  
> -	if (wr_mas->entry && (wr_mas->node_end < mt_slots[wr_mas->type] - 1) &&
> -	    (mas->offset == wr_mas->node_end) && mas_wr_append(wr_mas)) {
> -		if (!wr_mas->content || !wr_mas->entry)
> -			mas_update_gap(mas);
> +	/* Attempt to append */
> +	if (new_end == wr_mas->node_end + 1 && mas_wr_append(wr_mas))
>  		return;
> -	}
>  
>  	if ((wr_mas->offset_end - mas->offset <= 1) && mas_wr_slot_store(wr_mas))
>  		return;
> -- 
> 2.20.1
>