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 83F3AC54EE9 for ; Sun, 4 Sep 2022 09:03:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 797AB8D0033; Sun, 4 Sep 2022 05:03:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 748F68D0031; Sun, 4 Sep 2022 05:03:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 60E568D0033; Sun, 4 Sep 2022 05:03:44 -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 533478D0031 for ; Sun, 4 Sep 2022 05:03:44 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2B2571201C2 for ; Sun, 4 Sep 2022 09:03:44 +0000 (UTC) X-FDA: 79873815168.29.EB04333 Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by imf18.hostedemail.com (Postfix) with ESMTP id DDE921C0056 for ; Sun, 4 Sep 2022 09:03:42 +0000 (UTC) Received: by mail-pj1-f52.google.com with SMTP id j9-20020a17090a3e0900b001fd9568b117so5981585pjc.3 for ; Sun, 04 Sep 2022 02:03:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=/nOdZT0dya6kDHenQdU2BmxT3CFwwxmGevkfqFbAuFY=; b=DjQylxg2aCtbrA/Enu4bJ9WBUJ37jX/G4ldPk0TEIGmrJzSn/zLiF+TBkfaa+cbrX5 3rHQlye8TJ/GDmM4KVl3TCsFEEU/iFzXdTPBT0b5DvP7KbLGgJHVIjqeOvmSsslqOqAm Uo6DbAvaOyotLi2NAzyU42v8UyvuhP45zMHR09YVD+ISWm6pkUF9y//qdH0VDKUdBEuY ha9qmcpYaf3JZpVs/GCF0jTUMMQzngK8O2+fppzsCsGhyoEkY4XjtM6ZC8J9Im4tKuYf AjV5Yw4CeyR0jWCAuGEUnGKVqZhMAR/B721f5KUA9PNJFUx7sPeR3qe2zDNuchO3xwjI lQBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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; bh=/nOdZT0dya6kDHenQdU2BmxT3CFwwxmGevkfqFbAuFY=; b=hJE/vGi1+MfFIC3qVxp1wdarpfFjk0QRmtSG+0cTkLEIIuRKMHART1CYBJVhUPXjeO zhRr5dihAsqebEEDIqIWs5vuzrCy26ZIoGmxAA2iLd1T/G9tegdUQ1ATwRXkxDX98En+ nW0DvT7yd0e+rexz+VkYfCm1gQLvjHOWwKBHlOJEvfj5DvL1srWP2CvFCsqL0B8wwM0K 5hJoq8R87XgPUjaWdiXDt5/+NHbUn80ZqqfOM9FHQFNhKmnNvRlD21uI9+UnoQGU03h1 iZS8UvxZl/9q+cI/6CFusSVSCLR/ps9ZSDh1UZ9ShR6oEkD4aETZSPIeSzCQDBNnEhOM 83zQ== X-Gm-Message-State: ACgBeo27gR2/pCbIc7j1jWmhzh3ieo0rXpRZCOL+6GaQzj+A50hvjuRa TCe9GlQUrpPqyil+Oj1nvho= X-Google-Smtp-Source: AA6agR4HqDNxujARcBBMEzZncfiL/3fOmXb4uuIXGVv8HCNSZLvLPUW0S8S4ZdHoKEUeakdbt/4WqQ== X-Received: by 2002:a17:90a:cb07:b0:1fd:f273:dd90 with SMTP id z7-20020a17090acb0700b001fdf273dd90mr13777483pjt.95.1662282221574; Sun, 04 Sep 2022 02:03:41 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id m15-20020a17090a34cf00b001fb35e4044bsm8191706pjf.40.2022.09.04.02.03.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 04 Sep 2022 02:03:40 -0700 (PDT) Date: Sun, 4 Sep 2022 18:03:34 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Feng Tang Cc: Andrew Morton , Vlastimil Babka , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Dmitry Vyukov , "Hansen, Dave" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , Robin Murphy , John Garry , Kefeng Wang Subject: Re: [PATCH v4 1/4] mm/slub: enable debugging memory wasting of kmalloc Message-ID: References: <20220829075618.69069-1-feng.tang@intel.com> <20220829075618.69069-2-feng.tang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1662282222; a=rsa-sha256; cv=none; b=kfFj2zPumX2YHq9aT9hQvUb1ASGNu0nj0q9S+K46obnhqvCSQiJQO9Xgh7QYwvE8R9wzkM UD9Mfbu9bDu9m90oPLele770OoVZC6OVzRzH9fMFmZvejDDuZG4b1ut5iI/bvZGXh60P5n afv4qsfIdMbulg9oOjE7uFa9h93pi9k= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DjQylxg2; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1662282222; 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=/nOdZT0dya6kDHenQdU2BmxT3CFwwxmGevkfqFbAuFY=; b=HzzhtwfFnljFK9BpXcAtL+/jRp4+yu30jwMn1bZlctWhO/rZPGIjdwB3n3CzlD4nAxe1mR 0N/5Buxj77hDZ5lxf2wKpkuNEsIGPtfwsOVizZ8B4Z47fa153JpwtzrKo9NYsLNPVBii5n 8QAak7Op2HaG9ZpRxY3zWACh9aiUarQ= X-Stat-Signature: xgftimi8mpqmwjf1tgn8yjfhkm7qkw6y X-Rspamd-Queue-Id: DDE921C0056 Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=DjQylxg2; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com X-Rspam-User: X-Rspamd-Server: rspam02 X-HE-Tag: 1662282222-55256 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: On Fri, Sep 02, 2022 at 02:15:45PM +0800, Feng Tang wrote: > On Thu, Sep 01, 2022 at 10:01:13PM +0800, Hyeonggon Yoo wrote: > > On Mon, Aug 29, 2022 at 03:56:15PM +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. > > > [...] > > > > > > Signed-off-by: Feng Tang > > > Cc: Robin Murphy > > > Cc: John Garry > > > Cc: Kefeng Wang > > > --- > > > include/linux/slab.h | 2 + > > > mm/slub.c | 94 +++++++++++++++++++++++++++++++++++++------- > > > 2 files changed, 81 insertions(+), 15 deletions(-) > > > > > > Would you update Documentation/mm/slub.rst as well? > > (alloc_traces part) > > Sure, will do. > > > [...] > > > > > */ > > > static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, > > > - unsigned long addr, struct kmem_cache_cpu *c) > > > + unsigned long addr, struct kmem_cache_cpu *c, unsigned int orig_size) > > > { > > > void *freelist; > > > struct slab *slab; > > > @@ -3115,6 +3158,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, > > > > > > if (s->flags & SLAB_STORE_USER) > > > set_track(s, freelist, TRACK_ALLOC, addr); > > > + set_orig_size(s, freelist, orig_size); > > > > > > return freelist; > > > } > > > @@ -3140,6 +3184,8 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, > > > */ > > > if (s->flags & SLAB_STORE_USER) > > > set_track(s, freelist, TRACK_ALLOC, addr); > > > + set_orig_size(s, freelist, orig_size); > > > + > > > return freelist; > > > } > > > > > > This patch is okay but with patch 4, init_object() initializes redzone/poison area > > using s->object_size, and init_kmalloc_object() fixes redzone/poison area using orig_size. > > Why not do it in init_object() in the first time? > > > > Also, updating redzone/poison area after alloc_single_from_new_slab() > > (outside list_lock, after adding slab to list) will introduce races with validation. > > > > So I think doing set_orig_size()/init_kmalloc_object() in alloc_debug_processing() would make more sense. > > Yes, this makes sense, and in v3, kmalloc redzone/poison setup was > done in alloc_debug_processing() (through init_object()). When > rebasing to v4, I met the classical problem: how to pass 'orig_size' > parameter :) > > In latest 'for-next' branch, one call path for alloc_debug_processing() > is > ___slab_alloc > get_partial > get_any_partial > get_partial_node > alloc_debug_processing > > Adding 'orig_size' paramter to all these function looks horrible, and > I couldn't figure out a good way and chosed to put those ops after > 'set_track()' IMO adding a parameter to them isn't too horrible... I don't see better solution than adding a parameter with current implementation. (Yeah, the code is quite complicated...) It won't affect performance to meaningful degree as most of allocations will be served from cpu slab or percpu partial list. -- Thanks, Hyeonggon