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 D9A2EECAAD5 for ; Mon, 5 Sep 2022 06:29:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 58091801B0; Mon, 5 Sep 2022 02:29:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 509C58D0050; Mon, 5 Sep 2022 02:29:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3830A801B0; Mon, 5 Sep 2022 02:29:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 234428D0050 for ; Mon, 5 Sep 2022 02:29:54 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id ECD501C5C52 for ; Mon, 5 Sep 2022 06:29:53 +0000 (UTC) X-FDA: 79877056266.19.8D698E7 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf09.hostedemail.com (Postfix) with ESMTP id 6791F14007B for ; Mon, 5 Sep 2022 06:29:53 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 1726E5C0FF; Mon, 5 Sep 2022 06:29:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1662359392; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WhijfRZY40vjJfJyooNR8N9awwqjN0FM9rnSIBMt9qM=; b=XiBDhXtj8D9Xq/1iDTG64+f1FivzrJIciuouAcGpXbZRRXjh1w6nG5So+61oKVhokfTB+r m7RPRW95Tr6BKnYiHQ3liqzt67JA+nCpxZIfeWf5RyF+XOX6UTDp7PbfbAB6BEfr0G7Iad QcwXj6K3n8Inw8gNO8J6Ej8W13kp6SY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1662359392; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WhijfRZY40vjJfJyooNR8N9awwqjN0FM9rnSIBMt9qM=; b=af1PA2sGw+9FlXu0Q5xfOIZAWligcxM3LpUNzURuwWPx4WwAmOkykPLTMr0qNxjA95HLZh UcMGVph0UZ8jSjBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D571A139C7; Mon, 5 Sep 2022 06:29:51 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id FTd0M1+XFWNKWQAAMHmgww (envelope-from ); Mon, 05 Sep 2022 06:29:51 +0000 Message-ID: <8ff805f4-76ae-fc0f-424f-4d230c08285e@suse.cz> Date: Mon, 5 Sep 2022 08:29:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.0 Subject: Re: [PATCH v4 1/4] mm/slub: enable debugging memory wasting of kmalloc To: Feng Tang , Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: Andrew Morton , 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 References: <20220829075618.69069-1-feng.tang@intel.com> <20220829075618.69069-2-feng.tang@intel.com> Content-Language: en-US From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1662359393; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WhijfRZY40vjJfJyooNR8N9awwqjN0FM9rnSIBMt9qM=; b=6nkKhz2P5xBPqX8rxyYxx/1ihhpxqZ7UWkZSXwSgR9rTPoFg0jVhH/Ivu3ca42d3ZcfNQr vZ4B+yYCf9LMPwyg+QKCyWyA89awCswhMBaFUFm/tS4ffsJluWvPbFzS2JizXP6tf17mg/ Hd8nJuKHkoy/E0i2XOlOXJ7wd1yGHaU= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=XiBDhXtj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=af1PA2sG; spf=pass (imf09.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1662359393; a=rsa-sha256; cv=none; b=c4evlKjUC4INarQL6NdA5F3H9aQOcNV5OEFlFmkEhK9xxet/q+Abo0ZRG60I7PZCFFomBt 74lYUWgFdB67MrFolmlgJ6la6mKOWYdLhwMIi/jCClFnih82/Emg2Y8+Eb3QMRCgWa1AxQ UwbSdrbt2+bAllCOafC87sRl9dbxqlg= X-Rspam-User: X-Stat-Signature: kw45d618sa3cf95b1c5fd4urba3kbqwp X-Rspamd-Queue-Id: 6791F14007B Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=XiBDhXtj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=af1PA2sG; spf=pass (imf09.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none X-Rspamd-Server: rspam04 X-HE-Tag: 1662359393-134652 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 9/5/22 04:55, Feng Tang wrote: > On Sun, Sep 04, 2022 at 06:58:49PM +0800, Hyeonggon Yoo wrote: >> On Sun, Sep 04, 2022 at 05:42:33PM +0800, Feng Tang wrote: >> > On Sun, Sep 04, 2022 at 05:03:34PM +0800, Hyeonggon Yoo wrote: >> > [...] >> > > > > >> > > > > 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 for the suggestion! I'm fine with it and just afraid other >> > developers may dislike the extra parameter. >> > >> > The race condition you mentioned is a valid concern, and I have thought >> > about it, one way is moving the set_orig_size() after the redzone/poision >> > setup, and in 'check_object()' we can detect whether the 'orig_size' is >> > set, and skip that check if it's not set yet. As the manual validate_slab >> > triggered from sysfs interface is a rare debug activity, I think skipping >> > one object shouldn't hurt much. >> >> That will require smp_wmb()/smp_rmb() pair to make sure that >> effects of set_orig_size() to be visible after redzone/poison setup. > > Yes, synchronization is needed here. > >> Isn't it simpler to add a parameter? > > OK, I can go this way in v5 if other developers are fine. thanks How about get_partial() instantiates an on-stack structure that contains gfpflags, ret_slab, orig_size and passes pointer to that to all the nested functions. Would be similar to "struct alloc_context" in page allocation. Something like "struct partial_context pc"? > - Feng