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, 8 Aug 2023 10:29:45 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Peng Zhang <zhangpeng.00@bytedance.com>
Cc:     Vlastimil Babka <vbabka@suse.cz>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Christoph Lameter <cl@linux.com>,
        Roman Gushchin <roman.gushchin@linux.dev>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [RFC v1 4/5] maple_tree: avoid bulk alloc/free to use percpu
 array more
Message-ID: <20230808142945.tulcze5bjg5ciftk@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
        Peng Zhang <zhangpeng.00@bytedance.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Christoph Lameter <cl@linux.com>,
        Roman Gushchin <roman.gushchin@linux.dev>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        Matthew Wilcox <willy@infradead.org>
References: <20230808095342.12637-7-vbabka@suse.cz>
 <20230808095342.12637-11-vbabka@suse.cz>
 <853af8fa-0cef-b00b-3fd6-9780a2008050@bytedance.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <853af8fa-0cef-b00b-3fd6-9780a2008050@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?ZWszdU1JaVFSZUdSdFFxS0Y5UStlOEtJNVdjWGNVUzRyNDdnMGo3dlpkN3hr?=
 =?utf-8?B?RGY1UkNNQlJKNGFHenQvMlRFVDdJQThMT1VjRmFzeDQwdGk0V2V2VzBESnF0?=
 =?utf-8?B?aUtnT3VNbjFCM3Y4TlF6OTg2SWJmcytKMllrbUYyWkdkRVB6NnVFdWhPVklJ?=
 =?utf-8?B?YVZQRG5lcitOb2Vsc3E5WGhONVZ2QkI5WkdRZEhlcjVRdWE0c2tmSW5SeFRs?=
 =?utf-8?B?bVlCR3l2Y2YycVNMejFKdEduQmtETWlmWlRWcURNdEh0bWd1dTNqMFZXMEEr?=
 =?utf-8?B?VzkyS08rVkdoUXBXYVYyYU1NU1N6aVdDVHhLd1lMRytKZWtRbFRCVHhqYTRt?=
 =?utf-8?B?USt4ckJFSmdaY3NaZHg1eTk2K1NJemU0U2hIMFVzQ3VJdlVDTHM1cVRBTWJa?=
 =?utf-8?B?bG04eWVKYmNGa1JScVRpWExyN29nWWFQVGlLemQ4eUFQTHAyNEJrSFl1YlVz?=
 =?utf-8?B?WFJla2RHK2Y1ejRYQmlQTmlXdGtZQXNsTlpjVVFzdEx1WlVleXFhMSs0NVNy?=
 =?utf-8?B?ckhVWFoyYTFBbnN2cmxOU1IwU0ltdlVTcWxBOGV1UDRoNktCSS9Wc29MWTJS?=
 =?utf-8?B?V05CckNQdkJyKzRnNkZQQ3R4RXFZVmdGK2lFRENjajVGODdZWGl2SUZ3RytZ?=
 =?utf-8?B?Q1IvSVU0eHlzc0lzTVVTUHpmSko1RC9ET0xVamc4WmI3dTlaeUZ0eExWSGJG?=
 =?utf-8?B?V3RlYm01L3dIeWZpY0pwMlVsQmY5Z0p3N3RJNm1CZWNwNlhYM1Bpb2JDMzNS?=
 =?utf-8?B?eUdoM1QvSVdJSW1oaCs3bGRlN2J3ZWpGZUlwV3I3K2JRTUJnRzJwQ1krUWZm?=
 =?utf-8?B?OFFLQUh5S2cyTWNnWTBPMDZFbldLZFpxenJROHVCNk9qTWpGc2lBclJMeTZr?=
 =?utf-8?B?aFZaREZZc2hHbHp0blpNQVdIWUxBUXp0RnpjeitzM2xVS1FMQ1pWQXRDNVVt?=
 =?utf-8?B?OEc2ejA2ZlhBM0xRb2pkelB1Mmw1ejBHR0IvTXZwMUYxUEZZdWtudjQ5MmNh?=
 =?utf-8?B?QmJkRlprMklQSVdCQ21ZNllOdHRVOVdFdE5NOStLVHh3L1JEcUxYWTJqWng4?=
 =?utf-8?B?Rzl6UmRzWEI2VkVXZ1cyUE1Da0gydTFWcnlvOEVEcnl2MUNma0ZXQXJJL0xD?=
 =?utf-8?B?ZERNZmF0RDJGbmNPMkVXbzJ5YjRVTW1FWGxMSnQxY09wOXFDYzZUSE00eE1B?=
 =?utf-8?B?MnpCOWZPbEo2ZmMvNFordStwSFhvZnNzVnhlL2dTN3ZpdCtteWpieEFuNTds?=
 =?utf-8?B?bzRXNzRJUkthcTBqSEVtZm9qb0JaRHZubWR5NVFGeUxIVlJGWVNKSGsxS1pK?=
 =?utf-8?B?czB5YnRFS1IwVFRlR2hoYTIvYTExWGhrdnM2YU1TZFdNaklGUUlQOCtMNGx0?=
 =?utf-8?B?R3FIaWEwR0lTWHlKTFBvSEVtcm1FT3pJUE9kVmVpWnJ1MDhKbkxzSGdiWlJ3?=
 =?utf-8?B?NlhtTDlCU0ZQczZTZUNHMlgwWHZSbjduMXZGNGhUcWRlVU96bGYxUmdRWEhY?=
 =?utf-8?B?RFBQL0lIenhKS3F4SkRiRXB4cjN1QmdZY1BHamJudVpja0xsMXFJRmFtRUJ2?=
 =?utf-8?B?MlluY09jUjU3ZUF3bkpMSzlxK0Zjd2c5YWUvSEhmZTVHSktGMUJLU0ZtaUVH?=
 =?utf-8?B?OXMzUHpSbUl6K0pkVkliRUNqM3hMZThtQjM3NFZxK2F4aWFiNlFjTkdaalQ3?=
 =?utf-8?B?ZmR6RG8xMVQzSTR5YXFEQmQyQ2U5MnBVaFd6YzVYNytBUHhRNFRiVEtDaXRs?=
 =?utf-8?B?TDI4NDBqeGViK1BURXcvOGtiZzlTUFpzY3l4SFg0OCtCbTZ0TmJBQXNFaDJy?=
 =?utf-8?B?d3pabmR2aUJMNHpRQk5vajJWSEh2WUkyaW9aOEttK1h4OExYekFWL296ancz?=
 =?utf-8?B?RmFvWVRZa2JVdC82M0hPWnROcVN6SzU0ZHdacy9IbW51dDlwS04vSnBRZ2d3?=
 =?utf-8?B?WlA5VWlDcVZ6QlVHSmxKMUNnTXpaL0c4YnBITnpjSFptK3pKQjVqb1JMUkZ6?=
 =?utf-8?B?ODh0SDl1RlYxNkNtUHhCOUY4LzJmVVJncmxNa2dodFAxTlJFTjdmalpGLytS?=
 =?utf-8?B?OHdET05sQklpZVVCN285SlROWTBGakIvbGNSTzdxYXVsVFpHMkdtSHk1UUJS?=
 =?utf-8?B?S3REakxzWjNiRHYzOWdPV2FiTXd4YTMxRnpjeEtualljR1BoWEN3MVE3dmFs?=
 =?utf-8?B?VkE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?R0Q5bFpuS2NqOHBySVQyNGJPUE5MTmZHMlFJcXFTeTZ6NHhrYmVQTUNBWkky?=
 =?utf-8?B?azhYNjNRRVloR0V0N2IyVE5MNnBPY0IwTDVubXN5eG5PTE5tcHNmdzZ0anJ6?=
 =?utf-8?B?OTdYTFdNYjk0a2JYbG8xMUM5QXljelJUOUlUMzMvRUt3WC9NaVNPcGd6WTFG?=
 =?utf-8?B?SW9OMGQzUUovU0VLQ2VpUCt3WUhmQXJNY3ZQWTQ3QnFpYVpoU0lqZm8wYTRN?=
 =?utf-8?B?b2JXYnNTdFAwQzVHRE0wK3JLUUNXa2xqY3BmaVJGbXJpekhXS2E0YUM0WGN5?=
 =?utf-8?B?dU5YS0tnaTVtYVBha0xJTHdwK0daeCt2UzA4d0VKUDBwcEZrTFUvamtFcXEw?=
 =?utf-8?B?R1IxemJIREMxZzh3cTBJMnJNeThkbko4aTFESFRDUG5ZVlhpSnJLTkZkUXZG?=
 =?utf-8?B?eWRxdlFJaFZocnZuZDk2ZGZ1UjJiM0lOTUVGeDZXL0tvRkVsZnZTM3JUalZY?=
 =?utf-8?B?OGNnbVR5SGVhN1pMMWJmYm1TS3lFZENPRkduMEJhS3Y1UnZjRndyTUNONXRa?=
 =?utf-8?B?OTh2N3JlTm54ZGFxWWdvVExsU0J0Z1hnT3JUQTFudkdXa0lYY0lhcjVQdG5U?=
 =?utf-8?B?ZVhQODB4QSt4SFRENlBTRjFPNk4rUHJaWmV6c3R2MkxQbXBNOXFiZlNUOGZ4?=
 =?utf-8?B?SE9lczE1QnZuZ2pKOEtUUm1SZDlTL2pnOERrZW8zVUlZQ1g5aEhqa2pYNmdn?=
 =?utf-8?B?dTNwRStLSmFUSGxhdXNIR29OaytaUzRiNGFMZlVoektVc3VrSGNWelQ1OFZ6?=
 =?utf-8?B?dEdSWWxXc1ZKKzFFWVlMRm02VVdTR1pOSHlhaC9ReG9kNnh2cFpWK2pVNnl1?=
 =?utf-8?B?RXlRTmUxUFppeFhsK1dNUDd2cGtwRjdGOGZudDIvZVNCajJiTTQrYThYS2VP?=
 =?utf-8?B?ZCt3U2VpQkZUNjYxZmhuK1BieWZueEFheDNNNHFBQjlUOS91aVdINHo5V0RD?=
 =?utf-8?B?SFdGWFMzVU92WCt4Ymh6blZwenVObnJlUEJMNTdtRHVVakRrMWZXZXNvTGls?=
 =?utf-8?B?VzJXSXVnWUhqS0ZnOVgydWtvc1JyTkg3QWVYaHJHMVd5c3NWZzVpdWQwcjU4?=
 =?utf-8?B?NHRYanFjb0VVSlpjdnlqQW42Zmk5U0RXdkJpVDFWWEVGRWsyMHNMcGtSSzlQ?=
 =?utf-8?B?akFYUVpWR3Y2WXNNckxIUXpiRUc2N2IzVUdNUDczREdyWDRxaWxCeldiOHJN?=
 =?utf-8?B?bFMzRkpaQlJDNXlxWWE4WTBHMjRCUmkyeHdVelVSa1RTbmR4SUp2MkNiM0hT?=
 =?utf-8?B?ZHlVR25NSVZDbUlvWVMxOU9PY1RETkJDandPSURtOFArL3IrTDE5NVM0Ums2?=
 =?utf-8?B?azVMU1VFd3BYNEx1Yi9heHJyQ3VONlFMT2w1NWhtRUhGNW1JTGxMRkJGUklp?=
 =?utf-8?B?dkRTd2UwS2tqZ1pqa2FIdTBuOU5ET3JuNks3dWhFNCtrQVlSU2Uwa2JiWHov?=
 =?utf-8?B?TVBTQzIrMFhIbUd5NXVnYzJES056L1R2ZVFEbkVBPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 239449c7-6d8f-4001-2564-08db981bebf6
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2023 14:29:49.5031
 (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: 4/hhpJ/O0P8Joh4a5qNqyBf/Wstv43jtLllbFscY/oOgKWtbbQb/jI26qjZyB1StCDBQ9+fSi+tsicbsKNFvKw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB7051
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-08-08_12,2023-08-08_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0
 adultscore=0 mlxscore=0 spamscore=0 suspectscore=0 malwarescore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2306200000 definitions=main-2308080130
X-Proofpoint-GUID: u8KD0DjoG1lnaRxt9Z3jYmWBMfeiUlNk
X-Proofpoint-ORIG-GUID: u8KD0DjoG1lnaRxt9Z3jYmWBMfeiUlNk
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_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        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> [230808 07:17]:
>=20
>=20
> =E5=9C=A8 2023/8/8 17:53, Vlastimil Babka =E5=86=99=E9=81=93:
> > Using bulk alloc/free on a cache with percpu array should not be
> > necessary and the bulk alloc actually bypasses the array (the prefill
> > functionality currently relies on this).
> >=20
> > The simplest change is just to convert the respective maple tree
> > wrappers to do a loop of normal alloc/free.
> > ---
> >   lib/maple_tree.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> > index 1196d0a17f03..7a8e7c467d7c 100644
> > --- a/lib/maple_tree.c
> > +++ b/lib/maple_tree.c
> > @@ -161,12 +161,19 @@ static inline struct maple_node *mt_alloc_one(gfp=
_t gfp)
> >   static inline int mt_alloc_bulk(gfp_t gfp, size_t size, void **nodes)
> >   {
> > -	return kmem_cache_alloc_bulk(maple_node_cache, gfp, size, nodes);
> > +	int allocated =3D 0;
> > +	for (size_t i =3D 0; i < size; i++) {
> > +		nodes[i] =3D kmem_cache_alloc(maple_node_cache, gfp);
> > +		if (nodes[i])
> If the i-th allocation fails, node[i] will be NULL. This is wrong. We'd
> better guarantee that mt_alloc_bulk() allocates completely successfully,
> or returns 0. The following cases are not allowed:
> nodes: [addr1][addr2][NULL][addr3].

Thanks for pointing this out Peng.

We can handle a lower number than requested being returned, but we
cannot handle the sparse data.

The kmem_cache_alloc_bulk() can return a failure today - leaving the
array to be cleaned by the caller, so if this is changed to a full
success or full fail, then we will also have to change the caller to
handle whatever state is returned if it differs from
kmem_cache_alloc_bulk().

It might be best to return the size already allocated when a failure is
encountered. This will make the caller, mas_alloc_nodes(), request more
nodes.  Only in the case of zero allocations would this be seen as an
OOM event.

Vlastimil, Is the first kmem_cache_alloc() call failing a possibility?
If so, what should be the corrective action?

> > +			allocated++;
> > +	}
> > +	return allocated;
> >   }
> >   static inline void mt_free_bulk(size_t size, void __rcu **nodes)
> >   {
> > -	kmem_cache_free_bulk(maple_node_cache, size, (void **)nodes);
> > +	for (size_t i =3D 0; i < size; i++)
> > +		kmem_cache_free(maple_node_cache, nodes[i]);
> >   }
> >   static void mt_free_rcu(struct rcu_head *head)