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 E9E4DC5B552 for ; Tue, 10 Jun 2025 19:09:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 877D36B007B; Tue, 10 Jun 2025 15:09:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 828736B0088; Tue, 10 Jun 2025 15:09:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7185E6B0089; Tue, 10 Jun 2025 15:09:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4F6F46B007B for ; Tue, 10 Jun 2025 15:09:05 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E4B60120DC8 for ; Tue, 10 Jun 2025 19:09:04 +0000 (UTC) X-FDA: 83540428608.08.F613AFF Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf04.hostedemail.com (Postfix) with ESMTP id CA87C40016 for ; Tue, 10 Jun 2025 19:09:02 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="D/d7I17H"; spf=pass (imf04.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749582543; 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=EDLdDx52Tt0xGB18gauxVIElFFaGCMshYvQZPbVpNqI=; b=7LVawwe07xSxAI1Q6677V6kLTSiusOpbDCiRfBLNVkODPXz5pX35z/va15G8eToriiT2q8 RQufV71VKb2H4/b0VUafN+7jNP522GYmWjkYJQQbq0Hz/amNvwRMsx6z9EsQ3bb5VXh0Sw UNxDKv/kRLwpS+cWgjaMAPwxSNeRWJU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749582543; a=rsa-sha256; cv=none; b=yqs734hPmwI43pcR7lCm3XlC6GCikqTDvNO5xKzFlfYp7SPcz30UgAXJIaZzapuBnFXqUG toQwzxYP2Qli9QurEECHPFsYdcjXtDKTUOGkqCr0TmY1uCNkVpCjH3XpjAfsTxAVlas3nQ lcpB7ACR0hbASBmb+HHFJzmO2xL3nMU= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="D/d7I17H"; spf=pass (imf04.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1749582542; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EDLdDx52Tt0xGB18gauxVIElFFaGCMshYvQZPbVpNqI=; b=D/d7I17HPXkkntsvBMw4KhWoFesp8neVZ/7fRKBGwVacqcAfM9F4AOg9aj7YMeu7kD7rIp 7FdncO3LnIaoI3oWWIrSQ33oZY5NhJa1oPwTU2mo8Y6qXOy/OZmCijw5seb8ZKgT9TDuOq 7X3hWQAyvB3tD6euGPt4a6Z8jwQyZIM= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-487-Uvd6UUwyNea9QZ_I4i4zfg-1; Tue, 10 Jun 2025 15:09:00 -0400 X-MC-Unique: Uvd6UUwyNea9QZ_I4i4zfg-1 X-Mimecast-MFC-AGG-ID: Uvd6UUwyNea9QZ_I4i4zfg_1749582539 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 90FE3195608A; Tue, 10 Jun 2025 19:08:58 +0000 (UTC) Received: from bfoster (unknown [10.22.80.100]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 8D94519560AF; Tue, 10 Jun 2025 19:08:56 +0000 (UTC) Date: Tue, 10 Jun 2025 15:12:31 -0400 From: Brian Foster To: Christoph Hellwig Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH RFC 7/7] xfs: error tag to force zeroing on debug kernels Message-ID: References: <20250605173357.579720-1-bfoster@redhat.com> <20250605173357.579720-8-bfoster@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Rspam-User: X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: CA87C40016 X-Stat-Signature: dqjwmpj6hywfgota9ewzdjctwpmc47zm X-HE-Tag: 1749582542-846075 X-HE-Meta: U2FsdGVkX198Vz4c5lMC8g4Vqke9K4U8bXX8R68xKsZXs8JwTPyt1/6S6hmz+yaqm6heevkOR7XNlmYXKTqE9BtGJZg1HmcMZYjd8QgH5jey2AvxACaNqBHWQe9Jvkdr9iucy2IuYBZWvPaROrYzKioTmZoJx4+kYDkVY2q89nbQeini3T2tSGb6F2UNou6mcG8y03BQ/MF09CUceV62Wn+vbALJNJ2t5dtoPW9KHQouGPV/MaJYFylc/klEcQWSpMG0hd02lhi47YpIuKI4EA6H+Iks5l65aS4S/445ag0e7x+g+mldxgobOehuZW2LVnhxrwKXprsMer6TBithmqLnPDF0+0VrSJZ7igEIfqQt9j3pXG2AYlFl/9V1ykQqiYUkQJIZQgEDw19DChcZIOFLP9bRBxRg4mKGmlYaB0CYVy7ZxV5hTJQpIvi8vv4GJKL1Mmb5giSjI5k8ZC72i/8gyl8YqSaPr+sSTyq6qwKTtbkshxtLm032pdgBRCpzYOyuf6P3DVIQ82WAvBPZdSeZTvNTOjbbwZd+PC/XLIfyXXz7T9SXOJeB9FCRtvcGXHwdn6qaWmjXeuytOdt639nhWTZbUl8/jVj6AZW9D16TQPWWDThsYaO4P+8gRlEFNOGeT9MdDOgiMhkT04QdIVw4dh+JQiciAjfUfwalbuVuFfQ+gy0XXsMogXNwhHWkVdMt88T1hp1TUdmWwQS5RxaRnHqUsFW0VcRhJELDhJy3V/JxEWqBr8k13Z/82ksdDhFQktjYMOVw3afE0ppTe5YYC6XsWWZx9V3uAbIS3gRZFQ4fdU/nP8Q5NGIshkIa68pR4vKM29idScvV3K32a3Gx/Hrd0l//G/6OAVe/U+htLZ2AObDu9ZPHa46CNeCUyLICWGcYiWVMO/THPI9aFjO985cPAuDDhNw6Vx8jNtZRCqdJXe+xrfzOlZCWSzJlNhDhKjd42KVsJv0dwRA bAlAx/HT IB1lQOL1KOK06R32jQoShKJBecMNS5KGHoPIXc+1kk2scT+tubmFlJpk5dYIKRszGhjk3+4Ts/P6Lp/5pFDKqEvND4ZC0NUyZZUZhe21NjC/FFem8r+0nYUG4yoDyg2nxL+KYAbwocNTKonBI05GQtSrao/aF8tLDzDSzKPJGelXeHfpg16cHu1hgBLN6ODbE1RzD6tKtf+fdNZKjA40Ve54W0XPGgJNEpur1wTeF2kiK5Un1qe330/KsJwuCjjURDvAlBv31TroLBzmy1figCFnmUHjXSjX2huj2mweV5kLY91NMmmtTS00lfVIOaxXpySWCcg9swU4LB0N2YXR5Qu5STaMbMX3m1R+LH6xi9qGWZMc= 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 Tue, Jun 10, 2025 at 10:20:28AM -0400, Brian Foster wrote: > On Tue, Jun 10, 2025 at 06:30:29AM -0700, Christoph Hellwig wrote: > > On Tue, Jun 10, 2025 at 08:26:45AM -0400, Brian Foster wrote: > > > Well that is kind of the question.. ;) My preference was to either add > > > something to fstests to enable select errortags by default on every > > > mount (or do the same in-kernel via XFS_DEBUG[_ERRTAGS] or some such) > > > over just creating a one-off test that runs fsx or whatever with this > > > error tag turned on. [1]. > > > > > > That said, I wouldn't be opposed to just doing both if folks prefer > > > that. It just bugs me to add yet another test that only runs a specific > > > fsx test when we get much more coverage by running the full suite of > > > tests. IOW, whenever somebody is testing a kernel that would actually > > > run a custom test (XFS_DEBUG plus specific errortag support), we could > > > in theory be running the whole suite with the same errortag turned on > > > (albeit perhaps at a lesser frequency than a custom test would use). So > > > from that perspective I'm not sure it makes a whole lot of sense to do > > > both. > > > > > > So any thoughts from anyone on a custom test vs. enabling errortag > > > defaults (via fstests or kernel) vs. some combination of both? > > > > I definitively like a targeted test to exercise it. If you want > > additional knows to turn on error tags that's probably fine if it > > works out. I'm worried about adding more flags to xfstests because > > it makes it really hard to figure out what runs are need for good > > test coverage. > > > > > > Yeah, an fstests variable would add yet another configuration to test, > which maybe defeats the point. But we could still turn on certain tags > by default in the kernel. For example, see the couple of open coded > get_random_u32_below() callsites in XFS where we already effectively do > this for XFS_DEBUG, they just aren't implemented as proper errortags. > > I think the main thing that would need to change is to not xfs_warn() on > those knobs when they are enabled by default. I think there are a few > different ways that could possibly be done, ideally so we go back to > default/warn behavior when userspace makes an explicit errortag change, > but I'd have to play around with it a little bit. Hm? > > Anyways, given the fstests config matrix concern I'm inclined to at > least give something like that a try first and then fall back to a > custom test if that fails or is objectionable for some other reason.. > > Brian > > Here's a prototype for 1. an errtag quiet mode and 2. on-by-default tags. The alternative to a per-mount flag would be to hack a new struct into m_errortag that holds the current randfactor as well as a per-tag quiet flag, though I'm not sure how much people care about that. I didn't really plan on exposing this to userspace or anything for per-tag support, but this does mean all tags would start to warn once userspace changes any tag. I suppose that could become noisy if some day we end up with a bunch more default enabled tags. *shrug* I could go either way. Otherwise I think this would allow conversion of the two open coded get_random_u32_below() cases and the new force zero tag into on-by-default errortags. Any thoughts? --- 8< --- diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c index dbd87e137694..54b38143a7a6 100644 --- a/fs/xfs/xfs_error.c +++ b/fs/xfs/xfs_error.c @@ -69,6 +69,7 @@ static unsigned int xfs_errortag_random_default[] = { struct xfs_errortag_attr { struct attribute attr; unsigned int tag; + bool enable_default; }; static inline struct xfs_errortag_attr * @@ -129,12 +130,15 @@ static const struct sysfs_ops xfs_errortag_sysfs_ops = { .store = xfs_errortag_attr_store, }; -#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \ +#define __XFS_ERRORTAG_ATTR_RW(_name, _tag, enable) \ static struct xfs_errortag_attr xfs_errortag_attr_##_name = { \ .attr = {.name = __stringify(_name), \ .mode = VERIFY_OCTAL_PERMISSIONS(S_IWUSR | S_IRUGO) }, \ .tag = (_tag), \ + .enable_default = enable, \ } +#define XFS_ERRORTAG_ATTR_RW(_name, _tag) \ + __XFS_ERRORTAG_ATTR_RW(_name, _tag, false) #define XFS_ERRORTAG_ATTR_LIST(_name) &xfs_errortag_attr_##_name.attr @@ -240,6 +244,25 @@ static const struct kobj_type xfs_errortag_ktype = { .default_groups = xfs_errortag_groups, }; +static void +xfs_errortag_init_enable_defaults( + struct xfs_mount *mp) +{ + int i; + + for (i = 0; xfs_errortag_attrs[i]; i++) { + struct xfs_errortag_attr *xfs_attr = + to_attr(xfs_errortag_attrs[i]); + + if (!xfs_attr->enable_default) + continue; + + xfs_set_quiet_errtag(mp); + mp->m_errortag[xfs_attr->tag] = + xfs_errortag_random_default[xfs_attr->tag]; + } +} + int xfs_errortag_init( struct xfs_mount *mp) @@ -251,6 +274,8 @@ xfs_errortag_init( if (!mp->m_errortag) return -ENOMEM; + xfs_errortag_init_enable_defaults(mp); + ret = xfs_sysfs_init(&mp->m_errortag_kobj, &xfs_errortag_ktype, &mp->m_kobj, "errortag"); if (ret) @@ -320,9 +345,11 @@ xfs_errortag_test( if (!randfactor || get_random_u32_below(randfactor)) return false; - xfs_warn_ratelimited(mp, + if (!xfs_is_quiet_errtag(mp)) { + xfs_warn_ratelimited(mp, "Injecting error (%s) at file %s, line %d, on filesystem \"%s\"", expression, file, line, mp->m_super->s_id); + } return true; } @@ -346,6 +373,7 @@ xfs_errortag_set( if (!xfs_errortag_valid(error_tag)) return -EINVAL; + xfs_clear_quiet_errtag(mp); mp->m_errortag[error_tag] = tag_value; return 0; } diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index d85084f9f317..44b02728056f 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -558,6 +558,8 @@ __XFS_HAS_FEAT(nouuid, NOUUID) */ #define XFS_OPSTATE_BLOCKGC_ENABLED 6 +/* Debug kernel skips warning on errtag event triggers */ +#define XFS_OPSTATE_QUIET_ERRTAG 7 /* Kernel has logged a warning about shrink being used on this fs. */ #define XFS_OPSTATE_WARNED_SHRINK 9 /* Kernel has logged a warning about logged xattr updates being used. */ @@ -600,6 +602,7 @@ __XFS_IS_OPSTATE(inode32, INODE32) __XFS_IS_OPSTATE(readonly, READONLY) __XFS_IS_OPSTATE(inodegc_enabled, INODEGC_ENABLED) __XFS_IS_OPSTATE(blockgc_enabled, BLOCKGC_ENABLED) +__XFS_IS_OPSTATE(quiet_errtag, QUIET_ERRTAG) #ifdef CONFIG_XFS_QUOTA __XFS_IS_OPSTATE(quotacheck_running, QUOTACHECK_RUNNING) __XFS_IS_OPSTATE(resuming_quotaon, RESUMING_QUOTAON)