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 BF538C02194 for ; Thu, 6 Feb 2025 02:57:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 31CD96B007B; Wed, 5 Feb 2025 21:57:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2CCFB6B0082; Wed, 5 Feb 2025 21:57:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 16E2E6B0083; Wed, 5 Feb 2025 21:57:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id ECD016B007B for ; Wed, 5 Feb 2025 21:57:18 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 90453160E6A for ; Thu, 6 Feb 2025 02:57:18 +0000 (UTC) X-FDA: 83088008556.28.7A77C45 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) by imf06.hostedemail.com (Postfix) with ESMTP id C002A180002 for ; Thu, 6 Feb 2025 02:57:16 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="CVMZLVF/"; spf=pass (imf06.hostedemail.com: domain of rientjes@google.com designates 209.85.214.175 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=1738810636; 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=mk6PiHA2jUMIAAc6GXP8rm3bb0PFPxp1kBvPr1tdqBE=; b=iQGjbcXXlwHzOvs9P5ToacJdiokPiw8va4Q+x2g3A6hdYjwsotzJlTrnAuZ1dytr93vVup LlAcBRwFNp476yD4/3rGrY9brkqA1fDz//LTJbUtMf6eNcGpjRVOSUmi+IhmoHrHUnGPfN pde7vaqyIZL22u/czyQF1Da+MzjwDPQ= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="CVMZLVF/"; spf=pass (imf06.hostedemail.com: domain of rientjes@google.com designates 209.85.214.175 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=1738810636; a=rsa-sha256; cv=none; b=GsCbS4MxuC62CwnFIYC4P9OrbutSSmOWuCpG1utcoLKU/8wYddjJiqk5U9mR2nZrcS3D2C ZWb8l5uH9yxJE1CUt5tcosjYiWWi9zP7/NzlQUsRgZN9KPlGhdXY5Tn9NZHtOsxr+yg14W GFadFbRUTw/aoCLwvrJVTGMXzWU9z4I= Received: by mail-pl1-f175.google.com with SMTP id d9443c01a7336-219f6ca9a81so26015ad.1 for ; Wed, 05 Feb 2025 18:57:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1738810635; x=1739415435; 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=mk6PiHA2jUMIAAc6GXP8rm3bb0PFPxp1kBvPr1tdqBE=; b=CVMZLVF/B7vmSKVXMuy486p77X1Zg+8/HUNWrNdjul22oD01uya/M/K5qQK1khYM8M qzphHhQTKHV/TuKk5CALWUxF05U+o5yRdkncNvlN6bdMxUiha4+m5lPUxH1cNRwaftwz GXHcjGU+C5hFR5SwVyjOLfhA1JpGkuDa6NmVV6LfSkG1tLxZApPOK2XhBY/IwhBwWqWP 2l4J1m8xqOn46jum+TPjNPVZiDXLQhp9V5zCtpuCxiMvH/pm6TP94lRpmrIVO2JEKcQG Mc62seBs7EysrYt8xPHBia1YGc/2IhbrbAtKCdHh0vbvaeiBpfPeB31WE7mBQ+DOPWIc QmxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738810635; x=1739415435; 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=mk6PiHA2jUMIAAc6GXP8rm3bb0PFPxp1kBvPr1tdqBE=; b=mrsOxqUFJkBQPvbetKNiGDm4z+w7Q+qvyPTQlXVN+G8ZdRZmCJE0TflgCAGCfMmjWy ebYGOMG3nuIcNUhu/+gTH0oKtnnVYv3ZgBecu3rIhSQ2x8mwqDaLJ8vmSk0Hs/z+4pBF z78rG4q+3ntadSEclUDOkWdT64INarmXtfJW468lTHemnUW7Yl7MjIfQ85OAs3VCKOeD R6x7u3br0FCw1HamQl0GjnQ537hp0UxYriNyT+PzvX2kJV0krnV09AfA1k7WoaUPwCnc zhBFgDN/RWulo2Wkcb2g+WYqSqcvBoZPOYcTF3tfjpxrHIFehmllbE4fb9FgqpchXKk2 PktQ== X-Forwarded-Encrypted: i=1; AJvYcCVRUyrfkWfbsrpdb5xzeQrOr9gWGK0fmVcnS/Lw6bAVo3ymcQ00eTtENOgimud+24MIPVT6iBCWrg==@kvack.org X-Gm-Message-State: AOJu0YwkavIvtdckfCSJvRsE+mXMrcUUb2kzCLt5fWVu9OFIfMFkojHD a8E1j0gkulNHCyj53dYpOJLFaHCfRzpj7LhKwQhyWM1mvck30RJcADL1mZQNtA== X-Gm-Gg: ASbGncvjhbNpEBa9DHUiQFp4Pm/20L/PaUW3jN7bI4yOwzwz+v/lATIpJM4mSHdBDTt ZQJQL/iaxx3aHsNNlvfk/WnIuFByI/qLynH4i6xg0qaojLLw6sEM8eNKckdZNtb8eOvr/EulN4y P9bt7kNV0+ge8s3sl0z7tg1lyNUtOKiz9EMyjdeG7pvQlsfBNJ3AoAqRwyAAeSkFdKVCXceB0I0 3bD7owoP2OG+FdMImHovOSG1UXaHPOLrppz6qs5XVbYM1JUHSHDFutoeO+0QnomWfYNOvnhCJZY 1wuB/1VenBhr0EBeLTUBYMLRgFnAKEZO9FJBnv2EMIqZMXRr/Ad4ij9qPXgC9E/jLPgfTmo934y gSsa83AUIFg== X-Google-Smtp-Source: AGHT+IFM1DuRCvSQG9hQ+ar4awv2YVkN6K8eMWqiCWTuOg17j/wgyyp0NwmSKpshMMQFbEAWd4kqPw== X-Received: by 2002:a17:902:8f86:b0:215:65f3:27ef with SMTP id d9443c01a7336-21f34910ed6mr720325ad.12.1738810635348; Wed, 05 Feb 2025 18:57:15 -0800 (PST) Received: from [2a00:79e0:2eb0:8:23ad:2d69:b4a0:1176] ([2a00:79e0:2eb0:8:23ad:2d69:b4a0:1176]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-73048bf148dsm191432b3a.88.2025.02.05.18.57.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Feb 2025 18:57:14 -0800 (PST) Date: Wed, 5 Feb 2025 18:57:14 -0800 (PST) From: David Rientjes To: Vlastimil Babka cc: Hyesoo Yu , 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: <135f5cf7-6853-4715-bd7f-41c7f554ec31@suse.cz> Message-ID: <6c45ced8-3a01-0905-0d2c-f0e7b7acc2df@google.com> References: <20250205004615.1253389-1-hyesoo.yu@samsung.com> <55522d9a-7fd2-1df0-19df-1552644b009e@google.com> <135f5cf7-6853-4715-bd7f-41c7f554ec31@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: C002A180002 X-Stat-Signature: 5yt3fam3g9o9cix86wad184oqx4ydea8 X-HE-Tag: 1738810636-936287 X-HE-Meta: U2FsdGVkX18I5UES0MEuVdSO4KbfPMKp0MmZfyDBjhMs6Vr0fVVTsdfQK2xS9cwcKmlHZHRE/PRsJbbK9cBe9fg/kEVM0SsE6qIF7OuW9sdpsaa+xqYmd60cvTMC0/t1u2cOmhszpYuucHQYtB/WvdQx/80LZxXMLJcxEh4NZD1vuFnP+sdrLhDMxCWuRchMU+ovJ1HGdgGQLogn4+yCc+IFMkGiMn8aRv6ODdG+wqJItAFObMxsit6Fd3Rnt40q19GuPYZPjhKjLOquuiB3Bw1eyebWTB8nDIXz5XKI4N3ZicLd+JtlZEWh0SggcuEz6GNzyLva/KgkOKzD8S7n6EjeFfaHX6zLSM/pe8nlsWRW0IjyRLC82KfU9TH961obfq6whWCG/bo5iqVUi7o4xy3D2Dj/wURxXAW6Vh3rw1XC4Lr/7HLnUVVLkm1Tl13M99ED3oOH0WFU9glBw7y5M0g60ks5/xhuEkyRoRlrAnpjBvpTulg+5hjihvnYKOaiIHaK0z5q7pM1rUu7hI9xu3bd6SobltUVasbsN8Slq12+LBO0eCGc8Qj7cFcAe29lhqd07xmjzpZjtTTz5acTarA4lNgPobucB62dWyjyURZCzjuIoDbEdst8KJB+TpEV7K2qAeOsj4Ejs/H7Yd7tiS5e8C3jy4rafzzMXKOVdSkYF4E7o864uC2ArBstOJcXV8P9M8dFOB74vKzG9/vS54JyNSjjYwLGv3CMYFHjaymg0JtrWo3X0YXUFLXwlVjd+E+GdIWAW62fBDwfbjzIb/YzRxkIHvCYfFloOJLKP9j5SgIEBI5XaWMal9atcmlASOuj5pTaWSpQFIMwR3Dz7l7087H8Bj/k5JIIeCNxtJPaZ4c4Gau6zHlizrjexMXosaSh3FTAx/r1e8miXzWDbMV7N9xPz5/5ILK28nf3EIreCvDAz2UAmKDYiEDFfgu9GsTkrHBjpmmYKhdAyqV t35EYiJr Uiit0GW6ijkNVIE24jyqu1rF3xCi6LV5dHTZdgA8kiaPi3R5/BOM2o9Op6BWnuKiknOj95gzka9xaQDfMEyyfI5aZTw5GUhUd7WHPR5gEqkCJp9+r89czZ25e8P2b6hZTqNWqMz7uqLiH/82pKkH2Qk6G+naBbPGoZZsFCv/hFEr88ZE0CQ65MQXxrARU8ek41PtKyLuHJWu394qE5J72WOB9Oq1Lzz+rXKVLG9teFuOAUgCzFWBTC+f15QEMJGjHAFMfhlMdr5RIkUbRyej3gnNHoH00q8kQeQYfbkht3eEMBCCpe2r2bvDeCeyHcmmTjX5L6hGhZp3cOZr/kMJwytWzicNynVAjGin2vFaAdwFtGMQ96c+A3FlLmDMO53Y0nafDzA/ltLeGw9aeX23fdVT9369TxQX0T5YujiLryJ+XzXg9ww2BoOvq0Y1LnrTyrmTFeI2aCFcrGJuvuVZjfbM/crxgpDzzNxqaf7YoBBcanwgNUuz/ZDa+LI/xVC5DmIVL 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 Wed, 5 Feb 2025, Vlastimil Babka wrote: > On 2/5/25 18:10, David Rientjes wrote: > > On Wed, 5 Feb 2025, Hyesoo Yu wrote: > > > >> If a slab object is corrupted or an error occurs in its internal > >> value, continuing after restoration may cause other side effects. > >> At this point, it is difficult to debug because the problem occurred > >> in the past. It is better to use WARN() instead of pr_err to catch > >> errors at the point of issue because WARN() could trigger panic for > >> system debugging when panic_on_warn is enabled. WARN() should be > >> called prior to fixing the value because when a panic is triggered by WARN(), > >> it allows us to check corrupted data. > >> > > > > I think this makes sense, but it doesn't document why the other changes > > are being made, like moving the setting of *freelist to NULL. This is > > presumably something that you want in the crash dump when > > kernel.panic_on_warn is enabled. Probably best to call that out, but to > > also indicate what you're relying on in the crash dump to make forward > > progress on in diagnosing the issue. > > Well the last sentence of the changelog above says exactly that, no? > Sorry, I should have been more clear. It's unclear in the code why choosing WARN() here is helpful given the stack would be known. It makes sense to enable kernel.panic_on_warn this way for debugging purposes, but thought it should also carry a comment in the code on the rationale (and the state we're trying to capture in a crash dump) so a future change doesn't go and unravel this for us again. > >> 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 > >> > >> > >