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:   Thu, 8 Sep 2022 10:25:40 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Hyeonggon Yoo <42.hyeyoo@gmail.com>
CC:     Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        "Dmitry Vyukov" <dvyukov@google.com>,
        Jonathan Corbet <corbet@lwn.net>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>,
        Robin Murphy <robin.murphy@arm.com>,
        John Garry <john.garry@huawei.com>,
        Kefeng Wang <wangkefeng.wang@huawei.com>
Subject: Re: [PATCH v5 1/4] mm/slub: enable debugging memory wasting of
 kmalloc
Message-ID: <YxlSpK4Jo97g1OB/@feng-clx>
References: <20220907071023.3838692-1-feng.tang@intel.com>
 <20220907071023.3838692-2-feng.tang@intel.com>
 <Yxin8k0BFijbkGK0@hyeyoo>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Yxin8k0BFijbkGK0@hyeyoo>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?C+2CcWo/0lvAABD7TZ3BAmLmylL3/DkMcK60quFWXCvfPP9StjGKkl3CmafU?=
 =?us-ascii?Q?oEjNQdcL9GOJPXOl+WEe4TtRGmYWmUMeHzTHQ4aYU99RbzMM8gfAgsG+NDTT?=
 =?us-ascii?Q?1BNQUinyq7hse0E+bFRUqaseG0N39d2ThCH28Rr2bzfvWdooSIVcNrvIsjO5?=
 =?us-ascii?Q?SI9S7Pl01km7hL/4JD9i+qYkefd9JWdR+EmZhNHvQrKF+/Img7/cqd+EgcdM?=
 =?us-ascii?Q?/TA8M+4IAul8UCNmaKZxuOdXvabbEbO3+9vQCgAI4IBtACzksq1f9F1Qmmhp?=
 =?us-ascii?Q?2EgSYjySo11h34UwePfvNyfEElMdHvI7EcomkIc9+GAv0b88aGj0clvPIctN?=
 =?us-ascii?Q?LavbevK68aeseZij81pi7bH71ggF0QNEKfn/w5/TV4ptNaISzt6ztAtPlHtY?=
 =?us-ascii?Q?8rPV2XQpb1GKyN0+AI/S37YizuHGiUIY1CcF3tB9b+RqB7GjEehxkg8Piub5?=
 =?us-ascii?Q?f0sekP1Z3v25A9ONyN8SxdOOcKwNSZPBU70ouHwAkCvosu11HYT6cdCbsr2f?=
 =?us-ascii?Q?ZwIeXAzFWeToHb5QTEBaeQaEDdPAvCkWzzCt1Aa1whHLkCTlLgIO0mut7kvq?=
 =?us-ascii?Q?ep60VARV/C6VdF6Z13NHMQkDVgvXTOosUK8jdGpKw11kPahHDvizqU95efzV?=
 =?us-ascii?Q?ph2tcWMEEFQeHiUfKP8QSYBhW4mdZXXXMDx7q7jZa92N9IfJiO7SIROJjODn?=
 =?us-ascii?Q?jJgAkxRzNQIeEgJER5ci/FaJZMVIkc/kzkF2JfI3q8pSs/tCOdmqz2lpGe6K?=
 =?us-ascii?Q?aylcrPqi9V90RiLMwr9VLFz1BRuHtOmZY4IGpDVO7i6PGlubmKGQRc2VOEuf?=
 =?us-ascii?Q?Bcs4iCvA+IY89suQu1Pd9jKyvH2x8VlferG/jbDelVbu0LOoEqw1nabaKozn?=
 =?us-ascii?Q?TvKrumQkmEXZs4w3Z/r8mTkqap0jQsuaFsp9Vd+3C4LeO9EKZKIxgLayTRoq?=
 =?us-ascii?Q?hjNFGK1H96m9fxi177cjO6PbgCpfEop+NoxZP7pUY5EtoWrC9oIOENw2N5cL?=
 =?us-ascii?Q?WH6/v8tmS0nSqC7QTwVm3p8A1wjoAqpiLlhw4hmn8bAc1fw7DHIVWF5R/K8D?=
 =?us-ascii?Q?WFHJlkEzarrMPfThD5bA6uMYr6qtL6B+YHU16esoQ5UCPaHsiQO/npRkjyWf?=
 =?us-ascii?Q?hbDI3xsyU7ZEtyODCZ4Mws+q1B5D7XydL+sA3tuA9a3Xl14p2bh6N8SpH1Td?=
 =?us-ascii?Q?PSCWiVv+a9vACXpgwgNVvHoZq8CzqkPQRLQhzWtp3fwR3M3w3ucrL015RgoE?=
 =?us-ascii?Q?eBtt7xMKKC4MwWWVg842I+U+vxwAEuMx5hQiNMUXS+pnaLj/5Xf12u9de3P8?=
 =?us-ascii?Q?Jl73ldzwRn0zh4CWt+eym2fD1Z+U/6PWLSIBTOkopF6u1XaGqReyEG1gxmRq?=
 =?us-ascii?Q?lO7w5bzhi+Eq1SwkWsPVvzc5LAjRbQhUNt1enCgcYCjiNrnpW1+gCE7JUEaE?=
 =?us-ascii?Q?tysX7P8Qc+Kta57PB6QUccew8R7L65PakZgHNqReFkCQ8DPOF5YRMtfbDzmY?=
 =?us-ascii?Q?8rJRPGQwo4Xdn/Lh52m/3qG6zgbtNAqJcGEtsqFyH3B4kfSeri5UCJvP1mVf?=
 =?us-ascii?Q?IVcK9YD3Yi7DnlSzIayoc2/AAXTxzGIhb3efa0Ux?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 651d7945-719a-47a2-8cb3-08da91418308
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2022 02:26:18.4873
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Xg6P0fk5DdUoVB1ddMNO1O0w81jGvSv3WZt/kt9HsaX4E4bTQ5/LKB7il08serrfPG87+EYCQTuRaYB2yAOiTA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1101MB2098
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE 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

