fs/pipe: add simpler helpers for common cases

The fix to atomically read the pipe head and tail state when not holding
the pipe mutex has caused a number of headaches due to the size change
of the involved types.

It turns out that we don't have _that_ many places that access these
fields directly and were affected, but we have more than we strictly
should have, because our low-level helper functions have been designed
to have intimate knowledge of how the pipes work.

And as a result, that random noise of direct 'pipe->head' and
'pipe->tail' accesses makes it harder to pinpoint any actual potential
problem spots remaining.

For example, we didn't have a "is the pipe full" helper function, but
instead had a "given these pipe buffer indexes and this pipe size, is
the pipe full".  That's because some low-level pipe code does actually
want that much more complicated interface.

But most other places literally just want a "is the pipe full" helper,
and not having it meant that those places ended up being unnecessarily
much too aware of this all.

It would have been much better if only the very core pipe code that
cared had been the one aware of this all.

So let's fix it - better late than never.  This just introduces the
trivial wrappers for "is this pipe full or empty" and to get how many
pipe buffers are used, so that instead of writing

        if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))

the places that literally just want to know if a pipe is full can just
say

        if (pipe_is_full(pipe))

instead.  The existing trivial cases were converted with a 'sed' script.

This cuts down on the places that access pipe->head and pipe->tail
directly outside of the pipe code (and core splice code) quite a lot.

The splice code in particular still revels in doing the direct low-level
accesses, and the fuse fuse_dev_splice_write() code also seems a bit
unnecessarily eager to go very low-level, but it's at least a bit better
than it used to be.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 24442485e73e7113dddbdd0b1e51cf3e74b5ee22..18f92dd44d456d97c748627842eb113afda68fd3 100644 (file)
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -923,14 +923,14 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
 
        pipe_lock(pipe);
        ret = 0;
-       if (pipe_empty(pipe->head, pipe->tail))
+       if (pipe_is_empty(pipe))
                goto error_out;
 
        ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
        if (ret < 0)
                goto error_out;
 
-       occupancy = pipe_occupancy(pipe->head, pipe->tail);
+       occupancy = pipe_buf_usage(pipe);
        buf = alloc_buf(port->portdev->vdev, 0, occupancy);
 
        if (!buf) {

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3c9caafca9e29fd38fc922d6900947d2ff99064b..2c3a4d09e500f98232d5d9412a012235af6bec2e 100644 (file)
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1457,7 +1457,7 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
        if (ret < 0)
                goto out;
 
-       if (pipe_occupancy(pipe->head, pipe->tail) + cs.nr_segs > pipe->max_usage) {
+       if (pipe_buf_usage(pipe) + cs.nr_segs > pipe->max_usage) {
                ret = -EIO;
                goto out;
        }

diff --git a/fs/pipe.c b/fs/pipe.c
index 5c872775a6db95087153faa41e3e3cfcd32411c3..4d0799e4e7196b02df066520669ddc55fb7a6a19 100644 (file)
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -394,7 +394,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
                wake_next_reader = true;
                mutex_lock(&pipe->mutex);
        }
-       if (pipe_empty(pipe->head, pipe->tail))
+       if (pipe_is_empty(pipe))
                wake_next_reader = false;
        mutex_unlock(&pipe->mutex);
 
@@ -577,11 +577,11 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
                kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
                wait_event_interruptible_exclusive(pipe->wr_wait, pipe_writable(pipe));
                mutex_lock(&pipe->mutex);
-               was_empty = pipe_empty(pipe->head, pipe->tail);
+               was_empty = pipe_is_empty(pipe);
                wake_next_writer = true;
        }
 out:
-       if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
+       if (pipe_is_full(pipe))
                wake_next_writer = false;
        mutex_unlock(&pipe->mutex);
 

