linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
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
Date: Tue, 10 Jun 2025 10:20:28 -0400	[thread overview]
Message-ID: <aEg_LH2BelAnY7It@bfoster> (raw)
In-Reply-To: <aEgzdZKtL2Sp5RRa@infradead.org>

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


  reply	other threads:[~2025-06-10 14:17 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05 17:33 [PATCH 0/7] iomap: zero range folio batch support Brian Foster
2025-06-05 17:33 ` [PATCH 1/7] iomap: move pos+len BUG_ON() to after folio lookup Brian Foster
2025-06-09 16:16   ` Darrick J. Wong
2025-06-10  4:20     ` Christoph Hellwig
2025-06-10 12:16       ` Brian Foster
2025-06-05 17:33 ` [PATCH 2/7] filemap: add helper to look up dirty folios in a range Brian Foster
2025-06-09 15:48   ` Darrick J. Wong
2025-06-10  4:21     ` Christoph Hellwig
2025-06-10 12:17     ` Brian Foster
2025-06-10  4:22   ` Christoph Hellwig
2025-06-05 17:33 ` [PATCH 3/7] iomap: optional zero range dirty folio processing Brian Foster
2025-06-09 16:04   ` Darrick J. Wong
2025-06-10  4:27     ` Christoph Hellwig
2025-06-10 12:21       ` Brian Foster
2025-06-10 12:21     ` Brian Foster
2025-06-10 13:29       ` Christoph Hellwig
2025-06-10 14:19         ` Brian Foster
2025-06-11  3:54           ` Christoph Hellwig
2025-06-10 14:55       ` Darrick J. Wong
2025-06-11  3:55         ` Christoph Hellwig
2025-06-12  4:06           ` Darrick J. Wong
2025-06-10  4:27   ` Christoph Hellwig
2025-06-05 17:33 ` [PATCH 4/7] xfs: always trim mapping to requested range for zero range Brian Foster
2025-06-09 16:07   ` Darrick J. Wong
2025-06-05 17:33 ` [PATCH 5/7] xfs: fill dirty folios on zero range of unwritten mappings Brian Foster
2025-06-06  2:02   ` kernel test robot
2025-06-06 15:20     ` Brian Foster
2025-06-09 16:12   ` Darrick J. Wong
2025-06-10  4:31     ` Christoph Hellwig
2025-06-10 12:24     ` Brian Foster
2025-07-02 18:50       ` Darrick J. Wong
2025-06-05 17:33 ` [PATCH 6/7] iomap: remove old partial eof zeroing optimization Brian Foster
2025-06-10  4:32   ` Christoph Hellwig
2025-06-05 17:33 ` [PATCH RFC 7/7] xfs: error tag to force zeroing on debug kernels Brian Foster
2025-06-10  4:33   ` Christoph Hellwig
2025-06-10 12:26     ` Brian Foster
2025-06-10 13:30       ` Christoph Hellwig
2025-06-10 14:20         ` Brian Foster [this message]
2025-06-10 19:12           ` Brian Foster
2025-06-11  3:56             ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aEg_LH2BelAnY7It@bfoster \
    --to=bfoster@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).