[PATCH] pipe_fs_i.h: add pipe_buf_init()

From: Max Kellermann
Date: Tue Sep 19 2023 - 04:07:25 EST


Adds one central function which shall be used to initialize a newly
allocated struct pipe_buffer. This shall make the pipe code more
robust for the next time the pipe_buffer struct gets modified, to
avoid leaving new members uninitialized. Instead, adding new members
should also add a new pipe_buf_init() parameter, which causes
compile-time errors in call sites that were not adapted.

This commit doesn't refactor fs/fuse/dev.c because this code looks
obscure to me; it initializes pipe_buffers incrementally through a
variety of functions, too complicated for me to understand.

Signed-off-by: Max Kellermann <max.kellermann@xxxxxxxxx>
---
fs/pipe.c | 9 +++------
fs/splice.c | 9 ++++-----
include/linux/pipe_fs_i.h | 20 ++++++++++++++++++++
kernel/watch_queue.c | 8 +++-----
mm/filemap.c | 8 ++------
mm/shmem.c | 9 +++------
6 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 6c1a9b1db907..edba8c666c95 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -520,14 +520,11 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)

/* Insert it into the buffer array */
buf = &pipe->bufs[head & mask];
- buf->page = page;
- buf->ops = &anon_pipe_buf_ops;
- buf->offset = 0;
- buf->len = 0;
+ pipe_buf_init(buf, page, 0, 0,
+ &anon_pipe_buf_ops,
+ PIPE_BUF_FLAG_CAN_MERGE);
if (is_packetized(filp))
buf->flags = PIPE_BUF_FLAG_PACKET;
- else
- buf->flags = PIPE_BUF_FLAG_CAN_MERGE;
pipe->tmp_page = NULL;

copied = copy_page_from_iter(page, 0, PAGE_SIZE, from);
diff --git a/fs/splice.c b/fs/splice.c
index d983d375ff11..277bc4812164 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -215,12 +215,11 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe,
while (!pipe_full(head, tail, pipe->max_usage)) {
struct pipe_buffer *buf = &pipe->bufs[head & mask];

- buf->page = spd->pages[page_nr];
- buf->offset = spd->partial[page_nr].offset;
- buf->len = spd->partial[page_nr].len;
+ pipe_buf_init(buf, spd->pages[page_nr],
+ spd->partial[page_nr].offset,
+ spd->partial[page_nr].len,
+ spd->ops, 0);
buf->private = spd->partial[page_nr].private;
- buf->ops = spd->ops;
- buf->flags = 0;

head++;
pipe->head = head;
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 608a9eb86bff..2ef2bb218641 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -176,6 +176,26 @@ static inline struct pipe_buffer *pipe_head_buf(const struct pipe_inode_info *pi
return pipe_buf(pipe, pipe->head);
}

+/**
+ * Initialize a struct pipe_buffer.
+ */
+static inline void pipe_buf_init(struct pipe_buffer *buf,
+ struct page *page,
+ unsigned int offset, unsigned int len,
+ const struct pipe_buf_operations *ops,
+ unsigned int flags)
+{
+ buf->page = page;
+ buf->offset = offset;
+ buf->len = len;
+ buf->ops = ops;
+ buf->flags = flags;
+
+ /* not initializing the "private" member because it is only
+ used by pipe_buf_operations which inject it via struct
+ partial_page / struct splice_pipe_desc */
+}
+
/**
* pipe_buf_get - get a reference to a pipe_buffer
* @pipe: the pipe that the buffer belongs to
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index d0b6b390ee42..187ad7ca38b0 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -125,12 +125,10 @@ static bool post_one_notification(struct watch_queue *wqueue,
kunmap_atomic(p);

buf = &pipe->bufs[head & mask];
- buf->page = page;
+ pipe_buf_init(buf, page, offset, len,
+ &watch_queue_pipe_buf_ops,
+ PIPE_BUF_FLAG_WHOLE);
buf->private = (unsigned long)wqueue;
- buf->ops = &watch_queue_pipe_buf_ops;
- buf->offset = offset;
- buf->len = len;
- buf->flags = PIPE_BUF_FLAG_WHOLE;
smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */

if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
diff --git a/mm/filemap.c b/mm/filemap.c
index 582f5317ff71..74532e0cb8d7 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2850,12 +2850,8 @@ size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
struct pipe_buffer *buf = pipe_head_buf(pipe);
size_t part = min_t(size_t, PAGE_SIZE - offset, size - spliced);

- *buf = (struct pipe_buffer) {
- .ops = &page_cache_pipe_buf_ops,
- .page = page,
- .offset = offset,
- .len = part,
- };
+ pipe_buf_init(buf, page, offset, part,
+ &page_cache_pipe_buf_ops, 0);
folio_get(folio);
pipe->head++;
page++;
diff --git a/mm/shmem.c b/mm/shmem.c
index 02e62fccc80d..75d39653b028 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2901,12 +2901,9 @@ static size_t splice_zeropage_into_pipe(struct pipe_inode_info *pipe,
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
struct pipe_buffer *buf = pipe_head_buf(pipe);

- *buf = (struct pipe_buffer) {
- .ops = &zero_pipe_buf_ops,
- .page = ZERO_PAGE(0),
- .offset = offset,
- .len = size,
- };
+ pipe_buf_init(buf, ZERO_PAGE(0),
+ offset, size,
+ &zero_pipe_buf_ops, 0);
pipe->head++;
}

--
2.39.2