Skip to contentSkip to conversation

splice: remove pipe buffer stealing

PATCH In review Kiryl Shutsemau (Meta) 7 participants 16 messages Nov 10 – Nov 11

Subsystems (9)
lore
Share & tools
lore
#0 lore raw

cover splice: remove pipe buffer stealing

->try_steal() lets the consumer of a pipe buffer take its page instead of copying the data. Two consumers use it:

- fuse, when a server splices a read reply into /dev/fuse with

    SPLICE_F_MOVE: the page goes into the fuse file's page cache;

- virtio_console, which passes the page to the host.

The page can come from the page cache of another file, from a write() into the pipe, or from vmsplice(SPLICE_F_GIFT). Stealing it takes page cache folios out of their files, and turns pages into page cache folios and back. That gets in the way of reading the page cache without folio references [1] and of telling folios apart with a PG_folio flag. It works only on pages that nothing else holds, for fuse only on clean, unmapped, order-0 ones, and it has needed fixes again and again:

32f98877c57b ("fuse: don't check refcount after stealing page")
b89ecd60d38e ("fuse: ignore PG_workingset after stealing")
712a951025c0 ("fuse: fix page stealing")
59ba47b6be9c ("fuse: Check for large folio with SPLICE_F_MOVE")
15c20483d915 ("fuse: validate folio state before pipe_buf_try_steal() in fuse_try_move_folio()")

Linus and David suggested removing it [2]. Both consumers already copy when a steal fails; with this series they always do. vmsplice() and splice() still accept SPLICE_F_GIFT and SPLICE_F_MOVE, and fuse still advertises FUSE_SPLICE_MOVE, so that servers which ask for it mount as before.

Miklos, Amit: do you know of users of the fuse move, or of virtio_console splice, that depend on the copy being avoided?

Without ->try_steal(), nosteal_pipe_buf_ops is the same as default_pipe_buf_ops, and the last patch keeps only the latter.

Tested on x86_64 with a FUSE server that answers reads by splicing the page cache pages of a backing file into /dev/fuse with SPLICE_F_MOVE. On v7.4-rc1 how much moves depends on folio sizes: with order-0 folios on both sides, one read of a 16M fuse file took 3968 of the backing file's 4096 pages out of its page cache. With the series nothing moves, and the data read through fuse is the same, also when the server reads its requests with splice(). A virtio-serial port fed by splice() from a pipe and from a file, and vmsplice(SPLICE_F_GIFT), SPLICE_F_MOVE, tee() and AF_UNIX socket splice checks also give the same data before and after.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/CAHk-=whhp=Q8qR+wPoM9+mxYPQ_OXMOEoKmT7J0JZikFZusuqw@mail.gmail.com/

Kiryl Shutsemau (Meta) (4):
virtio_console: copy pipe buffers in splice_write()
fuse: don't move pipe buffers into the page cache
splice: remove pipe buffer stealing
splice: replace nosteal_pipe_buf_ops with default_pipe_buf_ops

 drivers/char/virtio_console.c |  39 ++++-----
 fs/fuse/args.h                |   1 -
 fs/fuse/dev.c                 | 146 +---------------------------------
 fs/fuse/file.c                |   1 -
 fs/fuse/fuse_dev_i.h          |   1 -
 fs/pipe.c                     |  43 ----------
 fs/splice.c                   | 104 ++----------------------
 include/linux/pipe_fs_i.h     |  38 ---------
 kernel/watch_queue.c          |   4 -
 mm/shmem.c                    |   7 --
 net/core/skbuff.c             |   2 +-
 11 files changed, 26 insertions(+), 360 deletions(-)

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.54.0
#1 lore raw

PATCH 1/4 virtio_console: copy pipe buffers in splice_write()

pipe_to_sg() tries to steal the page of each pipe buffer and pass it to the host as it is. When the steal fails, it copies the data into a page of its own.

Pages written into the pipe with write(), gifted user pages and page cache pages can be stolen. The ftrace ring buffer pages that virtio-trace splices cannot, so that in-tree user always gets the copy.

Always copy. The data that reaches the host does not change.