On Wed, Sep 07, 2022 at 10:17:22PM +0800, Hyeonggon Yoo wrote:
> On Wed, Sep 07, 2022 at 03:10:20PM +0800, Feng Tang wrote:
> > kmalloc's API family is critical for mm, with one nature that it will
> > round up the request size to a fixed one (mostly power of 2). Say
> > when user requests memory for '2^n + 1' bytes, actually 2^(n+1) bytes
> > could be allocated, so in worst case, there is around 50% memory
> > space waste.
> > 
> > The wastage is not a big issue for requests that get allocated/freed
> > quickly, but may cause problems with objects that have longer life
> > time.
> > 
> > We've met a kernel boot OOM panic (v5.10), and from the dumped slab
> > info:
> > 
> >     [   26.062145] kmalloc-2k            814056KB     814056KB
> > 
> > >From debug we found there are huge number of 'struct iova_magazine',
> > whose size is 1032 bytes (1024 + 8), so each allocation will waste
> > 1016 bytes. Though the issue was solved by giving the right (bigger)
> > size of RAM, it is still nice to optimize the size (either use a
> > kmalloc friendly size or create a dedicated slab for it).
> > 
> > And from lkml archive, there was another crash kernel OOM case [1]
> > back in 2019, which seems to be related with the similar slab waste
> > situation, as the log is similar:
> > 
> >     [    4.332648] iommu: Adding device 0000:20:02.0 to group 16
> >     [    4.338946] swapper/0 invoked oom-killer: gfp_mask=0x6040c0(GFP_KERNEL|__GFP_COMP), nodemask=(null), order=0, oom_score_adj=0
> >     ...
> >     [    4.857565] kmalloc-2048           59164KB      59164KB
> > 
> > The crash kernel only has 256M memory, and 59M is pretty big here.
> > (Note: the related code has been changed and optimised in recent
> > kernel [2], these logs are just picked to demo the problem, also
> > a patch changing its size to 1024 bytes has been merged)
> > 
> > So add an way to track each kmalloc's memory waste info, and
> > leverage the existing SLUB debug framework (specifically
> > SLUB_STORE_USER) to show its call stack of original allocation,
> > so that user can evaluate the waste situation, identify some hot
> > spots and optimize accordingly, for a better utilization of memory.
> > 
> > The waste info is integrated into existing interface:
> > '/sys/kernel/debug/slab/kmalloc-xx/alloc_traces', one example of
> > 'kmalloc-4k' after boot is:
> > 
> >  126 ixgbe_alloc_q_vector+0xbe/0x830 [ixgbe] waste=233856/1856 age=280763/281414/282065 pid=1330 cpus=32 nodes=1
> >      __kmem_cache_alloc_node+0x11f/0x4e0
> >      __kmalloc_node+0x4e/0x140
> >      ixgbe_alloc_q_vector+0xbe/0x830 [ixgbe]
> >      ixgbe_init_interrupt_scheme+0x2ae/0xc90 [ixgbe]
> >      ixgbe_probe+0x165f/0x1d20 [ixgbe]
> >      local_pci_probe+0x78/0xc0
> >      work_for_cpu_fn+0x26/0x40
> >      ...
> > 
> > which means in 'kmalloc-4k' slab, there are 126 requests of
> > 2240 bytes which got a 4KB space (wasting 1856 bytes each
> > and 233856 bytes in total), from ixgbe_alloc_q_vector().
> > 
> > And when system starts some real workload like multiple docker
> > instances, there could are more severe waste.
> > 
> > [1]. https://lkml.org/lkml/2019/8/12/266
> > [2]. https://lore.kernel.org/lkml/2920df89-9975-5785-f79b-257d3052dfaf@huawei.com/
> > 
> > [Thanks Hyeonggon for pointing out several bugs about sorting/format]
> > [Thanks Vlastimil for suggesting way to reduce memory usage of
> >  orig_size and keep it only for kmalloc objects]
> > 
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: John Garry <john.garry@huawei.com>
> > Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> > ---
> >  Documentation/mm/slub.rst |  33 +++++---
> >  include/linux/slab.h      |   2 +
> >  mm/slub.c                 | 156 ++++++++++++++++++++++++++++----------
> >  3 files changed, 141 insertions(+), 50 deletions(-)
> > 
> 
> Looks good to me.
> Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
 
Thank you!

> > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst
> 
> [...]
> 
> > +/* Structure holding parameters for get_partial() call chain */
> > +struct partial_context {
> > +	struct slab **slab;
> > +	gfp_t flags;
> > +	int orig_size;
> 
> Nit: unsigned int orig_size
 
Yes, will change. 'unsigned int' is more consistent with the orig_size saved
in meta data and others members size/object_size/inuse/offset of kmem_cache.

Thanks,
Feng

> Thanks!
>