From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 47B9EC3DA70 for ; Tue, 23 Jul 2024 13:38:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B3C456B0096; Tue, 23 Jul 2024 09:38:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AECB56B0098; Tue, 23 Jul 2024 09:38:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9B3856B0099; Tue, 23 Jul 2024 09:38:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7C8796B0096 for ; Tue, 23 Jul 2024 09:38:34 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2908A41EFA for ; Tue, 23 Jul 2024 13:38:30 +0000 (UTC) X-FDA: 82371121980.16.D66164A Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf01.hostedemail.com (Postfix) with ESMTP id 5083A4036E for ; Tue, 23 Jul 2024 13:33:41 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=YfHjhnId; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf01.hostedemail.com: domain of dakr@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dakr@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721741570; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zb9aNJOvch0pvj+LqTB0VnCzqhmsnl2gIog8rkikLjY=; b=3Rc/QB7eRlzBqTlWjnvTOnSQ4lQnO+ezRPu/XZdNEwapPr/6R5E2PXwJp1INVrNK5KXGy8 67IG2r+QI3hLo+DIPdqF4z4Br3VP16tnIHvtmwUsxA12UkcItazrE7nqEL4CUZlKx6SLzK 6zjMew5AxMJ6BysVr9HpaQb32Xfr9Ec= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721741570; a=rsa-sha256; cv=none; b=zvG0dOnUEV+sLFbQCEoAmfbgvbCklfvYtjwzQj49QVxgO3+jIc8u66T0Ws2TSGMMOuFLiQ wOKklh0EQQdZgDAVpmK0mlCya2Aq40o1Qsxt3o6o9dN7BRj45dg6MCIq5itrQiX6+0Dl1m 7knZu+JHujt2xmFzDxBPqKB8BJIx2VQ= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=YfHjhnId; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf01.hostedemail.com: domain of dakr@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dakr@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D602560FD7; Tue, 23 Jul 2024 13:33:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 318F5C4AF0A; Tue, 23 Jul 2024 13:33:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1721741620; bh=sbp9otUqlv/cDrBswp3q8g8x24As4Td5+acClwkG1Zk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YfHjhnIdE5SMmArN8TsXhyZc5mfeB4oxz5+nXPOTLu7hd4bk+ndBHFqhgv58waBJM 7N0ga68G/gzkct6mID7Mz4VybS56sB/UiTAzKgRdpPSfz+t+/9irxMvD3/diQvkngS eJwWsCCeKZLKNq7Tn45nl/OR5YpYlMq9CtkgoszKd9OYjaVxXb+L3cJozuY4FUd9d/ EI0pG6uLwrX92pdtgV6antoWWaF3I1IaGf3QSH0yHQlt7SFLoLxCE3MLEOL5yvUy/A BIcMe0UMfDK2StiRWAcmPwBan9anDoyr6Rl/4Fd9seiZEnCZ0H+D1NQqdpGNfQn7Ni wgXbaoFPvFgJg== Date: Tue, 23 Jul 2024 15:33:32 +0200 From: Danilo Krummrich To: Michal Hocko Cc: cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, akpm@linux-foundation.org, vbabka@suse.cz, roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, urezki@gmail.com, hch@infradead.org, kees@kernel.org, ojeda@kernel.org, wedsonaf@gmail.com, mpe@ellerman.id.au, chandan.babu@oracle.com, christian.koenig@amd.com, maz@kernel.org, oliver.upton@linux.dev, linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH v2 2/2] mm: kvmalloc: align kvrealloc() with krealloc() Message-ID: References: <20240722163111.4766-1-dakr@kernel.org> <20240722163111.4766-3-dakr@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 5083A4036E X-Stat-Signature: r35y74e654ruypecdki4ujnk46cy8gzu X-Rspam-User: X-HE-Tag: 1721741621-768985 X-HE-Meta: U2FsdGVkX1/dmpogWyMIzBSBGb6KvAWJ0VcLxxhWz7IW59C4jLV2vH2eac+BTWyg94PpB9Nl9ULaXrQXKLu0/0LBCGEJPVNwEbQ0xv+kTQng7bWfAT9jAmPc3+1JRxf8uyjTw2VLT+7noZb5LFCJEAAtaC/Fgy0HUHpwYD7WNqQCmOzFOiX4Qgj7iGNhPlqUarB8zi2RurTN5UVnN8+Xorj1Xz82w3D+B7RGQ3l6oGZRtaSm7AKI1SgscFKLT00F6qs+gxH3Y9ouOgwQIn5qzgsjrugvzxNMhw+7Jg0gOI47dE/DjnwONpZr0ok2RgGB4ADh29coTTdiK1B9C83ZjmXCYIFO7ihwwrzlS4F2nccqSESxuTb8317sqdNu73deYTCSxqMyuEFQF0qs31vd+1ybY1cRCVhGrpDSYuffzbOUIMjFBkNiVt5l92aT1VRipV4o7eBDSmX092wyzdeUp0NagWRQhGXd+ejaYfAV3/Pv5S8/tJ80IAm0sjGXgQES7VJGBxv6G6vX7Ac/F4KY38Zb/OoVjYAQdwuSYVex1ufZKIZCUNojFb0j5eXEM3KDgFbqHnY5q5K+ybc3Vj+FCwud8zvJMlr7ezkfpqhTto3/t6POpBzH9LGGV2GRrl3GO9YaKRB7wZPlWW7NqPm5j2iMj4qlazj5nmeGQW8Pgk0YO42NXct2lzQGDlSXj3YPPlm8X4pEuHBGwu38SwMqGK7ZFX+qirb4P3n+B+Nupho6BTwq6ixZTKhshCuQXkExnWnZZP2jo3aq6ulUYImyWZG6bLtfNVfF+M7BX1LqV5sXOSqB3lSiKdebRyiknuxnt92NakrZ2i3rsSRyA14c00d094njANpY7oYTrXNizeotwMkAEn0CC11BPl3KICUVLEvM+f7iRHFONtf+Ug0ygwKK2+g6JFsM8l7yOT/wo1YSjGkiFhUeB5AWe/jixYWrwbmwtrLjxolrVhPyL56 hlqxjuKY HrfrjFXkf8ZR2oHxyinoHUk9o00SuUmIxErv380iBrmfM3HEMdUuOCQpr4/d2U7kMJm3alx00fILgFT8lp0hMjdgevx5AkHcuX8C4WEWlXcCUSgmxEpYaPcKBABUvef+4rQrV/Ln1CDtrXXaB3CP4frBreYhWQOvmF5yQcjPvHrF1NI882+zaJaTftwTkQmSqL4h5XjTNJzKeBVzusrQGwGJ7yEdv7L/vNyBIxIUOKXiYCpYVFNkbQtuHUxDXO03X6zu1V0+/m2KLTgvqC8x1bJHUCvWoSqiAxABNFIbHJxJ2tBo= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Jul 23, 2024 at 02:12:23PM +0200, Michal Hocko wrote: > On Tue 23-07-24 13:55:48, Danilo Krummrich wrote: > > On Tue, Jul 23, 2024 at 12:55:45PM +0200, Michal Hocko wrote: > > > On Tue 23-07-24 12:42:17, Danilo Krummrich wrote: > > > > On Tue, Jul 23, 2024 at 09:50:13AM +0200, Michal Hocko wrote: > > > > > On Mon 22-07-24 18:29:24, Danilo Krummrich wrote: > > > [...] > > > > > > Besides that, implementing kvrealloc() by making use of krealloc() and > > > > > > vrealloc() provides oppertunities to grow (and shrink) allocations more > > > > > > efficiently. For instance, vrealloc() can be optimized to allocate and > > > > > > map additional pages to grow the allocation or unmap and free unused > > > > > > pages to shrink the allocation. > > > > > > > > > > This seems like a change that is independent on the above and should be > > > > > a patch on its own. > > > > > > > > The optimizations you mean? Yes, I intend to do this in a separate series. For > > > > now, I put TODOs in vrealloc. > > > > > > No I mean, that the change of the signature and semantic should be done along with > > > update to callers and the new implementation of the function itself > > > should be done in its own patch. > > > > Sorry, it seems like you lost me a bit. > > > > There is one patch that implements vrealloc() and one patch that does the change > > of krealloc()'s signature, semantics and the corresponding update to the > > callers. > > > > Isn't that already what you ask for? > > No, because this second patch reimplements kvrealloc wo to use krealloc > and vrealloc fallback. More clear now? I'm very sorry, but no. The second patch just changes kvrealloc(), how do you want to split it up? > > > > > > + if (is_vmalloc_addr(p)) > > > > > > + return vrealloc_noprof(p, size, flags); > > > > > > + > > > > > > + n = krealloc_noprof(p, size, kmalloc_gfp_adjust(flags, size)); > > > > > > + if (!n) { > > > > > > + /* We failed to krealloc(), fall back to kvmalloc(). */ > > > > > > + n = kvmalloc_noprof(size, flags); > > > > > > > > > > Why don't you simply use vrealloc_noprof here? > > > > > > > > We could do that, but we'd also need to do the same checks kvmalloc() does, i.e. > > > > > > > > /* > > > > * It doesn't really make sense to fallback to vmalloc for sub page > > > > * requests > > > > */ > > > > if (ret || size <= PAGE_SIZE) > > > > return ret; > > > > > > With the early !size && p check we wouldn't right? > > > > I think that's unrelated. Your proposed early check checks for size == 0 to free > > and return early. Whereas this check bails out if we fail kmalloc() with > > size <= PAGE_SIZE, because a subsequent vmalloc() wouldn't make a lot of sense. > > It seems we are not on the same page here. Here is what I would like > kvrealloc to look like in the end: > > void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags) > { > void *newp; > > if (!size && p) { > kvfree(p); > return NULL; > } > > if (!is_vmalloc_addr(p)) > newp = krealloc_noprof(p, size, kmalloc_gfp_adjust(flags, size)); > > if (newp) > return newp; > > return vrealloc_noprof(p, size, flags); > } > EXPORT_SYMBOL(kvrealloc_noprof); This looks weird. The fact that you're passing p to vrealloc_noprof() if krealloc_noprof() fails, implies that vrealloc_noprof() must be able to deal with pointers to kmalloc'd memory. Also, you never migrate from kmalloc memory to vmalloc memory and never free p. Given the above, do you mean to say that vrealloc_noprof() should do all that? If so, I strongly disagree here. vrealloc() should only deal with vmalloc memory. > > krealloc_noprof should be extended for the maximum allowed size krealloc_noprof() already has a maximum allowed size. > and so does vrealloc_noprof. Probably, but I don't think this series is the correct scope for this change. I'd offer to send a separate patch for this though. > The implementation of the kvrealloc cannot get any > easier and more straightforward AFAICS. See my point? > -- > Michal Hocko > SUSE Labs >