All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
From: Konstantin Khorenko <khorenko@virtuozzo.com>
To: Alexey Kuznetsov <kuznet@virtuozzo.com>
Cc: OpenVZ devel list <devel@openvz.org>,
	"Denis V. Lunev" <den@virtuozzo.com>
Subject: Re: [Devel] [PATCH vz10 v3] fs/ext4: let FALLOC_FL_PREALLOCATE through the vfs_fallocate mode switch
Date: Tue, 08 Sep 2026 10:26:22 +0200	[thread overview]
Message-ID: <178885598282.1734901.13689352465078529050.b4-reply@b4> (raw)
In-Reply-To: <CAMrnHh4LngkuaiXRb2nnM4tRtVcgG8U2gv2VMMJi3xp+DXr0SQ@mail.gmail.com>

On 2026-09-08 00:26 +0800, Alexey Kuznetsov wrote:
> 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.

OK.

> 
> 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.

OK.

> 
> 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?

i suggest the following scheme:
- we will keep 2 flags during a grace period in order to keep the old userspace
  + new kernel working

- we add a new FALLOC_FL_PREALLOCATE2 flag with a high value

- userspace does a probe on a start - tries to fallocate (FALLOC_FL_PREALLOCATE2),
if it returns -ENOTSUP, it falls back to use old FALLOC_FL_PREALLOCATE == 0x80.
If it passes - ok, we have a "new" kernel which supports a new number.

  int fd = open("/dev/null", O_RDONLY);
  rc = fallocate(fd, FALLOC_FL_PREALLOCATE2, 0, 1);
  /* EOPNOTSUPP -> the kernel does not know the flag; EBADF -> knows */


Alternative scheme:
the kernel publishes the number itself. falloc_prealloc is already exported as a
module_param (fs/ext4/extents.c:39), so it shows up as
/sys/module/ext4/parameters/falloc_prealloc. Nothing stops us from adding a
read-only parameter next to it holding the mode value:

  static const unsigned int falloc_prealloc_mode = FALLOC_FL_PREALLOCATE;
  module_param(falloc_prealloc_mode, uint, 0444);

Userspace then reads the number instead of guessing it: no file means no
feature, so send mode 0; the file is there, so send whatever it says.

https://virtuozzo.atlassian.net/browse/VSTOR-144521
Lesha, i have created a task for the kernel part.

Can you please handle both kernel and userspace part with any scheme you prefer,
but so they are in sync?

> 
> 
> 
> 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


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

  reply	other threads:[~2026-09-08  8:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 14:10 Konstantin Khorenko
2026-09-07 15:29 ` Konstantin Khorenko
2026-09-07 16:26   ` Alexey Kuznetsov
2026-09-08  8:26     ` Konstantin Khorenko [this message]
2026-09-08  8:28 ` Konstantin Khorenko

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=178885598282.1734901.13689352465078529050.b4-reply@b4 \
    --to=khorenko@virtuozzo.com \
    --cc=den@virtuozzo.com \
    --cc=devel@openvz.org \
    --cc=kuznet@virtuozzo.com \
    /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.