diff --git a/fs/splice.c b/fs/splice.c
index 28cfa63aa236473255c6bdebbfe7cb18ae176aae..23fa5561b9441969c0c998a377d5cd1b42942ea2 100644 (file)
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -331,7 +331,7 @@ ssize_t copy_splice_read(struct file *in, loff_t *ppos,
        int i;
 
        /* Work out how much data we can actually add into the pipe */
-       used = pipe_occupancy(pipe->head, pipe->tail);
+       used = pipe_buf_usage(pipe);
        npages = max_t(ssize_t, pipe->max_usage - used, 0);
        len = min_t(size_t, len, npages * PAGE_SIZE);
        npages = DIV_ROUND_UP(len, PAGE_SIZE);
@@ -527,7 +527,7 @@ static int splice_from_pipe_next(struct pipe_inode_info *pipe, struct splice_des
                return -ERESTARTSYS;
 
 repeat:
-       while (pipe_empty(pipe->head, pipe->tail)) {
+       while (pipe_is_empty(pipe)) {
                if (!pipe->writers)
                        return 0;
 
@@ -820,7 +820,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
                if (signal_pending(current))
                        break;
 
-               while (pipe_empty(pipe->head, pipe->tail)) {
+               while (pipe_is_empty(pipe)) {
                        ret = 0;
                        if (!pipe->writers)
                                goto out;
@@ -968,7 +968,7 @@ static ssize_t do_splice_read(struct file *in, loff_t *ppos,
                return 0;
 
        /* Don't try to read more the pipe has space for. */
-       p_space = pipe->max_usage - pipe_occupancy(pipe->head, pipe->tail);
+       p_space = pipe->max_usage - pipe_buf_usage(pipe);
        len = min_t(size_t, len, p_space << PAGE_SHIFT);
 
        if (unlikely(len > MAX_RW_COUNT))
@@ -1080,7 +1080,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
        more = sd->flags & SPLICE_F_MORE;
        sd->flags |= SPLICE_F_MORE;
 
-       WARN_ON_ONCE(!pipe_empty(pipe->head, pipe->tail));
+       WARN_ON_ONCE(!pipe_is_empty(pipe));
 
        while (len) {
                size_t read_len;
@@ -1268,7 +1268,7 @@ static int wait_for_space(struct pipe_inode_info *pipe, unsigned flags)
                        send_sig(SIGPIPE, current, 0);
                        return -EPIPE;
                }
-               if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
+               if (!pipe_is_full(pipe))
                        return 0;
                if (flags & SPLICE_F_NONBLOCK)
                        return -EAGAIN;
@@ -1652,13 +1652,13 @@ static int ipipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
         * Check the pipe occupancy without the inode lock first. This function
         * is speculative anyways, so missing one is ok.
         */
-       if (!pipe_empty(pipe->head, pipe->tail))
+       if (!pipe_is_empty(pipe))
                return 0;
 
        ret = 0;
        pipe_lock(pipe);
 
-       while (pipe_empty(pipe->head, pipe->tail)) {
+       while (pipe_is_empty(pipe)) {
                if (signal_pending(current)) {
                        ret = -ERESTARTSYS;
                        break;
@@ -1688,13 +1688,13 @@ static int opipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
         * Check pipe occupancy without the inode lock first. This function
         * is speculative anyways, so missing one is ok.
         */
-       if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
+       if (!pipe_is_full(pipe))
                return 0;
 
        ret = 0;
        pipe_lock(pipe);
 
-       while (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
+       while (pipe_is_full(pipe)) {
                if (!pipe->readers) {
                        send_sig(SIGPIPE, current, 0);
                        ret = -EPIPE;

diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 4d0a2267e6efce6750e490207bfae0ec309e22f9..b698758000f8b7ff884728a2b0f63cfeabc3f19d 100644 (file)
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -208,6 +208,33 @@ static inline bool pipe_full(unsigned int head, unsigned int tail,
        return pipe_occupancy(head, tail) >= limit;
 }
 
+/**
+ * pipe_is_full - Return true if the pipe is full
+ * @pipe: the pipe
+ */
+static inline bool pipe_is_full(const struct pipe_inode_info *pipe)
+{
+       return pipe_full(pipe->head, pipe->tail, pipe->max_usage);
+}
+
+/**
+ * pipe_is_empty - Return true if the pipe is empty
+ * @pipe: the pipe
+ */
+static inline bool pipe_is_empty(const struct pipe_inode_info *pipe)
+{
+       return pipe_empty(pipe->head, pipe->tail);
+}
+
+/**
+ * pipe_buf_usage - Return how many pipe buffers are in use
+ * @pipe: the pipe
+ */
+static inline unsigned int pipe_buf_usage(const struct pipe_inode_info *pipe)
+{
+       return pipe_occupancy(pipe->head, pipe->tail);
+}
+
 /**
  * pipe_buf - Return the pipe buffer for the specified slot in the pipe ring
  * @pipe: The pipe to access

diff --git a/mm/filemap.c b/mm/filemap.c
index d4564a79eb3536c5322e6075b19bab0d687647e3..2974691fdfad2463c526610dee7426472aa0b826 100644 (file)
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2897,8 +2897,7 @@ size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
        size = min(size, folio_size(folio) - offset);
        offset %= PAGE_SIZE;
 
-       while (spliced < size &&
-              !pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
+       while (spliced < size && !pipe_is_full(pipe)) {
                struct pipe_buffer *buf = pipe_head_buf(pipe);
                size_t part = min_t(size_t, PAGE_SIZE - offset, size - spliced);
 
@@ -2955,7 +2954,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
        iocb.ki_pos = *ppos;
 
        /* Work out how much data we can actually add into the pipe */
-       used = pipe_occupancy(pipe->head, pipe->tail);
+       used = pipe_buf_usage(pipe);
        npages = max_t(ssize_t, pipe->max_usage - used, 0);
        len = min_t(size_t, len, npages * PAGE_SIZE);
 
@@ -3015,7 +3014,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
                        total_spliced += n;
                        *ppos += n;
                        in->f_ra.prev_pos = *ppos;
-                       if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
+                       if (pipe_is_full(pipe))
                                goto out;
                }
 

diff --git a/mm/shmem.c b/mm/shmem.c
index 4ea6109a80431e5eeae15278064d5c86412f9fc9..20032a333d80ce39ab695645267b8c422de20113 100644 (file)
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3487,7 +3487,7 @@ static size_t splice_zeropage_into_pipe(struct pipe_inode_info *pipe,
 
        size = min_t(size_t, size, PAGE_SIZE - offset);
 
-       if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
+       if (!pipe_is_full(pipe)) {
                struct pipe_buffer *buf = pipe_head_buf(pipe);
 
                *buf = (struct pipe_buffer) {
@@ -3514,7 +3514,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
        int error = 0;
 
        /* Work out how much data we can actually add into the pipe */
-       used = pipe_occupancy(pipe->head, pipe->tail);
+       used = pipe_buf_usage(pipe);
        npages = max_t(ssize_t, pipe->max_usage - used, 0);
        len = min_t(size_t, len, npages * PAGE_SIZE);
 
@@ -3601,7 +3601,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
                total_spliced += n;
                *ppos += n;
                in->f_ra.prev_pos = *ppos;
-               if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
+               if (pipe_is_full(pipe))
                        break;
 
                cond_resched();