Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Tue, 12 Dec 2023 16:41:04 -0500
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Sidhartha Kumar <sidhartha.kumar@oracle.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        maple-tree@lists.infradead.org, akpm@linux-foundation.org,
        zhangpeng.00@bytedance.com
Subject: Re: [PATCH] maple_tree: do not preallocate nodes for slot stores
Message-ID: <20231212214104.fz7ifu5g76ayl3on@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Matthew Wilcox <willy@infradead.org>,
        Sidhartha Kumar <sidhartha.kumar@oracle.com>,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        maple-tree@lists.infradead.org, akpm@linux-foundation.org,
        zhangpeng.00@bytedance.com
References: <20231212194640.217966-1-sidhartha.kumar@oracle.com>
 <ZXjJTM0pDksnslgo@casper.infradead.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZXjJTM0pDksnslgo@casper.infradead.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?QfcaR+MWmolWqDixrbgWhiHoHKD/xp1Y9/FgHQAAMstz+LJ8uHacoWQHSGbr?=
 =?us-ascii?Q?RdW2JEhgTXjsTjEse1uXJ35w65AUeU7xvRoSv6wuHFlwuogdKc+4WGC17rhV?=
 =?us-ascii?Q?cOjGHHUClAz0rQ+7TrFU6VRJLMuuxT74jDWUv+2lQzW/RNwkXZGrfXI+fT6O?=
 =?us-ascii?Q?N5MWXi7ODL9sMNXm7ZLw5cMrxV9unBBQjgGePR2vxigbiEg74s0jWes9YgkU?=
 =?us-ascii?Q?12zFL6CqhgiR9PgDoYYD1D+sKpJo5r5cKSKpS+/feHpnG1p16GpQY5M3vtUv?=
 =?us-ascii?Q?uRJ5lgsJMKQowCr+8EW2XLoKzrAhUpZ0z9KECDmspb4CBDDG6pOkqzQgsSCl?=
 =?us-ascii?Q?8DeXCII4PEKJJMDMJre7xfqS4+b4Z+uqktiJ23ooOSyePJ4oIvUioFeEEqcK?=
 =?us-ascii?Q?K7qT/JBaJr74S7M727DEo7I5zxvTblbWq2ri+anCYQKE5il3vi6h0u2B3YRx?=
 =?us-ascii?Q?e2c6osQG6VSXYqU4mXqhgv099vEOCgkJD6FecwHnCUqGI6ve/qFibyfFeuxJ?=
 =?us-ascii?Q?nhxcZCP34sGZYeKov1CJWvO8aabr2Nmb/TN65acUxeim9C+pXiubBQX4kA/5?=
 =?us-ascii?Q?eCXAHjT2DyfY5qgt2Kt5nxYcZUAP+uk5oHELUCjE4s/CvkW/iKGhWkbAQtLx?=
 =?us-ascii?Q?HQeACI66jniQtg5cUqQJOKLhxQNMQ13qjg5q+JnsNSOPR3NmrqkwmWmVBV2W?=
 =?us-ascii?Q?3UZ77xGj841eYu+crOkJRFbVTDvXCcMgB2cPPhoH8cWN30a/c0HpZepKrLw+?=
 =?us-ascii?Q?E3NEJfUSy8V6CbdFEeUAWQbrldADsGW4GcvUI7wcjmNcnc14lfnH0Z1PIeIy?=
 =?us-ascii?Q?KdkBa4C6szlE3UxOn2yTWvnMAVEglKaokTW9FFoOXU9UrhK3FZiVazpKZq3V?=
 =?us-ascii?Q?f3l/De5SKo57Y7fq5jgSXcXXGxUirUFf8tWQQ4Ss2qzqR68iMzt3MLRwW7q4?=
 =?us-ascii?Q?+jS7PuJhTer3xmT/2JbMTLEeUEQ6DSwL1xy5ish6sOehAASQBvxZtj1x2JN2?=
 =?us-ascii?Q?qCAVySriSfMHR+hYX3+tcxckGhgxusCD14s1IphJLriQ9PN9uOvWuLrs+/F9?=
 =?us-ascii?Q?eq6XlFrh1mmAPgf1ykYVU/Tpa/Yo0qn5K2WWCsrxmzlbh0Wm1niTZWEzKeqa?=
 =?us-ascii?Q?P+dEWu0aQqxHEa1hx12+lPunr4l9e643MvrboSfiR6kYCXdc7yo5wpEjJwSu?=
 =?us-ascii?Q?r14NLviduldqp2xKc43rpvInyInMaNWj7KBVu1vzTqevtx9oLWOHVUy5uaaQ?=
 =?us-ascii?Q?AUQpHLQ7RkIUXd0C3gnKe/MaKK1S2k9+3KLUul6ddelPu6gFycsv9IA/hHL9?=
 =?us-ascii?Q?iLeiknDTy9LI+M2ch59kNVwMYgKbY41GqojJdbOy4mCOIKSasNOn+H6DAnx5?=
 =?us-ascii?Q?Aky6PDhD+bvPDoJarDRhI2n1/zF5Iyu28C3wxZByvSieauGllXbbme5wD0Y2?=
 =?us-ascii?Q?8yWylnS6lC4zfHRF3ASlaQWIdM901/0LSKAMk1C+rCp5n/0LyWFze+DjiApZ?=
 =?us-ascii?Q?KczMkuJvMvfsHPDfNet+bUVGG9P4kklTz/tP9eJfDw30usp4JoWNMIb4Okp7?=
 =?us-ascii?Q?BU7cOFiWy9/ZTXysEE/7fbv434XTX2TGSeQGadJ0?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?KBIbGoofltgIQqP2sf+LAujEnjJt+05FBb0L8P4lV1r0CIrFfVXQYRgejMwm?=
 =?us-ascii?Q?Pk2fzygOgCbIFvw6AV1ubxuzxWry0unvtjBYqxd4NIxrCosOxt9OrRvKVTWA?=
 =?us-ascii?Q?AyBbfp0ofgKprcx3ChLusUkCT/AWNDYmb6FYDMyy4UC8k7oIoIx5oP0kvfa4?=
 =?us-ascii?Q?YL7Yili6T7YnMctkIYkE21guxrEF2p5jryvwo43RuiSkocXHbXaE8h4kSktG?=
 =?us-ascii?Q?sfn0aLT/YbisEUM0AVAwzqjkJ9J5S0fpQOLNWYYKTRpJFlhi084P7D1bEx9C?=
 =?us-ascii?Q?llJHjW3hNWOZuOw0TpVqNJxMzoOS3NZaRc4NP1lEMDHuRy0lMbxit6xwDMPX?=
 =?us-ascii?Q?Zd6TWNIfXfAPJ8gFby78wbd/MstajlBI5gurBjsJl1R3jjZ7pBj6e/K8AEQJ?=
 =?us-ascii?Q?4yGq3yuxcOJCEYKxk/etP978gnzWXLDokDYeGYk9Mm0m0hwi461ivPqWF4vr?=
 =?us-ascii?Q?Rw9L+GQFqZr0maj2GJ2TIzNcPmNSqSs7NfH4BqxHdkPQwung2WGNkkbxqPDP?=
 =?us-ascii?Q?ZJXtt6jSEcQbWOev2YqMXPWSNTyFpQoiuVogeBNcelkectnE8o9td/nGPtfX?=
 =?us-ascii?Q?qH4GCFREKGtAl6EdWsza1gxLiq+34NLiEfg+E5cCy3/aBHflbsTdqyu7CIIM?=
 =?us-ascii?Q?+RE2ObEsFMyz/M9a60G9tokigaOEvX0+P/6uBnBopdSyjtfVwBCd06Igslm5?=
 =?us-ascii?Q?hrZoYs2vlmUwNqvYZ1e54ZwytZWqK7lkS86aieiOj44hs1FgmHfHYVh4fZA8?=
 =?us-ascii?Q?q6P8Q1/oS6Wf6o60uwkhhdLCocopTuQ+X1wzxiShj/aOPfCYR4Gdt7k7o1Q9?=
 =?us-ascii?Q?PeutUdTOiYOEwuo6lMBc3lGkQlAdsDotWvCtNo4E2tFRAaKNFxklV0dsOiPP?=
 =?us-ascii?Q?W4B+FqDuzboYOn/Sys5XcAbGqK0kgqYHyiZgUAY5pIfuqAC01n/+If53LTw7?=
 =?us-ascii?Q?VuLKsH105rWAmhc/ZGg3DnJ8USrBwZMwUlK7J46TYBw=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cb0ac229-894d-46e8-bde0-08dbfb5b0c79
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 21:41:07.4026
 (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: ndrGssVT4qpg2/4Oq6AZhfutFqHjKETlfiKk+V0eVdrDQZ5mUil3Ea0nIpytgdqt+FTz8CZw24K8w6rClbMrgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6282
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-12_12,2023-12-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=975 bulkscore=0 spamscore=0
 mlxscore=0 adultscore=0 phishscore=0 malwarescore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312120168
X-Proofpoint-GUID: g8sRg4pi5S-Mu5AyfaglUdLKe75LHrFL
X-Proofpoint-ORIG-GUID: g8sRg4pi5S-Mu5AyfaglUdLKe75LHrFL
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Tue, 12 Dec 2023 13:41:48 -0800 (PST)

* Matthew Wilcox <willy@infradead.org> [231212 15:58]:
> On Tue, Dec 12, 2023 at 11:46:40AM -0800, Sidhartha Kumar wrote:
> > +	/* Slot store, does not require additional nodes */
> > +	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
> > +		|| (wr_mas.offset_end - mas->offset == 1)))
> > +		return 0;
> 
> Should we refactor this into a mas_is_slot_store() predicate?