Preparation for removing ->try_steal() from pipe buffers.

---
 drivers/char/virtio_console.c | 39 +++++++++++++----------------------
 1 file changed, 14 insertions(+), 25 deletions(-)
drivers/char/virtio_console.c +14 -25
index 019bcae81af5..b1b1aaf43e07 100644
@@ -891,45 +891,34 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
 {
 	struct sg_list *sgl = sd->u.data;
 	unsigned int offset, len;
+	struct page *page;
+	char *src;
 
 	if (sgl->n == sgl->size)
 		return 0;
 
-	/* Try lock this page */
-	if (pipe_buf_try_steal(pipe, buf)) {
-		/* Get reference and unlock page for moving */
-		get_page(buf->page);
-		unlock_page(buf->page);
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
 
-		len = min(buf->len, sd->len);
-		sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
-	} else {
-		/* Failback to copying a page */
-		struct page *page = alloc_page(GFP_KERNEL);
-		char *src;
+	offset = sd->pos & ~PAGE_MASK;
 
-		if (!page)
-			return -ENOMEM;
+	len = sd->len;
+	if (len + offset > PAGE_SIZE)
+		len = PAGE_SIZE - offset;
 
-		offset = sd->pos & ~PAGE_MASK;
+	src = kmap_local_page(buf->page);
+	memcpy(page_address(page) + offset, src + buf->offset, len);
+	kunmap_local(src);
 
-		len = sd->len;
-		if (len + offset > PAGE_SIZE)
-			len = PAGE_SIZE - offset;
-
-		src = kmap_local_page(buf->page);
-		memcpy(page_address(page) + offset, src + buf->offset, len);
-		kunmap_local(src);
-
-		sg_set_page(&(sgl->sg[sgl->n]), page, len, offset);
-	}
+	sg_set_page(&sgl->sg[sgl->n], page, len, offset);
 	sgl->n++;
 	sgl->len += len;
 
 	return len;
 }
 
-/* Faster zero-copy write by splicing */
+/* Write by splicing, copying the data from the pipe */
 static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
 				      struct file *filp, loff_t *ppos,
 				      size_t len, unsigned int flags)
Signature · 1 line
2.54.0
Acked-by on v1 1/4
#2 lore raw

Re: [PATCH 1/4] virtio_console: copy pipe buffers in splice_write()

Quoted 10 lines · Kiryl Shutsemau (Meta)

pipe_to_sg() tries to steal the page of each pipe buffer and pass it to the host as it is. When the steal fails, it copies the data into a page of its own.

Pages written into the pipe with write(), gifted user pages and page cache pages can be stolen. The ftrace ring buffer pages that virtio-trace splices cannot, so that in-tree user always gets the copy.

Always copy. The data that reaches the host does not change.

Quoted 8 lines

Preparation for removing ->try_steal() from pipe buffers.

---
 drivers/char/virtio_console.c | 39 +++++++++++++----------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

LGTM.

Best Regards,
Yan, Zi

#3 lore raw

Re: [PATCH 1/4] virtio_console: copy pipe buffers in splice_write()

In reply to Kiryl Shutsemau (Meta)

drivers/char/virtio_console.c static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf, · line 912

Quoted 13 lines · Kiryl Shutsemau (Meta)
drivers/char/virtio_console.c +10 -0
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
 
+	offset = sd->pos & ~PAGE_MASK;
 
+	len = sd->len;
+	if (len + offset > PAGE_SIZE)
+		len = PAGE_SIZE - offset;
 
+	src = kmap_local_page(buf->page);
+	memcpy(page_address(page) + offset, src + buf->offset, len);
+	kunmap_local(src);
	memcpy_from_page(page_address(page) + offset, buf->page, buf->offset,
			len);
Quoted 6 lines
drivers/char/virtio_console.c +1 -0
+	sg_set_page(&sgl->sg[sgl->n], page, len, offset);
 	sgl->n++;
 	sgl->len += len;
 
 	return len;
 }
#4 lore raw

Re: [PATCH 1/4] virtio_console: copy pipe buffers in splice_write()

