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 85680C5479A for ; Wed, 28 Aug 2024 09:52:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DC7D76B008A; Wed, 28 Aug 2024 05:52:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D780B6B008C; Wed, 28 Aug 2024 05:52:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C3F286B0099; Wed, 28 Aug 2024 05:52:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A50166B008A for ; Wed, 28 Aug 2024 05:52:28 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id F3006161D93 for ; Wed, 28 Aug 2024 09:52:27 +0000 (UTC) X-FDA: 82501189134.03.F926C0C Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf25.hostedemail.com (Postfix) with ESMTP id 6A40BA0008 for ; Wed, 28 Aug 2024 09:52:25 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=C6pFQw9F; spf=pass (imf25.hostedemail.com: domain of brauner@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724838659; 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=ob7Yl6HUnZFMtvuNRbRuoM4YIOb5cp49lnqu4BNDZac=; b=s2QdI0d9u0INsCqog91/fi9swPEwLpkBbmL16oT0b1hRt7KmHozpjtRImjywL1SLzJNzrZ 3wM5c9XswIL3eeUmCzx6kMIo0y2/mFxJF6I/UiE8zoYHrtb+aoH5edBik1cCEpDQpLfZBf 6zttlNujTB0RGoJ3IpMCkHawE9kP230= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724838659; a=rsa-sha256; cv=none; b=ox0mwT0glL1uNp7SswV6+mhxHcAKa20J7fjEC861POZV2lvpE5axGAgbkNsvctVS/sOeeC d9NUVFEs8NEYc7L1j/l82uK8z4JU+l9jc+zN9QoN00mnAdwvJz3OITIjs2i1EqRXQTRhDE lDBsKzepsmlgP5VzZR+iisL51QFfg/8= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=C6pFQw9F; spf=pass (imf25.hostedemail.com: domain of brauner@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 808FDA434BF; Wed, 28 Aug 2024 09:52:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 22BC5C8DC25; Wed, 28 Aug 2024 09:47:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1724838440; bh=8e9pG6U+yU2XijdDDN5D22TTXrNVdvVl3m3ZxA2sVss=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C6pFQw9FF3b1cP9Y8OcFlf4gPzS/9ZSEV3uJ+vrpzWSQLgrtJCOQo3Y70m3i27vOR b2Ese0kc8iwIDhCcoWkW9V63GNlMWS9OiLJHbqRkCzDEMklkw2uMT+pwhwD9Omxk2S aQe1QPXooXeCyNF6roqh5wciXUbOnQkWVxC6tWsCmOtIzZmwi5QCo2qfhlPznOtqgt KCkBgGB6DnWnik4gKxGNJEBHDLHF8YTcJjvMpl7UqKq5Vko6nSKhiLnuPOp+G2xKIz V739bTsye85ZOLFr+F2YsrozBPPMCmF+PPYX2hjMQ6b4gq9Iww5u2oY/SJyMBLIeUG brUK0OhKGtsgA== Date: Wed, 28 Aug 2024 11:47:15 +0200 From: Christian Brauner To: Vlastimil Babka Cc: Jens Axboe , "Paul E. McKenney" , Roman Gushchin , Jann Horn , Linus Torvalds , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 2/3] mm: add kmem_cache_create_rcu() Message-ID: <20240828-blicken-zeiteinheit-bbbe9724f1ea@brauner> References: <20240827-work-kmem_cache-rcu-v2-0-7bc9c90d5eef@kernel.org> <20240827-work-kmem_cache-rcu-v2-2-7bc9c90d5eef@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 6A40BA0008 X-Stat-Signature: 5utmn75443gsgxzdxzjofkm36ab759rf X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1724838745-16565 X-HE-Meta: U2FsdGVkX1/s/fG0KV/OgT2BcqQtdihfS+VahHHAWy2pF5SuUSNVbIKbcw3jBZILHK3NaaQv4sSpezjMNKAmwrqhGFQUTREwfCBvbq/A2hzqwX8R6DiMk3IEzENOcmJC9XPmKIx2fUvGko3yU9NBAFnWgzkEXIgr9ThAcIjVVX6Aly0hgnKeD9PdUwB7YFPr4mczyKLdf55Bw+8wCRjIRionau6S7AcHNbvsIirpWMcfsitPHgAK2qDZ2Yhusaa7fF0n9YOsB5+y3ggpa+dk1fOjGPkEk1/k6fOxiLCqjQXBg359wtsnppqh6OmxrKWnp/zp4/c3592zMdDT6gugCtsK5d5835BGB3GSwlQmwv8HxhkQJnfs56J6P56MEFsuFtP77H9KGwrK4/YVOI8hHTHfoUDT+nUbBenWGMBH9+JU7zArLAbezcM/uipci22JhsO7mUpha25/So6DIEIQTITmIwTl6A4LXGp/xWqCH+90WPB5MGOTOX/jt6D6WpORu3Pf2PM97lwZ17j0R3nlGvo7xu956suw09oPDaYla4I5Nn3Kv22ZNVvmLR+SgQ+jJ2TUJX5XbpZ2bx95MxnfOhtz0bibMqB2uKuTJK1mAjVLH3Q5fHMRXaiv1PjB9uIBFlw3L4kzW0kgbIKf1rqEELVyKLAdmq8lseJux/KC/trUh6PPQyLvQobYfsxtkqxoLfPSThnkNejYQh+mU/+duTlT4mebMoaSIMt/aStpC+vDDSJ887t6PNDC3bxjw87BDBQmGR1VG2OwvpGsMbrM9/+76oH1Tzq2CGKrHp0tPL/LzXRgRbnYrW+dyg13XgQo1uP8BHw7Z4Dd/eMSazLTCKpSkEb9TWr4I2pgw6GVU2VMCd6gZyViGt7+530buQ8+594dhW4rZHp/YhuR7hpj5KwbmYpKTHWPGkYjmS2bRMDVQfKurKNhKcLj1fsmbr3ScX5OC2EZen+DTSvjoDC VE/BIkJB 9j1FYJ/NVEx9/ycuOPrqxNtXC8F9bbw9oYzC1UnITn2TTTOUikRmCRyLqkabsPx3cmm71Ja3tKkdmerhrbNiRXMwjR0WttfLVPzlC0YKvot30cqjOJVgsbFIT8b1M1UBSMFeooe/MDlnWivXudnn8Z2qhLZrBrGxuXmI+1a3jfeThrd0AM+ayKZNlmnWoymTIdguI6b6mgQu37dDQhD614SRkAX3XWf4kI7D5yD1WHCzKRk8/0KWpTtyOeCLObGtBVU73CgM7OgWUUqbGJMwFsoUO3eyLEkSOANFP3/6M6OoAB5s0ERaRj5peC1hi+mpUjwvp6UsmVP5c/u04M2ITibt9lzOkKrsjwguMxKXnbuCrpaJWjwZw76X+HmWdwsDF1PKOVo63/dXpRU37WBxURRjl/n4C6nSuofS+h1IU0Es/w+s= 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, Aug 27, 2024 at 11:10:10PM GMT, Vlastimil Babka wrote: > On 8/27/24 17:59, Christian Brauner wrote: > > When a kmem cache is created with SLAB_TYPESAFE_BY_RCU the free pointer > > must be located outside of the object because we don't know what part of > > the memory can safely be overwritten as it may be needed to prevent > > object recycling. > > > > That has the consequence that SLAB_TYPESAFE_BY_RCU may end up adding a > > new cacheline. This is the case for .e.g, struct file. After having it > > shrunk down by 40 bytes and having it fit in three cachelines we still > > have SLAB_TYPESAFE_BY_RCU adding a fourth cacheline because it needs to > > accomodate the free pointer and is hardware cacheline aligned. > > > > I tried to find ways to rectify this as struct file is pretty much > > everywhere and having it use less memory is a good thing. So here's a > > proposal. > > > > Signed-off-by: Christian Brauner > > So logistically patch 3 needs stuff in the vfs tree and having 1+2 in slab > tree and 3 in vfs that depends on 1+2 elsewhere is infeasible, so it will be > easiest for whole series to be in vfs, right? Yeah, that's fine by me. > > > --- > > include/linux/slab.h | 9 ++++ > > mm/slab.h | 1 + > > mm/slab_common.c | 133 ++++++++++++++++++++++++++++++++++++--------------- > > mm/slub.c | 17 ++++--- > > 4 files changed, 114 insertions(+), 46 deletions(-) > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index eb2bf4629157..5b2da2cf31a8 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -212,6 +212,12 @@ enum _slab_flag_bits { > > #define SLAB_NO_OBJ_EXT __SLAB_FLAG_UNUSED > > #endif > > > > +/* > > + * freeptr_t represents a SLUB freelist pointer, which might be encoded > > + * and not dereferenceable if CONFIG_SLAB_FREELIST_HARDENED is enabled. > > + */ > > +typedef struct { unsigned long v; } freeptr_t; > > + > > /* > > * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests. > > * > > @@ -242,6 +248,9 @@ struct kmem_cache *kmem_cache_create_usercopy(const char *name, > > slab_flags_t flags, > > unsigned int useroffset, unsigned int usersize, > > void (*ctor)(void *)); > > +struct kmem_cache *kmem_cache_create_rcu(const char *name, unsigned int size, > > + unsigned int freeptr_offset, > > + slab_flags_t flags); > > void kmem_cache_destroy(struct kmem_cache *s); > > int kmem_cache_shrink(struct kmem_cache *s); > > > > diff --git a/mm/slab.h b/mm/slab.h > > index dcdb56b8e7f5..b05512a14f07 100644 > > --- a/mm/slab.h > > +++ b/mm/slab.h > > @@ -261,6 +261,7 @@ struct kmem_cache { > > unsigned int object_size; /* Object size without metadata */ > > struct reciprocal_value reciprocal_size; > > unsigned int offset; /* Free pointer offset */ > > + unsigned int rcu_freeptr_offset; /* Specific free pointer requested */ > > More precisely something like: > > Specific offset requested (if not > UINT_MAX) Yep, added that. > > ? > > > #ifdef CONFIG_SLUB_CPU_PARTIAL > > /* Number of per cpu partial objects to keep around */ > > unsigned int cpu_partial; > > diff --git a/mm/slab_common.c b/mm/slab_common.c > > index c8dd7e08c5f6..c4beff642fff 100644 > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -202,9 +202,10 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align, > > } > > > > static struct kmem_cache *create_cache(const char *name, > > - unsigned int object_size, unsigned int align, > > - slab_flags_t flags, unsigned int useroffset, > > - unsigned int usersize, void (*ctor)(void *)) > > + unsigned int object_size, unsigned int freeptr_offset, > > + unsigned int align, slab_flags_t flags, > > + unsigned int useroffset, unsigned int usersize, > > + void (*ctor)(void *)) > > { > > struct kmem_cache *s; > > int err; > > @@ -212,6 +213,12 @@ static struct kmem_cache *create_cache(const char *name, > > if (WARN_ON(useroffset + usersize > object_size)) > > useroffset = usersize = 0; > > > > + err = -EINVAL; > > + if (freeptr_offset < UINT_MAX && > > freeptr_offset != UINT_MAX to be more obvious and match has_freeptr_offset() ? Done. > > > + (freeptr_offset >= object_size || > > + (freeptr_offset && !(flags & SLAB_TYPESAFE_BY_RCU)))) > > and here drop the "freeptr_offset &&" as zero is a valid value Yes, thank you. > > instead we could want alignment to sizeof(freeptr_t) if we were paranoid? Added a check for that. > > > + goto out; > > The rest seems good to me now. Thanks for the review! v3 incoming