I'm not sure it's worth it as some of these are deciding factors on how
the store is executed so I would expect this to live in a single place,
long term.

Although, long-term this could be two store types: slot store rcu and
slot store so that the check only happens once.

> 
> A few coding-style problems with it as it's currently written:
> 
> 1. The indentation on the second line is wrong.  It makes the
> continuation of the condition look like part of the statement.  Use
> extra whitespace to indent.  eg:
> 
> 	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
> 			|| (wr_mas.offset_end - mas->offset == 1)))
> 		return 0;
> 
> 2. The operator goes last on the line, not at the beginning of the
> continuation line.  ie:
> 
> 	if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree)) ||
> 			(wr_mas.offset_end - mas->offset == 1)))
> 		return 0;
> 
> 3. You don't need parens around the !mt_in_rcu(mas->tree).  There's
> no ambiguity to solve here:
> 
> 	if ((node_size == mas->end) && (!mt_in_rcu(mas->tree) ||
> 			(wr_mas.offset_end - mas->offset == 1)))
> 		return 0;
> 
> But I'd write it as:
> 
> 	if ((node_size == mas->end) &&
> 	    (!mt_in_rcu(mas->tree) || (wr_mas.offset_end - mas->offset == 1)))
> 		return 0;
> 
> because then the whitespace matches how you're supposed to parse the
> condition, and so the next person to read this code will have an easier
> time of it.
> 
> 
> -- 
> maple-tree mailing list
> maple-tree@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/maple-tree