Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

[Christoph Hellwig]

I just noticed this test failing on zoned xfs in current for-next.

That's because for out of place overwrite file systems writing at
ENOSPC will obviously fail, and I think the test acknowledges that
by forcing nocow for btrfs.

But that leaves "real" out of place write file systems affected, which
should also include zone btrfs, but the test actually fails there
in mkfs already due to some reason.

Can you please rework the patch to see that setting the nocow flag
works first and only try with that or something like that?

Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

[Johannes Thumshirn]

On 22.07.25 08:56, Christoph Hellwig wrote:
> I just noticed this test failing on zoned xfs in current for-next.
> 
> That's because for out of place overwrite file systems writing at
> ENOSPC will obviously fail, and I think the test acknowledges that
> by forcing nocow for btrfs.
> 
> But that leaves "real" out of place write file systems affected, which
> should also include zone btrfs, but the test actually fails there
> in mkfs already due to some reason.

It probably fails the _try_scratch_mkfs_sized() call because that goes 
into _check_minimal_fs_size() and that should _notrun if $fssize < 
$MIN_FSSIZE.

> Can you please rework the patch to see that setting the nocow flag
> works first and only try with that or something like that?
> 
> 

Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

[hch]

On Tue, Jul 22, 2025 at 07:50:48AM +0000, Johannes Thumshirn wrote:
> On 22.07.25 08:56, Christoph Hellwig wrote:
> > I just noticed this test failing on zoned xfs in current for-next.
> > 
> > That's because for out of place overwrite file systems writing at
> > ENOSPC will obviously fail, and I think the test acknowledges that
> > by forcing nocow for btrfs.
> > 
> > But that leaves "real" out of place write file systems affected, which
> > should also include zone btrfs, but the test actually fails there
> > in mkfs already due to some reason.
> 
> It probably fails the _try_scratch_mkfs_sized() call because that goes 
> into _check_minimal_fs_size() and that should _notrun if $fssize < 
> $MIN_FSSIZE.

Btw, it would be really helpful to improve the test coverage if btrfs did
the same adjustment for the nimimum size as zone xfs, where mkfs run up 
the requested size to the zone size and then adds the required extra
zones that don't show up in the user capacity.  That way all the small
size based tests just work.

Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

[Filipe Manana]

On Tue, Jul 22, 2025 at 7:56 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> I just noticed this test failing on zoned xfs in current for-next.
>
> That's because for out of place overwrite file systems writing at
> ENOSPC will obviously fail, and I think the test acknowledges that
> by forcing nocow for btrfs.
>
> But that leaves "real" out of place write file systems affected, which
> should also include zone btrfs, but the test actually fails there
> in mkfs already due to some reason.

If the mkfs fails, it's probably _small_fs_size_mb that needs to be
updated with a sane size for zoned btrfs.
Johannes at Aota could give some advice there.

>
> Can you please rework the patch to see that setting the nocow flag
> works first and only try with that or something like that?

Reworking it is late as it's already in for-next, but we can add a
patch to skip it on zoned xfs:

1) The quickest way would be to add to the test:
_require_non_zoned_device $SCRATCH_DEV

2) Or add a "_require_nocow_data_writes" helper to check we can write
in place and skip the test if not, as you suggest, as it's more
generic in case there are other filesystems or configurations where
data writes are always COWed.

Re: [PATCH v2] generic: test overwriting file with mmap on a full filesystem

[Christoph Hellwig]

On Tue, Jul 22, 2025 at 11:27:35AM +0100, Filipe Manana wrote:
> > Can you please rework the patch to see that setting the nocow flag
> > works first and only try with that or something like that?
> 
> Reworking it is late as it's already in for-next, but we can add a
> patch to skip it on zoned xfs:

It though I explained it before, but let me try again.

This is not about zoned xfs, zoned xfs is just the canary in the coal
mine.

The test fundamentally assumes file systems can overwrite without
space allocations.  And then noticed that this isn't true with a weird
btrfs hack.

It needs to be reworked to only run when that is known to be true.

> 
> 1) The quickest way would be to add to the test:
> _require_non_zoned_device $SCRATCH_DEV

That test is not relevant here.  While zoned device require out of place
updates, they are also common for many other cases.

> 2) Or add a "_require_nocow_data_writes" helper to check we can write
> in place and skip the test if not, as you suggest, as it's more
> generic in case there are other filesystems or configurations where
> data writes are always COWed.

That's the only thing that works.  Only run on file systems that are
known to do in-place updates, or in the odd btrfs case can be forced to
even if they don't normally do it.

To be honest the hardcoded btrfs hack should have been a big red flag,
those almost always means the test is fishy.