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 D423AC3DA63 for ; Tue, 23 Jul 2024 12:12:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 640016B008C; Tue, 23 Jul 2024 08:12:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5EF016B0092; Tue, 23 Jul 2024 08:12:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B5CB6B0093; Tue, 23 Jul 2024 08:12:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2E24D6B008C for ; Tue, 23 Jul 2024 08:12:29 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id BBBF4121EC0 for ; Tue, 23 Jul 2024 12:12:28 +0000 (UTC) X-FDA: 82370905176.02.3C9B33D Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf15.hostedemail.com (Postfix) with ESMTP id C17DEA0002 for ; Tue, 23 Jul 2024 12:12:26 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=cp9BK1u2; spf=pass (imf15.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721736724; 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=c5TBBYvkaFdlMlxn3O2wAlHCINLNphPO6tKhTgLcung=; b=5WndUahb13dDJ53482fsMqmB4z8tRiC8iqOhgIneRscxxn5Z5qMDj3YKh2ivlrWW8CKtzH FB1PH5XgZe0xyY07FOrxFrWkJcA4oolwn7IT8npeUzrITpPxTSeaberKheeku/D3HQoIvo oc+LGYxP03d5s/c2eAUv4WhbI5Aw4k8= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=cp9BK1u2; spf=pass (imf15.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721736724; a=rsa-sha256; cv=none; b=KOBX95MI386tSpaiti6vztud8GlP+Qq+HDMql8E9LXCYcgowHBWm41doct/Ban1eRFwF/3 NAT3oct8TighDwpBrq7xnTKBkzhbVsXvXrMbcVXk1pni5OnRBFg22ETz7rWbuqwBoENZoK /nPhvO4rzjzvaeaF/QRxxEa0771qd2E= Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-a7aa4ca9d72so22655566b.0 for ; Tue, 23 Jul 2024 05:12:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721736745; x=1722341545; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=c5TBBYvkaFdlMlxn3O2wAlHCINLNphPO6tKhTgLcung=; b=cp9BK1u2RSz4bgEC95JInm8CnXugRCKZ+qgbc7u9vwFYPSPW5P7REUQMEO2h+IwBtF 3YChBYrBtUpzahWfkAkVoMtvkCZX+VJDXmE1udBJoH1D/VGlZBfxPaS0PDO33r7vc32+ qIcc/vgPs5HDI3swXUnffCtEVw2iLIA4EZPGTeszFj9USuctoPL5zVAFo8biw7hLHrB8 SQBKs0qrYqxz9tH9KiOynwHHvAlbic2HZ6bRcHGSiXGSU3aDwA0bG0IQt4PvG/nDxM50 YMFnSrsdedbBtP/cqZ8CQiqcsnRB8qZvl1Ftbp5HudBLWmU2BBK2/xiFtAM/Zbr2i3Rh ODJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721736745; x=1722341545; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=c5TBBYvkaFdlMlxn3O2wAlHCINLNphPO6tKhTgLcung=; b=ocPYmdDX2DJK4FUhsUxo5deOm2sd+Xv0EAXdeuwnPXe0Wh8EiVyRLFjtPoCppmI4VF 9PTyJbFuLdWeCclHstuIEzotkV5rOAFuSYbJ4nLqVHsEedBbrz9ouQbweVjkmm8D0mVu w5p9fIxlcsYPUAca1v8o/ovUnvhmQAbKxUjSBuI4qsR0kI9KWaocX4ExQRHKevUhjAdc +M2EwzOZ2cad2ylB4wlUFWvizSIFJPVoCQ71ZUXlfKOttAygN6jNvsvNp3typscYstBM zj6u7QNA2P1rYpfposwG3sJL38FjgLYHmo5ptrdeS/E4/mYG/4LbD7y2kgzWHQtxUoD/ pTxg== X-Forwarded-Encrypted: i=1; AJvYcCWfIBNMK77R0Gj4XsrFG8NRpzOU5GnUvw0S0P4vT7GZUcH2RHaj6PNzY9/ZTL1uEgndubZgHQNAJYj+wqojjAH1UxM= X-Gm-Message-State: AOJu0YzJq6f72om8TZUobGbV3s3aZsOOy7wVnBFt47MSVpeE4ejyg6Tf 2XLC4jlmCkiEEzrg1uCoymKpitRfB35c6MXlYBZrLUGRKG9gY18PWPiY0zSjSaY= X-Google-Smtp-Source: AGHT+IHKUI+ybyEF8IA4etomsFPdqHkKhA1yjCsCtCHNHdtBaOTVOe+K/W/574pOPrGEAdtQTfOWpw== X-Received: by 2002:a17:907:60cb:b0:a7a:8c55:6b2 with SMTP id a640c23a62f3a-a7a8c550709mr197491766b.14.1721736745045; Tue, 23 Jul 2024 05:12:25 -0700 (PDT) Received: from localhost (109-81-94-157.rct.o2.cz. [109.81.94.157]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a7a9e29153dsm49768766b.7.2024.07.23.05.12.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jul 2024 05:12:24 -0700 (PDT) Date: Tue, 23 Jul 2024 14:12:23 +0200 From: Michal Hocko To: Danilo Krummrich 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: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: C17DEA0002 X-Stat-Signature: ftbon3mswhqp3j475x7sug37mgbt9ome X-HE-Tag: 1721736746-271969 X-HE-Meta: U2FsdGVkX1+i0VhgS/ejZaR1vaYT5yUw7gHp878XP2n/dXIXDdQzL5HZ3Qgq+EYgB3rNupyzNWhoAQh4DcNY0EBrSpLeke53d6IM0TV2VF4CXEUcnagnOtWPkxK+Cbc9RDgHzqx5OGGuIkMkCuSIVJui87mWU93612fpI6DAozXIqPeArEGbFbNhh5u2g3NKVCo1CPD64Y4IYV3HmeZrOz/VMAToeoYi5MKE/FCh0JFBDxZ7oHPAVgFGIOA/boblXFrixGIMDm9eVROVN5IVETyX++g3C88WANiaR8GYGIPzHRhgByiss4rwcWWQH09nhFNL6bT4QzyFfNnia590uQ7ODr3ZP1e1Wnhlwex/NQvDsTmQ8rMTWP+yVASew3lm9uIur5OVIICYOjczI9kdLCLcICy0ko0ZH13k28O/1A7kIx0wRvv+eVZSYJ7iBdhBrhuXwH/7R95McBEqMa8HOwG5IRtWoEjArZJCG4BP3vg4Rqb+WCWYV/ZBrbCDtm2bTLtSGpmU1fHHlFq0TayIwaEqD3dUW1M+MN7S4j6GM1OuDNCrmr/bPKERNOEJHaoI9qJcNJp9PMEew4o4FxsavJIfg+lyQfbObycpyagJftyX00QQq1To9XENRy8izdkZfpBywjg5hFtoKkzHkv/oWb+tl2zaT3Duw5rf7S3iy+zJ0EiQiMReYZ6tVwBSE6vFj1rPKZzxKNv1mOIYxNTIKwkrml1iUFtVdwXp8uLDpAz2DqKHWVQ7RqhEQazjOiHc1syI3tnBlH8lLTyY/QMDUnThonNRN2BVchUulpWKPM0ocovJqRbbcpLVzKkk4Gbx/I7eiAv+MliAZ8hru5HaPYWyzXhCDvUARc42NwQoW6djA6nTdiBaJmvaxErXVP1s13O3LIP2scadYgM7QhKq0lMUolw91X0OU2gO2c1yUcMsHH+atxA1UHeo+4GHJsNQoC/mwj7nacq9FdbvV2e M9A/U9tW K9Ok1CV+PLgb0aPc7zGzKI1JXPBtMOXsxsbdMnzHgamgD+/6NkP8BzoBuybk9byyOTlXJO/W6mUlBGRuWeLxq8BprehAEJwPEhq0a/q3SKwPlXex5ac+yHazH/X+c0AtmuNWdtAG0+QwQA/Ee6uJv6HlqsFSVp2L8mC/OgZ4lA0UxmqjPXn6oA2FTRlMFOByx8Y2E6Mcg4CdtZKaZlf4corriET5EhqQcjgdOjgOVXM8+QK/xn+QjHTTepYlEaL5EfgfmvCCMhd42qfgIlUN3jqzfcB/PBfmpnwAgS+V69t9ShJjfjF5pr3K13CSnZqUkybmFZdeIGpSl1KTi67KnV5ckvDameLsBxOhqRjMpZor5lrM= 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 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? > > [...] > > > > > +void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags) > > > > > { > > > > > - void *newp; > > > > > + void *n; > > > > > + > > > > > > > > if (!size && p) { > > > > kvfree(p); > > > > return NULL; > > > > } > > > > > > > > would make this code flow slightly easier to read because the freeing > > > > path would be shared for all compbinations IMO. > > > > > > Personally, I like it without. For me the simplicity comes from directing things > > > to either krealloc() or vrealloc(). But I'd be open to change it however. > > > > I would really prefer to have it there because it makes the follow up > > code easier. > > I don't think it does (see below). > > Either way, I got notified that Andrew applied the patches to mm-unstable. How > to proceed from there for further changes, if any? Andrew will either apply follow up fixes are replace the series by a new version. > > > > > > > + 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); krealloc_noprof should be extended for the maximum allowed size and so does vrealloc_noprof. The implementation of the kvrealloc cannot get any easier and more straightforward AFAICS. See my point? -- Michal Hocko SUSE Labs