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 24D91C3DA63 for ; Tue, 23 Jul 2024 07:50:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3C9EE6B008A; Tue, 23 Jul 2024 03:50:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 304536B0096; Tue, 23 Jul 2024 03:50:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 157816B0098; Tue, 23 Jul 2024 03:50:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id E61E16B008A for ; Tue, 23 Jul 2024 03:50:18 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 468AD161C0B for ; Tue, 23 Jul 2024 07:50:18 +0000 (UTC) X-FDA: 82370244516.16.9F1AEFA Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf08.hostedemail.com (Postfix) with ESMTP id 4123616001E for ; Tue, 23 Jul 2024 07:50:16 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="f//zekB6"; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.49 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=1721720993; 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=AHbbiSpbwBUG2vvtlGIUjQprGr8z88fyXfa4wfrh8N0=; b=27hNIDygTFe7CWDkt3kFx6Q0sH6+Hwkq9hAS9kXOA1gy9ZjBUECLeJrEfSA7qNM2VPQGc4 5Ncmk59Y0Qc0RZAC7eYjuFPmvSRdzbLiABWgqzIA5w81vc+8NKiJ4riRMrPxfWgwux+XA+ R4tcOHQLZXAKW+QxZm4yTDwa8EpF2i4= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="f//zekB6"; spf=pass (imf08.hostedemail.com: domain of mhocko@suse.com designates 209.85.128.49 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=1721720993; a=rsa-sha256; cv=none; b=cKQOxgA9HzDbHA7FVWfQOXHVqprJGElvXznqZR9QJgHYyLvwOp0CI0fuLYmvaHejucdais Vb3XLehsfK9sWrjUtf3wdy2RbRqA4smsB3bsYlPqK4xk/s1shLQyGhRrOzNLS8fuYjOKKt WC+xSE4Ym+3GSW3SwAETStzYo89ds78= Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-42793fc0a6dso37027625e9.0 for ; Tue, 23 Jul 2024 00:50:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1721721015; x=1722325815; 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=AHbbiSpbwBUG2vvtlGIUjQprGr8z88fyXfa4wfrh8N0=; b=f//zekB6jQhDIBuMvhDTruQXKlFvuqFgCqrKOrPErcKelJr0eJbC6Dv08UCITb0KZc ui6Vjm2dJr9nOEoIe+cDavj+FD63c6r8SETgkR+TFXVZsxzDjSYukENEkLZ9W4n5kAj2 KdWRsi39kf6iHU5TqDBVwnLdibFKTHChxgcR+cycra0rq4h46HcNGybKwefY6MNSFzfl 7yvg8TsIi3sD0UGaYdE+dLakD1Dl6XEjzPmmV2G0AvkgL4ftnknaQJXa+6h32Gm7UgyB J3wGg5qMU/CY9QycJgklCfKSgDsTWLgDMrv1VJ2e8jd1GlOk1Cy4KZim6hesxZWw9ux2 CEuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721721015; x=1722325815; 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=AHbbiSpbwBUG2vvtlGIUjQprGr8z88fyXfa4wfrh8N0=; b=udVm+QqLAOGMntpxw9Lzg1lWTLwDbHLU6Tf+N+6hw68ObZNT4sgGBUYwL3+Cq+fYWW 1/ETcOy4QSdFt272RiNyqJWK8m8MKktir5DgqT2jCTfJo7TpEbJHATiw3RUx4o4ivCBt 1UB7hJLXhMu4Uv8/sCDZxV0YFXotGm6ERQ9icvEfjxZzG+7eTfBCUmFSs6z5f1DcE9ei P/s9oz2kxWXIo0yQzJ40Nj7lDbjQHQQtMkagEfLmfaa9bkIGyo1rG7FdX3ctx2EKCFqX AgiRZrHsJ2o4zZlQcc7CTxuhEwK0wD9jLSKMpkdEXNRWPoQgxPJqqV+atCktEzvjMuLt wIoQ== X-Forwarded-Encrypted: i=1; AJvYcCVliGeTeqoX4JWxaqyWwbetyNLVd0Znq1dlDncgD9t7U9uDgGLNcqEYuxt8hE+0sRwlF/83Fp80WOTLfUn/1F9Np8U= X-Gm-Message-State: AOJu0Yw0C4waZVgLvDExkV3vriblWOhh9pHqP6cYn6ZDpGoyXtP9jnN+ NXxdh8N4KTQNArm7d8DLdUUaQLlC0x5CUospZrvsHJd03pIlRYO5w+PFecz9KlY= X-Google-Smtp-Source: AGHT+IFEa0dYyhrMKUpXQUIthirOp4aQ1GbTQq/jTNpcnXJtqU5/ALKEJJJtG0qMv9pXImiM03usdA== X-Received: by 2002:a05:600c:3ba4:b0:426:5dd0:a1ee with SMTP id 5b1f17b1804b1-427ecfce9c9mr14968095e9.2.1721721014618; Tue, 23 Jul 2024 00:50:14 -0700 (PDT) Received: from localhost (109-81-94-157.rct.o2.cz. [109.81.94.157]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-427d2a9466bsm187338875e9.41.2024.07.23.00.50.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jul 2024 00:50:14 -0700 (PDT) Date: Tue, 23 Jul 2024 09:50:13 +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: <20240722163111.4766-3-dakr@kernel.org> X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 4123616001E X-Stat-Signature: jt7d77boar564mo4kd8aq7ngkq48uh1f X-Rspam-User: X-HE-Tag: 1721721016-868743 X-HE-Meta: U2FsdGVkX18ExLpiCRipKcBGBTGXatG+E+kQ/GlFpagD/XnbGgDDKJnL0Y2HhZdlYNX7G4iTJvrAWd5h95Clr/ekn5wjrEMCDrlqATIC88MsIYy0gqe/kkk838e2CtNNpqzGC8UnnvRk/9PxyFrLuPqAGHX9siaoeFAfh+nZFhTEVqtmHe7Iu+4xeUtUz4D/CXYLk3Px5k4erzk0dyiKcI/K05oSq9btVYEr3RwDEU2WZHgaVD13P1gjhvCuHqhg30wrvomAKOYWDK8VvuOphqkNf5QKmXq66dC3gIKFJIb0hQcyD1JoZfvQayKv7y08O5bLQVyqFWJD3xW9voL6BxyDEfgnJX0tyUxEoiUS1gzMs5I1ugKEd6eyAd0KoZHffQbqoMlSmAZ2FPkay2b/KTbU9mjzjrqiNUosiLvW4KLCm66QWDOHWjRturLNPf4z+T/6yfBw7hIfGLJme4Rizri0H9NYFQBTqVdkh3WPFdQuwoImBZH5EkOLRoGVudTgwUThXvjwUbpnYakTn+Fsz5dfe3HTxiTmPUb0aUmAf3ZDu5tyUxaoyrsm6LlUu765ny2q6T/N5h9AjnD8nCNdzRqIeo6kdrbyJjU75TK8L7jLZthv132GYtnbS/CJXwsWO3Z2Hzr8ZEiJEI1YMVOHUMZI67KYsmyS0p1ENnJ3eNwzUonCAKgoy//+/7KYCv4NeQx7Mb1kKCWJm+yjub1d+9n0CC7TgVzqZpVf4qIE48qz11Ld7Fbjks/guEO+kbmi/z3dwyQ+KyTWYsv+ODuCeXI7Kk75KkzhcmZa545FGllB9fjIL9JDet+5JqSqVZh4n8xAPg+79KvOsOYHVM2PDhuhFsguMozWLRxCdr3166K3jQnA6AjNR+x7fHSXh+lIYr3UaKkbTrkCc1fwLES3LwJptwOPGd2rcbBGpAkSKJzrohCS2tkc0oVh1b65RO5PDs8Ti5J/RPsyjW00PyU Bld1/dL1 w+M2jlaXsDzgxCn3+cEmnvYrMAfcS/KJQOnYFexMrU8cmnYGdv3y/mZupFJuh7fm8Z4j0BKDdMcRa76cZ2lqDjK+Aio6WydwuJbvlOOu0w70dxFOPp0Spjheju1iM0E6r6UntOuwneB4+YgAO6Ge+VihEqHy2e/P/R6PGmJy7A1zAL07nT9UjkmmVDCxuWRjKOWEi8kX0hJnO/03RC2rSMfLwBvJp5NDSZhye9rL1sopQ9yldjnRd/xKh04nfuKA5jBXAlgrJt6bX57cWeiNWzZU5nfFMeUQu+bmCYruD+XRSqFUU+UbNI5ttq++gj5ytExUA1FRDlnJRKEZ1TE01cXztgGAjE8/lbpsca2EeqAMU/b0= 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 Mon 22-07-24 18:29:24, Danilo Krummrich wrote: > Besides the obvious (and desired) difference between krealloc() and > kvrealloc(), there is some inconsistency in their function signatures > and behavior: > > - krealloc() frees the memory when the requested size is zero, whereas > kvrealloc() simply returns a pointer to the existing allocation. > > - krealloc() behaves like kmalloc() if a NULL pointer is passed, whereas > kvrealloc() does not accept a NULL pointer at all and, if passed, > would fault instead. > > - krealloc() is self-contained, whereas kvrealloc() relies on the caller > to provide the size of the previous allocation. > > Inconsistent behavior throughout allocation APIs is error prone, hence make > kvrealloc() behave like krealloc(), which seems superior in all mentioned > aspects. I completely agree with this. Fortunately the number of existing callers is small and none of them really seem to depend on the current behavior in that aspect. > 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. [...] > diff --git a/mm/util.c b/mm/util.c > index bc488f0121a7..0ff5898cc6de 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -608,6 +608,28 @@ unsigned long vm_mmap(struct file *file, unsigned long addr, > } > EXPORT_SYMBOL(vm_mmap); > > +static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size) This seems like a generally useful helper which it is not. I would call it something like __kvmalloc_gfp_adjust or something similar so that it is clear that this is just a helper to adjust gfp flag for slab allocator path [...] > -void *kvrealloc_noprof(const void *p, size_t oldsize, size_t newsize, gfp_t flags) > +/** > + * kvrealloc - reallocate memory; contents remain unchanged > + * @p: object to reallocate memory for > + * @size: the size to reallocate > + * @flags: the flags for the page level allocator > + * > + * The contents of the object pointed to are preserved up to the lesser of the > + * new and old size (__GFP_ZERO flag is effectively ignored). > + * > + * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0 > + * and @p is not a %NULL pointer, the object pointed to is freed. > + * > + * Return: pointer to the allocated memory or %NULL in case of error > + */ > +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. > + 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? > + if (!n) > + return NULL; > + > + if (p) { > + /* We already know that `p` is not a vmalloc address. */ > + memcpy(n, p, ksize(p)); > + kfree(p); > + } > + } > > - if (oldsize >= newsize) > - return (void *)p; > - newp = kvmalloc_noprof(newsize, flags); > - if (!newp) > - return NULL; > - memcpy(newp, p, oldsize); > - kvfree(p); > - return newp; > + return n; > } > EXPORT_SYMBOL(kvrealloc_noprof); > > -- > 2.45.2 -- Michal Hocko SUSE Labs