Re: [PATCH 0/3] vmsplice: make vmsplice a trivial wrapper for preadv2/pwritev2
Andrei Vagin <[email protected]> Wed, 24 Jun 2026 10:52:10 -0700
| Newsgroups | dev.linux.lists.criu,dev.linux.lists.fuse-devel,dev.linux.lists.patches,org.kernel.vger.linux-api,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kvack.linux-mm |
|---|---|
| Message-ID | <CANaxB-xUrLQYGiRJZc4Boi+KX=0TJSWymErNovANVko20fMDVA@mail.gmail.com> |
On Wed, Jun 24, 2026 at 12:12=E2=80=AFAM Askar Safin <[email protected]>= wrote: > > Andrei Vagin <[email protected]>: > > The CRIU fifo test fails with this change. The problem is that vmsplice > > with SPLICE_F_NONBLOCK to a fifo file descriptor fails with -EOPNOTSUPP= . > > > > It seems we need a fix like this one: > > > > diff --git a/fs/pipe.c b/fs/pipe.c > > index 429b0714ec57..6fc49e933727 100644 > > --- a/fs/pipe.c > > +++ b/fs/pipe.c > > @@ -1253,6 +1253,7 @@ static int fifo_open(struct inode *inode, struct > > file *filp) > > > > /* We can only do regular read/write on fifos */ > > stream_open(inode, filp); > > + filp->f_mode |=3D FMODE_NOWAIT; > > > > switch (filp->f_mode & (FMODE_READ | FMODE_WRITE)) { > > case FMODE_READ: > > Does CRIU actually rely on ability to do SPLICE_F_NONBLOCK vmsplice into > named fifos? Or this is merely a test? Yes, it does. > > If this is just a test, I think we need not to preserve this behavior. > > I did debian code search with regex "vmsplice.*SPLICE_F_NONBLOCK" and I > found very few packages. And it seems all them use pipes, not named fifos= . In short, this isn't how such cases are handled in the kernel. The fix is simple and should be applied to avoid breaking random software. > > (On speed: I still think that my vmsplice patches are good thing, > despite performance regressions in CRIU.) I already explained that this isn't just a perfomance degradation, it actually breaks the pre-dump mechanism in CRIU. vmsplice is invoked from our parasite code within the context of a user process, where execution speed is critical. A heavy performance penalty completely invalidates the pre-dump logic, making the feature useless. Under normal circumstances, patches that cause this kind of breakage would never be merged. However, since there are exceptions to every rule, we should let the maintainers decide how to proceed here. In CRIU, we have a backup plan to utilize process_vm_readv to dump process memory. We already support this mode, but it isn't the default due to performance concerns. If these patches are merged, it will be the only option left for CRIU to implement pre-dumping. However, we need to look at this case in a broader context. This is yet another example where the change introduces a workflow breakage, meaning there might be other workloads out there that could be broken by this change. At a minimum, we may need to consider a deprecation plan where vmsplice with SPLICE_F_GIFT triggers a warning for a few releases before these changes are applied. Alternatively, we could introduce the proposed behavior alongside a sysctl to fall back to the old behavior and explicitly state that this fallback path will be completely deprecated in a future ker= nel version. Thanks, Andrei