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 808ADC02199 for ; Thu, 6 Feb 2025 17:59:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DB0BD6B0085; Thu, 6 Feb 2025 12:59:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D39FB6B0088; Thu, 6 Feb 2025 12:59:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BDAB66B0089; Thu, 6 Feb 2025 12:59:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 9CF9D6B0085 for ; Thu, 6 Feb 2025 12:59:38 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 5724812143E for ; Thu, 6 Feb 2025 17:59:38 +0000 (UTC) X-FDA: 83090282436.04.3745169 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) by imf26.hostedemail.com (Postfix) with ESMTP id 84875140008 for ; Thu, 6 Feb 2025 17:59:36 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NGwnTbh1; spf=pass (imf26.hostedemail.com: domain of rientjes@google.com designates 209.85.214.180 as permitted sender) smtp.mailfrom=rientjes@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738864776; 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=WJ8YPkOMUO80cyX2dVP3JLGw5LXCqDVZtBhpVk7cKgA=; b=lU1Q1P6gf1jdHc6ol6dVJraXw5gfNDjHnkINcmgkWcjOEhiP+dZqhgg/FymZr/cCxflMF/ zDUIN35eOxCjOSt05am+B8pR8+aqNCNrNDgNsP+XhJG2TQwLX7DyLehRj6Qnk3imiADneJ 2SGGTcfgCzCnLhKq0gIz+wmRP8NpkQE= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NGwnTbh1; spf=pass (imf26.hostedemail.com: domain of rientjes@google.com designates 209.85.214.180 as permitted sender) smtp.mailfrom=rientjes@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738864776; a=rsa-sha256; cv=none; b=SLvrJd6pToGeORwG5hxE60gtr9PWGa46FSWE+9P7aCEdWy5JFKXvbe4NFIYDAIuiVHQFR0 oivJtFZ1KdND1Lsw+6R6mmyki+Pq1OiXyjyU/pvN8kR0dSf2vO/BORiJbdlfcfD2ai42k9 xx2GJ8IIrXNI4NWsk23dtKC7pwECYXE= Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-2163affd184so5535ad.1 for ; Thu, 06 Feb 2025 09:59:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1738864775; x=1739469575; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=WJ8YPkOMUO80cyX2dVP3JLGw5LXCqDVZtBhpVk7cKgA=; b=NGwnTbh19GEBclPy7qDKuMo7DwKaSTkJANc1CkpPYoHtBOuYRIXUojVMOXJS6fKkUX 39ndt8s9v93VE4iKYLuxLyUT3tsuHC+vQ58iOfUo1yv/hXuCiW009CMlWA7m1PUoSK0p bROmHJjcUTpBth1h4CetkhtYQmiNkIeR/kYTvLijMbwe6hCCWzU8k/q3VxZ3jle4a9me vgfJKQGvmmTTQs0DYtp8jPAb18ODB140JNKoQ7pO+fGLCgk+nNvyenENbMTQHiiMxaa/ y8DVF2aHt9/ONdJeR5lA9o6q4wa+Qd65wgV73ZjrSK3ctf4DDsv2nuDLzeI2acX2mD3D SbLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738864775; x=1739469575; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=WJ8YPkOMUO80cyX2dVP3JLGw5LXCqDVZtBhpVk7cKgA=; b=R0sur2BVdk26WZGx5IExhM5UucuaCLHdfNWp2AoMSU5sT6G909mrkrmbYdv/OkbDgj gUqGcd5dH1o4BqgY6sNx7R7rN2z3WQ54/viXPEeAxTZr51kQfUZ5vvaAr2ATVS89Gucy jxJhAqFaKLjd+ftg6mU74KqGpupqJ03FLiRuGHrnmOaeeAsXxXE5W772oHp4VN1sE2k5 Vq3O3xlzHgzDA3ebb2m+SMcmwVGXrqb9hQ6DEgJDcXwdKJW9DplIo7voQVO/NMANk8xD /3hz5nV0+FjEDUayp3xeSm2cinZ6F4cLXkXAkSRObRaNDKD57/d1zCeIZK4P7CvoZRGG 2fng== X-Forwarded-Encrypted: i=1; AJvYcCUhQw+zmSbx2xQel8wNGiTxRjTMM1LPSORwy1YknGvj/UabjaG5tZLdwNvojjmMK/PWfdvimHUI6g==@kvack.org X-Gm-Message-State: AOJu0YzVriS/UafIltPxMr8hA4RSR5+JzK/S65K8Xdvj46fmSI9LdfT5 KBlZZFuQC6ATEyaCcc4mhvgREEddmPtmuSeSEsWpbJJ5sV9zKFp7ikR9pTBPBw== X-Gm-Gg: ASbGncvijfQZNY9P/iplpm5DaevfIYUnhoIYdqrmZyFLmVH0ncFsQn4NBrbHyrQknwe eLUaI21O1b25BS+kvt9aWCBKfqlhfQLRJiKhCly9WSDW0FD6RUb8uTS5CjKpRN79diskQjiQks3 j32MJU+UldXyVOjJhUUr0MN1nrb12T/1/pRsEI8dZgyHLtkYWgP34Th1aKN4zk2rQKsQZ0n5weg Y9IkAg5Q3waGyS2u192z0aa9wivRndsxg7xzqFnL5uMqW1uPrapSjyYLqdDt0nzQRXhGdfDfNec Jwutu6iLi3xTjthzo1zPOLlaYYiv82fvZoWIRssLFPBWTc4s9J+uuFMY4DzM6RK6A16RxPIQfC8 HwvgIGaFTZQ== X-Google-Smtp-Source: AGHT+IEN/Y0NxcItL47cvZJJoEqFsp2piQSpMBfdLfPIfzK96cWROUKdUK6zxEOm5Ig3xp/wn81yxw== X-Received: by 2002:a17:902:76c4:b0:216:6ecd:8950 with SMTP id d9443c01a7336-21f4c51c6b2mr388115ad.19.1738864775015; Thu, 06 Feb 2025 09:59:35 -0800 (PST) Received: from [2a00:79e0:2eb0:8:3f6a:24e9:5ba0:72c3] ([2a00:79e0:2eb0:8:3f6a:24e9:5ba0:72c3]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21f3650cde8sm15999595ad.45.2025.02.06.09.59.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Feb 2025 09:59:34 -0800 (PST) Date: Thu, 6 Feb 2025 09:59:33 -0800 (PST) From: David Rientjes To: Hyesoo Yu cc: Vlastimil Babka , janghyuck.kim@samsung.com, Christoph Lameter , Pekka Enberg , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mm: slub: call WARN() instead of pr_err on slab_fix. In-Reply-To: <20250206061507.GA3959749@tiffany> Message-ID: References: <20250205004615.1253389-1-hyesoo.yu@samsung.com> <55522d9a-7fd2-1df0-19df-1552644b009e@google.com> <135f5cf7-6853-4715-bd7f-41c7f554ec31@suse.cz> <6c45ced8-3a01-0905-0d2c-f0e7b7acc2df@google.com> <20250206061507.GA3959749@tiffany> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 84875140008 X-Stat-Signature: 5mwutj5xg3fbtmwq4s8dahr4f75g3obh X-HE-Tag: 1738864776-312119 X-HE-Meta: U2FsdGVkX1/AUsSsRfiZt4ZRBTqtJsDK1JsCWJRXXPZD6w5VY5pOKsSB8OUWy9ugGGS4YbYjXC2NZbC9oBxMRv14JfMejH1zy15pPkIAZCu/F6HFvfuRhuclw0FkveMZl3baI4wjq2zCNhFaFTc7oFXmyomXX108IsRxKqeVrc6CDwwb67ldlcaDvF710aSxbGIUujorJwp/AGUjmv7LK2NMqbaYO/X0wSv1uXZnlSG91r/0IqLIGRCwVHfY3KBLg7UcHnXp+BIueV86520EIu+zGiH9oMterp29CYskHz3wrzaZnV0rorX+yD3gjOcDLDqk3whoGwzVWDHwB8d1vfyRGXfRkI6/p2sirRcgZCSouAaae4dFjCfl41/QaYlHjycfWfNE7pLbYkjWXL+Ul0BnNCLgumH+bOHXFhYLHrJ5EaYSqsfTMxUfAvqkn58Yi7qfYnBy/N0HYnpU4irth/9InnT5KccZIRKEW2p0noyevcob50XH2Y1GJrspfSoYvzNe00sfQzxPm9yMn90/rwdrdr/6UKp7Edrcl4pBq8yKfkVlzLk0QWxo1UkMgT5nsTqYluSXQ3bpVwlxLYGeP8i2J3o/0g2+cIEoBqJLHwBveyfhKWaNJjt81XPZIwvAmX92Du4rFhWLVDETo0RrHKZNovWYd3ytAnF1phQW5ASsA/MEEMEPlhvZBzayAst9bn31lJpiux834rtIaf0NdwDl/yfLqi/8skAf7tSibT6KxuTARJT+yoOG59eb2izX+6i+4k4HNv8//QcBWl0bV1ivY3GWdbSbU8jZ6GNd3QqV10s3Tyx+VpQcDmZF2kFNHQCvdPLkpuD6JTXnZVVyBrz4Z4LTcSdr7Ikv8/Q1lzVjE79SCqMykyLxWXdvHiyN7/v/Jen1eKcTY+lDxdmEyG1lbgRLkUSNbCD39qFRD7DEt81mdlfyZopw/aeaLgcr1cvStyXJMyCi3l+2PyM dqkKUGFZ XGkeX0Xc7ZmbGPURZhYsYoOaugkuRgYZ7Z0+FkN9cbHqN9yyGMF8IfD2zpI+7wb/anR3JYW5aEOiRDJjpkRbj6rqMTrFuGXU6Jym0RaWyl2bPBoEDVOQcVDrYHHwdKxAd6lTzAmZuxHtYRo5w+UKtN0BY9uotj31vwUjtqoU42AtIzFdG/5N81Sf5L0M7mdfWFYIsTYDDAhvy4+196TLXyEKrxk+a+8XGD8DJlgFijlVLTDwNJuqPac4NxE9IsWOtclLI5d/0ibRZENM7batF06hTrAvKN7Ahql3CIFsj9FWuEyr/gNt6OBsD5j68oZ1RuxB9c35/I7iGj17Pc/9EL9nyLs3TBHynMFmdd1Pp8V6Pov6CmyO6LzG8PTCb57W+hwyQqEUlvbGYX0E/91RLl0UcTd3F4GwMn/kvimDqCUkBfP8gu1t4rZUolbofBgTVacDTNuFPKXahRJMkGTk61luBh/HuGNn8Ipw7XGHrk3HeksSUnXyfibNEGshReslUPrTF 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 Thu, 6 Feb 2025, Hyesoo Yu wrote: > What do you think of this comment ? I will add it in patch v3. > > /* > * slab_fix indicates that the value would be restored even if an error occurs. > * Or, it is possible to trigger a panic without restoring using WARN() if panic_on_warn > * is enabled. This can obtain a crash dump at the point of issue to debug. > * It is advisable not to restore the data before calling slab_fix() to check for corrupted > * data in the crash dump. > */ > Looks good, you might consider it to be a bit more brief, but otherwise looks to clearly document the intent here. Was thinking of something like: /* * If panic_on_warn is enabled to generate a crash dump, be careful to not * restore data before this point. */ WARN() But I don't feel strongly. Thanks! > Thanks, > Regards. > > > > >> Changes in v2: > > > >> - Replace direct calling with BUG_ON with the use of WARN in slab_fix. > > > >> > > > >> Signed-off-by: Hyesoo Yu > > > >> --- > > > >> mm/slub.c | 10 +++++----- > > > >> 1 file changed, 5 insertions(+), 5 deletions(-) > > > >> > > > >> diff --git a/mm/slub.c b/mm/slub.c > > > >> index 1f50129dcfb3..ea956cb4b8be 100644 > > > >> --- a/mm/slub.c > > > >> +++ b/mm/slub.c > > > >> @@ -1043,7 +1043,7 @@ static void slab_fix(struct kmem_cache *s, char *fmt, ...) > > > >> va_start(args, fmt); > > > >> vaf.fmt = fmt; > > > >> vaf.va = &args; > > > >> - pr_err("FIX %s: %pV\n", s->name, &vaf); > > > >> + WARN(1, "FIX %s: %pV\n", s->name, &vaf); > > > >> va_end(args); > > > >> } > > > >> > > > >> @@ -1106,8 +1106,8 @@ static bool freelist_corrupted(struct kmem_cache *s, struct slab *slab, > > > >> if ((s->flags & SLAB_CONSISTENCY_CHECKS) && > > > >> !check_valid_pointer(s, slab, nextfree) && freelist) { > > > >> object_err(s, slab, *freelist, "Freechain corrupt"); > > > >> - *freelist = NULL; > > > >> slab_fix(s, "Isolate corrupted freechain"); > > > >> + *freelist = NULL; > > > >> return true; > > > >> } > > > >> > > > >> @@ -1445,9 +1445,9 @@ static int on_freelist(struct kmem_cache *s, struct slab *slab, void *search) > > > >> set_freepointer(s, object, NULL); > > > >> } else { > > > >> slab_err(s, slab, "Freepointer corrupt"); > > > >> + slab_fix(s, "Freelist cleared"); > > > >> slab->freelist = NULL; > > > >> slab->inuse = slab->objects; > > > >> - slab_fix(s, "Freelist cleared"); > > > >> return 0; > > > >> } > > > >> break; > > > >> @@ -1464,14 +1464,14 @@ static int on_freelist(struct kmem_cache *s, struct slab *slab, void *search) > > > >> if (slab->objects != max_objects) { > > > >> slab_err(s, slab, "Wrong number of objects. Found %d but should be %d", > > > >> slab->objects, max_objects); > > > >> - slab->objects = max_objects; > > > >> slab_fix(s, "Number of objects adjusted"); > > > >> + slab->objects = max_objects; > > > >> } > > > >> if (slab->inuse != slab->objects - nr) { > > > >> slab_err(s, slab, "Wrong object count. Counter is %d but counted were %d", > > > >> slab->inuse, slab->objects - nr); > > > >> - slab->inuse = slab->objects - nr; > > > >> slab_fix(s, "Object count adjusted"); > > > >> + slab->inuse = slab->objects - nr; > > > >> } > > > >> return search == NULL; > > > >> } > > > >> -- > > > >> 2.48.0 > > > >> > > > >> > > > > > > > > >