All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: cem@kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: fix XFS_ERRTAG_FORCE_ZERO_RANGE for zoned file system
Date: Wed, 10 Dec 2025 12:14:35 -0500	[thread overview]
Message-ID: <aTmqe3lDL2BkZe3b@bfoster> (raw)
In-Reply-To: <20251210154016.GA3851@lst.de>

On Wed, Dec 10, 2025 at 04:40:16PM +0100, Christoph Hellwig wrote:
> On Wed, Dec 10, 2025 at 10:36:55AM -0500, Brian Foster wrote:
> > Is there a reason in particular for testing this with the zone mode?
> > It's just a DEBUG thing for the zeroing mechanism. Why not just filter
> > out the is_zoned_inode() case at the injection site?
> 
> Because I also want to be able to test the zeroing code for zoned
> file systems, especially given zeroing is a bit of painful area
> for out of place write file systems like zoned XFS.
> 
> > I suppose you could argue there is a point if we have separate zoned
> > mode iomap callbacks and whatnot, but I agree the factoring here is a
> > little unfortunate. I wonder if it would be nicer if we could set a flag
> > or something on an ac and toggle the zone mode off that, but on a quick
> > look I don't see a flag field in the zone ctx.
> 
> I don't really follow what you mean here.
> 

I was just rambling about if/how we might be able to use the ac..

> > Hmm.. I wonder if we could still do something more clever where the zone
> > mode has its own injection site to bump the res, and then the lower
> > level logic just checks whether the reservation is sufficient for a full
> > zero..? I'm not totally sure if that's ultimately cleaner, but maybe
> > worth a thought..
> 
> We could have a different site for that injection, but we'd still need
> to move the current one or at least make it conditional so that it
> can't trigger for zoned mode.  I doubt that's less ugly than this
> version.
> 

Well yeah, it would look something like this at the current site:

	if (!is_inode_zoned() && XFS_TEST_ERROR(...) ||
	    ac->reserved_blocks == magic_default_res + len)
		xfs_zero_range(...);
	else
		xfs_free_file_space(...);

... and the higher level zoned code would clone the XFS_TEST_ERROR() to
create the block reservation condition to trigger it.

Alternatively perhaps you could make that check look something like:

	if (XFS_TEST_ERROR() && (!ac || ac->res > len))
		...
	else
		...

... and let the res side always bump the res in DEBUG mode, with a
fallback on -ENOSPC or something.

Actually the latter sounds potentially more clean to me, but I don't
object to this if not.

Brian


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

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-10  9:03 [PATCH] xfs: fix XFS_ERRTAG_FORCE_ZERO_RANGE for zoned file system Christoph Hellwig
2025-12-10 15:36 ` Brian Foster
2025-12-10 15:40   ` Christoph Hellwig
2025-12-10 17:14     ` Brian Foster [this message]
2025-12-10 17:18       ` Christoph Hellwig
2025-12-12  7:39       ` Christoph Hellwig
2025-12-12 12:24         ` Brian Foster

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=aTmqe3lDL2BkZe3b@bfoster \
    --to=bfoster@redhat.com \
    --cc=cem@kernel.org \
    --cc=hch@lst.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.