Quoted 7 lines · Matthew Wilcox

drivers/char/virtio_console.c static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf, · line 912

Quoted 3 lines · Kiryl Shutsemau (Meta)
drivers/char/virtio_console.c +3 -0
+	src = kmap_local_page(buf->page);
+	memcpy(page_address(page) + offset, src + buf->offset, len);
+	kunmap_local(src);
	memcpy_from_page(page_address(page) + offset, buf->page, buf->offset,
			len);

I don't think that works. pipe_to_sg() only clamps len to the destination page. A pipe buffer however can span multiple pages of a compound page iircu. And buf->offset can point past the first one.

I found two exmples:

(1) skb_splice_bits() for AF_UNIX and TCP
(2) tracing_buffers_splice_read() with buffer_subbuf_size_kb > 4,

And virtio-trace splices that stuff.

Which means VM_BUG_ON() in memcpy_from_page() fires:

kernel BUG at include/linux/highmem.h:542!

Oops: invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:pipe_to_sg+0x130/0x150
Call Trace:
 <TASK>
 __splice_from_pipe+0x92/0x1e0
 port_fops_splice_write+0x13f/0x1a0
 do_splice+0x3b6/0x8e0
 __do_splice+0x20a/0x230
 __x64_sys_splice+0x97/0x130
 do_syscall_64+0xde/0x4b0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fwiw, the open-coded copy is broken as well. Just on HIGHMEM...

Starting with b46e09667411 ("fuse: enable large folios") fuse_dev_splice_read() hands out large page cache folios and kmap_local_page() only maps the first page.

So a fuse server on a 32-bit HIGHMEM guest that splices a WRITE request into a virtio port crashes mainline:

BUG: unable to handle page fault for address: fffab000
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
EIP: pipe_to_sg+0x184/0x2b0
Call Trace:
 __splice_from_pipe+0xd6/0x1f0
 port_fops_splice_write+0xf9/0x170
 do_splice+0x2c1/0x810
 __ia32_sys_splice+0x2e7/0x310

So we need to copy per page which would fix both.

#5 lore raw

Re: [PATCH 1/4] virtio_console: copy pipe buffers in splice_write()

In reply to Kiryl Shutsemau (Meta)
Sashiko has reviewed this patch and found no issues.
Quoted 8 lines

pipe_to_sg() tries to steal the page of each pipe buffer and pass it to the host as it is. When the steal fails, it copies the data into a page of its own.

Pages written into the pipe with write(), gifted user pages and page cache pages can be stolen. The ftrace ring buffer pages that virtio-trace splices cannot, so that in-tree user always gets the copy.

Quoted 6 lines

Always copy. The data that reaches the host does not change.

Preparation for removing ->try_steal() from pipe buffers.

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#6 lore raw

PATCH 2/4 fuse: don't move pipe buffers into the page cache

In reply to Kiryl Shutsemau (Meta)

A splice() into /dev/fuse with SPLICE_F_MOVE lets the reply to a read request steal full pages from the pipe. Each stolen page replaces, in the fuse file's page cache, the folio that the request allocated. The page can be a page cache folio of another file, a page written into the pipe, or a gifted user page.

Always copy. The server sees the same data, and the move was a hint that splice() never promised to honour.

Keep advertising FUSE_SPLICE_MOVE. libfuse fails the mount of a filesystem that asks for a capability the kernel does not offer.

Preparation for removing ->try_steal() from pipe buffers.

---
 fs/fuse/args.h       |   1 -
 fs/fuse/dev.c        | 144 +------------------------------------------
 fs/fuse/file.c       |   1 -
 fs/fuse/fuse_dev_i.h |   1 -
 4 files changed, 3 insertions(+), 144 deletions(-)
