Re: [Devel] [PATCH vz10 v3] fs/ext4: let FALLOC_FL_PREALLOCATE through the vfs_fallocate mode switch

[Alexey Kuznetsov <kuznet@virtuozzo.com>]

Hello!

1. Probably. This is our private patch, we do not use any ext4 attrs
    and can leave behaviour as it is.
    The only target criterium is to keep the patch as small as possible.
    Actually, it is the reason why this patch is not going to upsteam ever,
    I have not much life time left to argue with anal freaks in
linux-kernel. :-)

    It is all right to keep it as it is.

2. It is OK. FALLOC_FL_ZERO_RANGE | FALLOC_FL_PREALLOCATE combination
    should be rejected, FALLOC_FL_PREALLOCATE is mode value, not a
flag modifier.
    Frankly it really makes sense with ZERO_RANGE. But change of status
    of the bit from mode to misc requires some todying up, which is
not reasonable.

3. This is very serious. Yes, we have to change current value. No hurry though,
    this can be done in the next release. While this patch must go to 8.0
    and we defintely do not want such trouble now. BTW was 7.4 released
    with 6.12?

    I would suggest to add the second flag and support them as the same,
    then we have grace time to wash out old value before it gets reused.
    Objections?



On Mon, Sep 7, 2026 at 11:41 PM Konstantin Khorenko
<khorenko@virtuozzo.com> wrote:
>
> 1. Append-only files, pre-existing.
>
> A few lines below the switch the patch touched, vfs_fallocate() still has:
>
>       /*
>        * On append-only files only space preallocation is supported.
>        */
>       if ((mode & ~FALLOC_FL_KEEP_SIZE) && IS_APPEND(inode))
>               return -EPERM;
>
> so fallocate(0) works on an append-only file while
> fallocate(FALLOC_FL_PREALLOCATE) gets EPERM, even though the comment
> says space preallocation is the thing that is allowed and that is
> exactly what the new mode does.
>
> vz9 behaved the same way, so this is not a regression, but probably need
> to fix it?
>
> 2. One flag combination stopped working.
>
>       fallocate(fd, FALLOC_FL_ZERO_RANGE | FALLOC_FL_PREALLOCATE, ...)
>
> worked in vz9 and returns EOPNOTSUPP now. That mode value is 0x90, and
> since both bits sit in FALLOC_FL_MODE_MASK the switch sees it as one
> unknown mode and takes the default label. vz9 had no such switch and
> ZERO_RANGE had no exclusivity check of its own, so the combination
> passed, and ext4_fallocate() carried the whole mode down through
> ext4_zero_range() into ext4_alloc_file_blocks(), where the flag is read.
>
> Nothing is broken today, csd passes the flag on its own:
>
>       fallocate(req->inplace_wr_fd,
>                 j->use_swapext ? FALLOC_FL_PREALLOCATE : 0, ...)
>
> but zero range together with preallocation is no longer expressible.
>
> 3. Upstream has taken bit 0x80 in 6.17.
>
> Commit 7bd43cc79cab ("fs: introduce FALLOC_FL_WRITE_ZEROES"), released
> in v6.17, defines
>
>       #define FALLOC_FL_WRITE_ZEROES          0x80
>
> adds it to FALLOC_FL_MODE_MASK, and ext4 implements it. Upstream also
> puts it in the group of that same switch which refuses
> FALLOC_FL_KEEP_SIZE, next to collapse and insert range, while the mode
> added here sits in the group that allows KEEP_SIZE.
>
> On a rebase to v6.17 or later the two are one value in one switch.
> Keeping both case labels does not build, the switch then has a
> duplicate case value. Dropping either label silently changes the
> meaning of a mode: drop ours and the feature quietly disappears again,
> which is the very bug this patch is fixing; drop theirs and a
> FALLOC_FL_WRITE_ZEROES request gets unwritten extents instead of zeroed
> blocks, with KEEP_SIZE accepted where upstream refuses it.
>
> Adopting the upstream mode instead of ours does not get us out of it.
> It reaches ext4_alloc_file_blocks() through ext4_zero_range(), where
>
>       if (len_lblk <= EXT_UNWRITTEN_MAX_LEN)
>               flags |= EXT4_GET_BLOCKS_NO_NORMALIZE;
>
> is unchanged, so it loses mballoc preallocation exactly like plain
> fallocate and does nothing for us. It also does real zeroing plus
> conversion to written, requires the device to report unmap write
> zeroes, and refuses KEEP_SIZE.
>
> So the way out is moving our flag to a free high bit, and yes, that
> needs a coordinated userspace rebuild. csd_journal.c carries its own
> copy of the value:
>
>       #ifndef FALLOC_FL_PREALLOCATE
>       #define FALLOC_FL_PREALLOCATE          0x80
>       #endif
>
> A rebuild against a header without the name would keep sending 0x80,
> which on v6.17 is someone else's mode, so the renumbering has to remove
> that block too.
>
> --
> Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel