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 8F286C04A95 for ; Wed, 28 Sep 2022 15:39:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CB78A6B0072; Wed, 28 Sep 2022 11:39:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C66DD6B0073; Wed, 28 Sep 2022 11:39:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B2EA26B0074; Wed, 28 Sep 2022 11:39:06 -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 A38886B0072 for ; Wed, 28 Sep 2022 11:39:06 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 615911C6A8D for ; Wed, 28 Sep 2022 15:39:06 +0000 (UTC) X-FDA: 79961902692.16.4A9B20E Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by imf20.hostedemail.com (Postfix) with ESMTP id AA4851C0985 for ; Wed, 28 Sep 2022 15:21:20 +0000 (UTC) Received: by mail-pl1-f174.google.com with SMTP id y20so1969367plb.2 for ; Wed, 28 Sep 2022 08:21:20 -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=+WEGG8bdhAe2IeIV7nqdCMLjsT+NYXr3vPeV8ohPUdo=; b=NWQyIQIxOLBbO8SUWUFvnbiVWQh98VMz4aaF7yBvntNeo0lhVe58k8UbVlT21ceSy3 N73J25O0TqMb7/Thn4zweOWpAI5H0J0ntIKpBQEAuOKdh6FgK1uAI485b6iHqcWhI1Pl bo4Rx9klC7pi4nz6dh5u3ntQhBZe+lpNuMRyBggNRNpyvKDUds8WsQLcy/KE+8aJLfWQ iRlwAUj5xdkEv8czArv0Qq0NY/vLa4Ty7E6RWatQOK91Alq1xvljZFnjyb1DOCEtMwSs 2qMFu7mVUL95Ss/7LqKnn37AkHMHMy2RYH+POfMXUwH6+T16znqYpIEGsYIdmODza/xY 3F5w== 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=+WEGG8bdhAe2IeIV7nqdCMLjsT+NYXr3vPeV8ohPUdo=; b=nn09iaHkIdBcSQhX6K0/ME8cp5qAFwdlciqJ3O7NFMHjCQsubskaoTjtc1a3j/rTv/ RsgikASG9on+bIkOpkLDpVz6J4mDLwDTaFnFRDF6VdQnDDwR/IQ3BNBS94uz5I8YetNY HPcMPHC6jxkKgNAuDZku9T+BE2pG4Eva9H903Q76ggGTPBtweyjLWRHCc8Rw8YboYPGu N9dXeeqWeHE91bYn48LA0S13Zu7vwpH8nCRD49sJIDgGTY3FyGsfc/+hVg7vxouErsaP 26BkKOQ+D2SnyRgDdlPxrjQu1Kl3WmK8Co9bWqb8pSyVX7RbzBlJKZ9Dx2DyTo9ajIQW fUwg== X-Gm-Message-State: ACrzQf2de5i9YGLs3qAyIOdH4wOysdgJBdoE4ErQ9SHvRBL3gJC8mhtX BVeO5iJWjeNv0gYUG6r7UWc= X-Google-Smtp-Source: AMsMyM5mMWhYz3X1436kKEvjTcv3eDyNmzU671ZYKCbUm/02hn0itjtfv1akLF+RCA7rJZzYyQPCdw== X-Received: by 2002:a17:90a:2fc9:b0:202:5605:65ae with SMTP id n9-20020a17090a2fc900b00202560565aemr11170445pjm.167.1664378479610; Wed, 28 Sep 2022 08:21:19 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id y2-20020a170902864200b0016c0c82e85csm3784367plt.75.2022.09.28.08.21.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Sep 2022 08:21:17 -0700 (PDT) Date: Thu, 29 Sep 2022 00:21:10 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Alexander Atanasov Cc: Jonathan Corbet , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin , kernel@openvz.org, Kees Cook , Roman Gushchin , Jann Horn , Vijayanand Jitta , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2] mm: Make failslab writable again Message-ID: References: <20220920121111.1792905-1-alexander.atanasov@virtuozzo.com> <30063d97-69f0-bea2-9d59-108140995bfc@virtuozzo.com> <7640a2d9-a32d-2fd7-8f64-586edb9b781e@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7640a2d9-a32d-2fd7-8f64-586edb9b781e@virtuozzo.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664378481; a=rsa-sha256; cv=none; b=bqzpBowZHffFSczVG3gmQvZNe8/QI7JD6fFdgFu8P4Icq+RVRpNu3g3yzMHtoPprgm0fxS fuEohq2/Z7yVxL1U41d5X1/Zb54Cz/lGDW3ITU0+tSomCImZfPCj1nIqkC5db+cBd9azsY 0iRxLui7NdRX3i53MlcvEdvYGuhAEbY= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=NWQyIQIx; spf=pass (imf20.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.174 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664378481; 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=+WEGG8bdhAe2IeIV7nqdCMLjsT+NYXr3vPeV8ohPUdo=; b=RAWGDFOjKJr4o5rKD8pSHY+bgRCFoS5+Pb4BLoJdno5Ef4KthvYI9EImY8V7C9uSZGMc9I gXeXQn6ZLPcx5QiS96Vv9LWm8qeh3WLPjkrDkbabakmkMxyYzPT+Kxj6/69EqFKl9qJIRU m2p2LSbdGOhQXXTQt/X051IBhTzoPwI= X-Stat-Signature: 35mmsypphbxdtnp897hajpgmptmwnakt X-Rspamd-Queue-Id: AA4851C0985 Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=NWQyIQIx; spf=pass (imf20.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.174 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1664378480-564876 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 Tue, Sep 27, 2022 at 10:44:20AM +0300, Alexander Atanasov wrote: > Hello, > > On 27.09.22 3:49, Hyeonggon Yoo wrote: > > On Fri, Sep 23, 2022 at 10:34:28AM +0300, Alexander Atanasov wrote: > > > Hello, > > > > > > On 21.09.22 14:30, Hyeonggon Yoo wrote: > > > > On Tue, Sep 20, 2022 at 03:11:11PM +0300, Alexander Atanasov wrote: > > > > > In (060807f841ac mm, slub: make remaining slub_debug related attributes > > > > > read-only) failslab was made read-only. > > > > > I think it became a collateral victim to the two other options for which > > > > > the reasons are perfectly valid. > > > > > Here is why: > > > > > - sanity_checks and trace are slab internal debug options, > > > > > failslab is used for fault injection. > > > > > - for fault injections, which by presumption are random, it > > > > > does not matter if it is not set atomically. And you need to > > > > > set atleast one more option to trigger fault injection. > > > > > - in a testing scenario you may need to change it at runtime > > > > > example: module loading - you test all allocations limited > > > > > by the space option. Then you move to test only your module's > > > > > own slabs. > > > > > - when set by command line flags it effectively disables all > > > > > cache merges. > > > > > > > > Maybe we can make failslab= boot parameter to consider cache filtering? > > > > > > > > With that, just pass something like this: > > > > failslab=X,X,X,X,cache_filter slub_debug=A,> > > > > > > > Users should pass slub_debug=A, anyway to prevent cache merging. > > > > > > It will be good to have this in case you want to test cache that is used > > > early. But why push something to command line option only when it can be > > > changed at runtime? > > > > Hmm okay. I'm not against changing it writable. (it looks okay to me.) > > Okay. Good to know that. > > > Just wanted to understand your use case! > > Can you please elaborate why booting with slub_debug=A, > > and enabling cache_filter after boot does not work? > > I didn't say it does not work - it does work but requires reboot. You may > want to test variations of caches for example. Cache A, Cache B ... C and so > on one by one. Reboots might be fast these days with VMs but you may not be > able to test everything in a VM. And ... reboots used to be the signature > move of one Other OS. Thank you for elaboration! Makes sense. > > > Or is it trying to changnig these steps, > > > > FROM > > 1. booting with slub_debug=A, > > 2. write to cache_filter to enable cache filtering > > 3. setup probability, interval, times, size > > > > TO > > > > 1. write to failslab attribute of (may fail it has alias) > > 2. write to cache_filter to enable cache filtering > > 3. setup probability, interval, times, size > > ? > > > > as you may know, SLAB_FAILSLAB does nothing whens > > cache_filter is disabled, and you should pass slub_debug=A, anyway > > Okay , i think there awaits another problem: > bool __should_failslab(struct kmem_cache *s, gfp_t gfpflags) > { > ... > > if (failslab.cache_filter && !(s->flags & SLAB_FAILSLAB)) > return false; > ... > return should_fail(&failslab.attr, s->object_size); > } > > So if you do not have cache_filter set ... you go to should_fail for all > slabs. Yes. > I've been hit by that and spend a lot of time trying to understand why i got > crashes at random places. And the reason was that i read an old > documentation that said cache_filter is writable and i blindly wrote 1 to > it. > > If the intent is to only work with cache filter set - then i will update > the patch to do so. You mean to set cache_filter to true when writing to 'failslab', or when setting SLAB_FAILSLAB slab flag? I'm not so confident for that because it's implicitly changing. Maybe more documentation would be proper? what do you think, Vlastimil? > This is the only place where SLAB_FAILSLAB is explicitly > tested, other places check it as part of SLAB_NEVER_MERGE. > > But even for all caches it is kind of possible to test with size(space) > which is in turn useful because you need to figure out how you handle > failures from external caches - external to your code under test and you > don't want to keep track for all of them (same goes for too much options in > command line). Yeah, we should be able to inject fault in all caches, or a specific cache(s). > > to prevent doing cache merging with . > > Or you can pass SLAB_FAILSLAB from your module when creating the cache to > prevent merge when under test. Right. I missed that. > > > -- > Regards, > Alexander Atanasov > -- Thanks, Hyeonggon