fs/fuse/args.h +0 -1
index 5173264a1261..36ef0c6d5999 100644
@@ -36,7 +36,6 @@ struct fuse_args {
 	bool user_pages:1;
 	bool out_argvar:1;
 	bool page_zeroing:1;
-	bool page_replace:1;
 	bool may_block:1;
 	bool is_ext:1;
 	bool is_pinned:1;
fs/fuse/dev.c +3 -141

180 lines not on this page · raw message

fs/fuse/file.c +0 -1
index d5e1cb53d86a..ab955e4c3e5d 100644
@@ -1096,7 +1096,6 @@ static void fuse_send_readpages(struct fuse_io_args *ia, struct file *file,
 
 	ap->args.out_pages = true;
 	ap->args.page_zeroing = true;
-	ap->args.page_replace = true;
 
 	/*
 	 * Don't overflow end offset.
fs/fuse/fuse_dev_i.h +0 -1
index 4b412a76225f..a90ce26ded31 100644
@@ -326,7 +326,6 @@ struct fuse_copy_state {
 	unsigned int len;
 	unsigned int offset;
 	bool write:1;
-	bool move_folios:1;
 	bool is_uring:1;
 	/* set when the payload is zero-copied. folios are filled in place */
 	bool skip_folio_copy:1;
Signature · 1 line
2.54.0
Acked-by on v1 2/4
#7 lore raw

Re: [PATCH 2/4] fuse: don't move pipe buffers into the page cache

Quoted 17 lines · Kiryl Shutsemau (Meta)

A splice() into /dev/fuse with SPLICE_F_MOVE lets the reply to a read request steal full pages from the pipe. Each stolen page replaces, in the fuse file's page cache, the folio that the request allocated. The page can be a page cache folio of another file, a page written into the pipe, or a gifted user page.

Always copy. The server sees the same data, and the move was a hint that splice() never promised to honour.

Keep advertising FUSE_SPLICE_MOVE. libfuse fails the mount of a filesystem that asks for a capability the kernel does not offer.

Preparation for removing ->try_steal() from pipe buffers.

Quoted 7 lines
---
 fs/fuse/args.h       |   1 -
 fs/fuse/dev.c        | 144 +------------------------------------------
 fs/fuse/file.c       |   1 -
 fs/fuse/fuse_dev_i.h |   1 -
 4 files changed, 3 insertions(+), 144 deletions(-)

LGTM.

Best Regards,
Yan, Zi

#8 lore raw

Re: [PATCH 2/4] fuse: don't move pipe buffers into the page cache

In reply to Kiryl Shutsemau (Meta)
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines

A splice() into /dev/fuse with SPLICE_F_MOVE lets the reply to a read request steal full pages from the pipe. Each stolen page replaces, in the fuse file's page cache, the folio that the request allocated. The page can be a page cache folio of another file, a page written into the pipe, or a gifted user page.

Always copy. The server sees the same data, and the move was a hint that splice() never promised to honour.

Quoted 8 lines

Keep advertising FUSE_SPLICE_MOVE. libfuse fails the mount of a filesystem that asks for a capability the kernel does not offer.

Preparation for removing ->try_steal() from pipe buffers.

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
maintainer Acked-by on v1 2/4
#9 lore raw

Re: [PATCH 2/4] fuse: don't move pipe buffers into the page cache

In reply to Kiryl Shutsemau (Meta)
Quoted 9 lines · Kiryl Shutsemau (Meta)

A splice() into /dev/fuse with SPLICE_F_MOVE lets the reply to a read request steal full pages from the pipe. Each stolen page replaces, in the fuse file's page cache, the folio that the request allocated. The page can be a page cache folio of another file, a page written into the pipe, or a gifted user page.

Always copy. The server sees the same data, and the move was a hint that splice() never promised to honour.

Did some research and looks like this feature is not used by any major project.

Thanks,
Miklos

#10 lore raw

PATCH 3/4 splice: remove pipe buffer stealing

In reply to Kiryl Shutsemau (Meta)

Nothing calls pipe_buf_try_steal() any more. Remove ->try_steal() from struct pipe_buf_operations, its implementations and the exported generic_pipe_buf_try_steal().

PIPE_BUF_FLAG_LRU and PIPE_BUF_FLAG_GIFT were set only for stealing, and go too. page_cache_pipe_buf_release() did nothing but drop the page and the LRU flag, so page cache and user page buffers use generic_pipe_buf_release().

vmsplice() still accepts SPLICE_F_GIFT, and splice() SPLICE_F_MOVE. Both were hints, and nothing acts on them now.

---
 fs/pipe.c                 | 43 ------------------
 fs/splice.c               | 96 +++------------------------------------
 include/linux/pipe_fs_i.h | 36 ---------------
 kernel/watch_queue.c      |  4 --
 mm/shmem.c                |  7 ---
 5 files changed, 6 insertions(+), 180 deletions(-)
fs/pipe.c +0 -43
index 4fa69192e6fb..4c1122597a3c 100644
@@ -222,48 +222,6 @@ static void anon_pipe_buf_release(struct pipe_inode_info *pipe,
 	anon_pipe_put_page(pipe, page);
 }
 
-static bool anon_pipe_buf_try_steal(struct pipe_inode_info *pipe,
-		struct pipe_buffer *buf)
-{
-	struct page *page = buf->page;
-
-	if (page_count(page) != 1)
-		return false;
-	memcg_kmem_uncharge_page(page, 0);
-	__SetPageLocked(page);
-	return true;
-}
-
-/**
- * generic_pipe_buf_try_steal - attempt to take ownership of a &pipe_buffer
- * @pipe:	the pipe that the buffer belongs to
- * @buf:	the buffer to attempt to steal
- *
- * Description:
- *	This function attempts to steal the &struct page attached to
- *	@buf. If successful, this function returns 0 and returns with
- *	the page locked. The caller may then reuse the page for whatever
- *	he wishes; the typical use is insertion into a different file
- *	page cache.
- */
-bool generic_pipe_buf_try_steal(struct pipe_inode_info *pipe,
-		struct pipe_buffer *buf)
-{
-	struct page *page = buf->page;
-
-	/*
-	 * A reference of one is golden, that means that the owner of this
-	 * page is the only one holding a reference to it. lock the page
-	 * and return OK.
-	 */
-	if (page_count(page) == 1) {
-		lock_page(page);
-		return true;
-	}
-	return false;
-}
-EXPORT_SYMBOL(generic_pipe_buf_try_steal);
-
 /**
  * generic_pipe_buf_get - get a reference to a &struct pipe_buffer
  * @pipe:	the pipe that the buffer belongs to
@@ -297,7 +255,6 @@ EXPORT_SYMBOL(generic_pipe_buf_release);
 
 static const struct pipe_buf_operations anon_pipe_buf_ops = {
 	.release	= anon_pipe_buf_release,
-	.try_steal	= anon_pipe_buf_try_steal,
 	.get		= generic_pipe_buf_get,
 };
 
fs/splice.c +6 -90

151 lines not on this page · raw message

include/linux/pipe_fs_i.h +0 -36
index 6402930282e5..fb6570e80d01 100644
@@ -4,9 +4,7 @@
 
 #define PIPE_DEF_BUFFERS	16
 
-#define PIPE_BUF_FLAG_LRU	0x01	/* page is on the LRU */
 #define PIPE_BUF_FLAG_ATOMIC	0x02	/* was atomically mapped */
-#define PIPE_BUF_FLAG_GIFT	0x04	/* page is a gift */
 #define PIPE_BUF_FLAG_PACKET	0x08	/* read() as a packet */
 #define PIPE_BUF_FLAG_CAN_MERGE	0x10	/* can merge buffers */
 #define PIPE_BUF_FLAG_WHOLE	0x20	/* read() must return entire buffer or error */
@@ -127,16 +125,6 @@ struct pipe_inode_info {
 #endif
 };
 
-/*
- * Note on the nesting of these functions:
- *
- * ->confirm()
- *	->try_steal()
- *
- * That is, ->try_steal() must be called on a confirmed buffer.  See below for
- * the meaning of each operation.  Also see the kerneldoc in fs/pipe.c for the
- * pipe and generic variants of these hooks.
- */
 struct pipe_buf_operations {
 	/*
 	 * ->confirm() verifies that the data in the pipe buffer is there
@@ -153,16 +141,6 @@ struct pipe_buf_operations {
 	 */
 	void (*release)(struct pipe_inode_info *, struct pipe_buffer *);
 
-	/*
-	 * Attempt to take ownership of the pipe buffer and its contents.
-	 * ->try_steal() returns %true for success, in which case the contents
-	 * of the pipe (the buf->page) is locked and now completely owned by the
-	 * caller. The page may then be transferred to a different mapping, the
-	 * most often used case is insertion into different file address space
-	 * cache.
-	 */
-	bool (*try_steal)(struct pipe_inode_info *, struct pipe_buffer *);
-
 	/*
 	 * Get a reference to the pipe buffer.
 	 */
@@ -304,19 +282,6 @@ static inline int pipe_buf_confirm(struct pipe_inode_info *pipe,
 	return buf->ops->confirm(pipe, buf);
 }
 
-/**
- * pipe_buf_try_steal - attempt to take ownership of a pipe_buffer
- * @pipe:	the pipe that the buffer belongs to
- * @buf:	the buffer to attempt to steal
- */
-static inline bool pipe_buf_try_steal(struct pipe_inode_info *pipe,
-		struct pipe_buffer *buf)
-{
-	if (!buf->ops->try_steal)
-		return false;
-	return buf->ops->try_steal(pipe, buf);
-}
-
 /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual
    memory allocation, whereas PIPE_BUF makes atomicity guarantees.  */
 #define PIPE_SIZE		PAGE_SIZE
@@ -335,7 +300,6 @@ void free_pipe_info(struct pipe_inode_info *);
 
 /* Generic pipe buffer ops functions */
 bool generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *);
-bool generic_pipe_buf_try_steal(struct pipe_inode_info *, struct pipe_buffer *);
 void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *);
 
 extern const struct pipe_buf_operations nosteal_pipe_buf_ops;
kernel/watch_queue.c +0 -4
index 453dcf613205..e2bdd5fb0db1 100644
@@ -77,13 +77,9 @@ static void watch_queue_pipe_buf_release(struct pipe_inode_info *pipe,
 	generic_pipe_buf_release(pipe, buf);
 }
 
-// No try_steal function => no stealing
-#define watch_queue_pipe_buf_try_steal NULL
-
 /* New data written to a pipe may be appended to a buffer with this type. */
 static const struct pipe_buf_operations watch_queue_pipe_buf_ops = {
 	.release	= watch_queue_pipe_buf_release,
-	.try_steal	= watch_queue_pipe_buf_try_steal,
 	.get		= generic_pipe_buf_get,
 };
 
mm/shmem.c +0 -7
index af21a89abb0f..4a9440a1e1f8 100644
@@ -3655,15 +3655,8 @@ static void zero_pipe_buf_release(struct pipe_inode_info *pipe,
 {
 }
 
-static bool zero_pipe_buf_try_steal(struct pipe_inode_info *pipe,
-				    struct pipe_buffer *buf)
-{
-	return false;
-}
-
 static const struct pipe_buf_operations zero_pipe_buf_ops = {
 	.release	= zero_pipe_buf_release,
-	.try_steal	= zero_pipe_buf_try_steal,
 	.get		= zero_pipe_buf_get,
 };
 
Signature · 1 line
2.54.0
#11 lore raw
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines

Nothing calls pipe_buf_try_steal() any more. Remove ->try_steal() from struct pipe_buf_operations, its implementations and the exported generic_pipe_buf_try_steal().

PIPE_BUF_FLAG_LRU and PIPE_BUF_FLAG_GIFT were set only for stealing, and go too. page_cache_pipe_buf_release() did nothing but drop the page and the LRU flag, so page cache and user page buffers use generic_pipe_buf_release().

Quoted 5 lines

vmsplice() still accepts SPLICE_F_GIFT, and splice() SPLICE_F_MOVE. Both were hints, and nothing acts on them now.

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
#12 lore raw

PATCH 4/4 splice: replace nosteal_pipe_buf_ops with default_pipe_buf_ops

In reply to Kiryl Shutsemau (Meta)

Without ->try_steal(), nosteal_pipe_buf_ops and default_pipe_buf_ops are the same: generic_pipe_buf_release() and generic_pipe_buf_get().

Keep default_pipe_buf_ops, and use it in skb_splice_bits() and fuse_dev_splice_read(). Export it in place of nosteal_pipe_buf_ops, since fuse can be built as a module.

---
 fs/fuse/dev.c             | 2 +-
 fs/splice.c               | 8 +-------
 include/linux/pipe_fs_i.h | 2 --
 net/core/skbuff.c         | 2 +-
 4 files changed, 3 insertions(+), 11 deletions(-)
fs/fuse/dev.c +1 -1
index e7238eed21fb..a5c08a00cf83 100644
@@ -1627,7 +1627,7 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
 		 * Need to be careful about this.  Having buf->ops in module
 		 * code can Oops if the buffer persists after module unload.
 		 */
-		bufs[page_nr].ops = &nosteal_pipe_buf_ops;
+		bufs[page_nr].ops = &default_pipe_buf_ops;
 		bufs[page_nr].flags = 0;
 		ret = add_to_pipe(pipe, &bufs[page_nr++]);
 		if (unlikely(ret < 0))
fs/splice.c +1 -7
index ba8f6f9aa3f0..e9e0c6a7b23c 100644
@@ -333,13 +333,7 @@ const struct pipe_buf_operations default_pipe_buf_ops = {
 	.release	= generic_pipe_buf_release,
 	.get		= generic_pipe_buf_get,
 };
-
-/* Pipe buffer operations for a socket and similar. */
-const struct pipe_buf_operations nosteal_pipe_buf_ops = {
-	.release	= generic_pipe_buf_release,
-	.get		= generic_pipe_buf_get,
-};
-EXPORT_SYMBOL(nosteal_pipe_buf_ops);
+EXPORT_SYMBOL(default_pipe_buf_ops);
 
 static void wakeup_pipe_writers(struct pipe_inode_info *pipe)
 {
include/linux/pipe_fs_i.h +0 -2
index fb6570e80d01..95f6236c5c5b 100644
@@ -302,8 +302,6 @@ void free_pipe_info(struct pipe_inode_info *);
 bool generic_pipe_buf_get(struct pipe_inode_info *, struct pipe_buffer *);
 void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *);
 
-extern const struct pipe_buf_operations nosteal_pipe_buf_ops;
-
 unsigned long account_pipe_buffers(struct user_struct *user,
 				   unsigned long old, unsigned long new);
 bool too_many_pipe_buffers_soft(unsigned long user_bufs);
net/core/skbuff.c +1 -1
index 48f676411724..e063241b2005 100644
@@ -3281,7 +3281,7 @@ int skb_splice_bits(struct sk_buff *skb, struct sock *sk, unsigned int offset,
 		.pages = pages,
 		.partial = partial,
 		.nr_pages_max = MAX_SKB_FRAGS,
-		.ops = &nosteal_pipe_buf_ops,
+		.ops = &default_pipe_buf_ops,
 		.spd_release = sock_spd_release,
 	};
 	int ret = 0;
Signature · 1 line
2.54.0
#13 lore raw

Re: [PATCH 4/4] splice: replace nosteal_pipe_buf_ops with default_pipe_buf_ops

Sashiko has reviewed this patch and found no issues.
Quoted 8 lines

Without ->try_steal(), nosteal_pipe_buf_ops and default_pipe_buf_ops are the same: generic_pipe_buf_release() and generic_pipe_buf_get().

Keep default_pipe_buf_ops, and use it in skb_splice_bits() and fuse_dev_splice_read(). Export it in place of nosteal_pipe_buf_ops, since fuse can be built as a module.

Sashiko has reviewed this patch and found no issues.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
#15 lore raw
Quoted 6 lines · Linus Torvalds
Quoted 2 lines · Kiryl Shutsemau (Meta)
 11 files changed, 26 insertions(+), 360 deletions(-)

Ack. Lovely. I'm considering just taking this as-is now, rather than delaying it. Comments?

I'll pull it into vfs.fixes and send you a pull request today. I have more stuff anyway.