OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox