Skip to content

Commit 8c71e2d

Browse files
torvaldsopsiff
authored andcommitted
fs/pipe: add simpler helpers for common cases
mainline inclusion from mainline-v6.14-rc6 category: bugfix 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> (cherry picked from commit 00a7d39) Signed-off-by: Wentao Guan <guanwentao@uniontech.com> Change-Id: I5c3d7f08c2fa78c42549ca60c359b27ee48db24f
1 parent 247a725 commit 8c71e2d

7 files changed

Lines changed: 49 additions & 23 deletions

File tree

drivers/char/virtio_console.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -926,14 +926,14 @@ static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
926926

927927
pipe_lock(pipe);
928928
ret = 0;
929-
if (pipe_empty(pipe->head, pipe->tail))
929+
if (pipe_is_empty(pipe))
930930
goto error_out;
931931

932932
ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
933933
if (ret < 0)
934934
goto error_out;
935935

936-
occupancy = pipe_occupancy(pipe->head, pipe->tail);
936+
occupancy = pipe_buf_usage(pipe);
937937
buf = alloc_buf(port->portdev->vdev, 0, occupancy);
938938

939939
if (!buf) {

fs/fuse/dev.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
13931393
if (ret < 0)
13941394
goto out;
13951395

1396-
if (pipe_occupancy(pipe->head, pipe->tail) + cs.nr_segs > pipe->max_usage) {
1396+
if (pipe_buf_usage(pipe) + cs.nr_segs > pipe->max_usage) {
13971397
ret = -EIO;
13981398
goto out;
13991399
}

fs/pipe.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
394394
wake_next_reader = true;
395395
mutex_lock(&pipe->mutex);
396396
}
397-
if (pipe_empty(pipe->head, pipe->tail))
397+
if (pipe_is_empty(pipe))
398398
wake_next_reader = false;
399399
mutex_unlock(&pipe->mutex);
400400

@@ -577,11 +577,11 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
577577
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
578578
wait_event_interruptible_exclusive(pipe->wr_wait, pipe_writable(pipe));
579579
mutex_lock(&pipe->mutex);
580-
was_empty = pipe_empty(pipe->head, pipe->tail);
580+
was_empty = pipe_is_empty(pipe);
581581
wake_next_writer = true;
582582
}
583583
out:
584-
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
584+
if (pipe_is_full(pipe))
585585
wake_next_writer = false;
586586
mutex_unlock(&pipe->mutex);
587587

fs/splice.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ ssize_t copy_splice_read(struct file *in, loff_t *ppos,
330330
int i;
331331

332332
/* Work out how much data we can actually add into the pipe */
333-
used = pipe_occupancy(pipe->head, pipe->tail);
333+
used = pipe_buf_usage(pipe);
334334
npages = max_t(ssize_t, pipe->max_usage - used, 0);
335335
len = min_t(size_t, len, npages * PAGE_SIZE);
336336
npages = DIV_ROUND_UP(len, PAGE_SIZE);
@@ -526,7 +526,7 @@ static int splice_from_pipe_next(struct pipe_inode_info *pipe, struct splice_des
526526
return -ERESTARTSYS;
527527

528528
repeat:
529-
while (pipe_empty(pipe->head, pipe->tail)) {
529+
while (pipe_is_empty(pipe)) {
530530
if (!pipe->writers)
531531
return 0;
532532

@@ -812,7 +812,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
812812
if (signal_pending(current))
813813
break;
814814

815-
while (pipe_empty(pipe->head, pipe->tail)) {
815+
while (pipe_is_empty(pipe)) {
816816
ret = 0;
817817
if (!pipe->writers)
818818
goto out;
@@ -972,7 +972,7 @@ long vfs_splice_read(struct file *in, loff_t *ppos,
972972
return 0;
973973

974974
/* Don't try to read more the pipe has space for. */
975-
p_space = pipe->max_usage - pipe_occupancy(pipe->head, pipe->tail);
975+
p_space = pipe->max_usage - pipe_buf_usage(pipe);
976976
len = min_t(size_t, len, p_space << PAGE_SHIFT);
977977

978978
ret = rw_verify_area(READ, in, ppos, len);
@@ -1060,7 +1060,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
10601060
more = sd->flags & SPLICE_F_MORE;
10611061
sd->flags |= SPLICE_F_MORE;
10621062

1063-
WARN_ON_ONCE(!pipe_empty(pipe->head, pipe->tail));
1063+
WARN_ON_ONCE(!pipe_is_empty(pipe));
10641064

10651065
while (len) {
10661066
size_t read_len;
@@ -1206,7 +1206,7 @@ static int wait_for_space(struct pipe_inode_info *pipe, unsigned flags)
12061206
send_sig(SIGPIPE, current, 0);
12071207
return -EPIPE;
12081208
}
1209-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
1209+
if (!pipe_is_full(pipe))
12101210
return 0;
12111211
if (flags & SPLICE_F_NONBLOCK)
12121212
return -EAGAIN;
@@ -1604,13 +1604,13 @@ static int ipipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
16041604
* Check the pipe occupancy without the inode lock first. This function
16051605
* is speculative anyways, so missing one is ok.
16061606
*/
1607-
if (!pipe_empty(pipe->head, pipe->tail))
1607+
if (!pipe_is_empty(pipe))
16081608
return 0;
16091609

16101610
ret = 0;
16111611
pipe_lock(pipe);
16121612

1613-
while (pipe_empty(pipe->head, pipe->tail)) {
1613+
while (pipe_is_empty(pipe)) {
16141614
if (signal_pending(current)) {
16151615
ret = -ERESTARTSYS;
16161616
break;
@@ -1640,13 +1640,13 @@ static int opipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
16401640
* Check pipe occupancy without the inode lock first. This function
16411641
* is speculative anyways, so missing one is ok.
16421642
*/
1643-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
1643+
if (!pipe_is_full(pipe))
16441644
return 0;
16451645

16461646
ret = 0;
16471647
pipe_lock(pipe);
16481648

1649-
while (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
1649+
while (pipe_is_full(pipe)) {
16501650
if (!pipe->readers) {
16511651
send_sig(SIGPIPE, current, 0);
16521652
ret = -EPIPE;

include/linux/pipe_fs_i.h

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,33 @@ static inline bool pipe_full(unsigned int head, unsigned int tail,
208208
return pipe_occupancy(head, tail) >= limit;
209209
}
210210

211+
/**
212+
* pipe_is_full - Return true if the pipe is full
213+
* @pipe: the pipe
214+
*/
215+
static inline bool pipe_is_full(const struct pipe_inode_info *pipe)
216+
{
217+
return pipe_full(pipe->head, pipe->tail, pipe->max_usage);
218+
}
219+
220+
/**
221+
* pipe_is_empty - Return true if the pipe is empty
222+
* @pipe: the pipe
223+
*/
224+
static inline bool pipe_is_empty(const struct pipe_inode_info *pipe)
225+
{
226+
return pipe_empty(pipe->head, pipe->tail);
227+
}
228+
229+
/**
230+
* pipe_buf_usage - Return how many pipe buffers are in use
231+
* @pipe: the pipe
232+
*/
233+
static inline unsigned int pipe_buf_usage(const struct pipe_inode_info *pipe)
234+
{
235+
return pipe_occupancy(pipe->head, pipe->tail);
236+
}
237+
211238
/**
212239
* pipe_buf - Return the pipe buffer for the specified slot in the pipe ring
213240
* @pipe: The pipe to access

mm/filemap.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2892,8 +2892,7 @@ size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
28922892
size = min(size, folio_size(folio) - offset);
28932893
offset %= PAGE_SIZE;
28942894

2895-
while (spliced < size &&
2896-
!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
2895+
while (spliced < size && !pipe_is_full(pipe)) {
28972896
struct pipe_buffer *buf = pipe_head_buf(pipe);
28982897
size_t part = min_t(size_t, PAGE_SIZE - offset, size - spliced);
28992898

@@ -2950,7 +2949,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
29502949
iocb.ki_pos = *ppos;
29512950

29522951
/* Work out how much data we can actually add into the pipe */
2953-
used = pipe_occupancy(pipe->head, pipe->tail);
2952+
used = pipe_buf_usage(pipe);
29542953
npages = max_t(ssize_t, pipe->max_usage - used, 0);
29552954
len = min_t(size_t, len, npages * PAGE_SIZE);
29562955

@@ -3010,7 +3009,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
30103009
total_spliced += n;
30113010
*ppos += n;
30123011
in->f_ra.prev_pos = *ppos;
3013-
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
3012+
if (pipe_is_full(pipe))
30143013
goto out;
30153014
}
30163015

mm/shmem.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2917,7 +2917,7 @@ static size_t splice_zeropage_into_pipe(struct pipe_inode_info *pipe,
29172917

29182918
size = min_t(size_t, size, PAGE_SIZE - offset);
29192919

2920-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
2920+
if (!pipe_is_full(pipe)) {
29212921
struct pipe_buffer *buf = pipe_head_buf(pipe);
29222922

29232923
*buf = (struct pipe_buffer) {
@@ -2944,7 +2944,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
29442944
int error = 0;
29452945

29462946
/* Work out how much data we can actually add into the pipe */
2947-
used = pipe_occupancy(pipe->head, pipe->tail);
2947+
used = pipe_buf_usage(pipe);
29482948
npages = max_t(ssize_t, pipe->max_usage - used, 0);
29492949
len = min_t(size_t, len, npages * PAGE_SIZE);
29502950

@@ -3009,7 +3009,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
30093009
total_spliced += n;
30103010
*ppos += n;
30113011
in->f_ra.prev_pos = *ppos;
3012-
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
3012+
if (pipe_is_full(pipe))
30133013
break;
30143014

30153015
cond_resched();

0 commit comments

Comments
 (0)