Skip to content

Commit 00a7d39

Browse files
committed
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>
1 parent 28f587a commit 00a7d39

File tree

7 files changed

+49
-23
lines changed

7 files changed

+49
-23
lines changed

drivers/char/virtio_console.c

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

924924
pipe_lock(pipe);
925925
ret = 0;
926-
if (pipe_empty(pipe->head, pipe->tail))
926+
if (pipe_is_empty(pipe))
927927
goto error_out;
928928

929929
ret = wait_port_writable(port, filp->f_flags & O_NONBLOCK);
930930
if (ret < 0)
931931
goto error_out;
932932

933-
occupancy = pipe_occupancy(pipe->head, pipe->tail);
933+
occupancy = pipe_buf_usage(pipe);
934934
buf = alloc_buf(port->portdev->vdev, 0, occupancy);
935935

936936
if (!buf) {

fs/fuse/dev.c

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

1460-
if (pipe_occupancy(pipe->head, pipe->tail) + cs.nr_segs > pipe->max_usage) {
1460+
if (pipe_buf_usage(pipe) + cs.nr_segs > pipe->max_usage) {
14611461
ret = -EIO;
14621462
goto out;
14631463
}

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
@@ -331,7 +331,7 @@ ssize_t copy_splice_read(struct file *in, loff_t *ppos,
331331
int i;
332332

333333
/* Work out how much data we can actually add into the pipe */
334-
used = pipe_occupancy(pipe->head, pipe->tail);
334+
used = pipe_buf_usage(pipe);
335335
npages = max_t(ssize_t, pipe->max_usage - used, 0);
336336
len = min_t(size_t, len, npages * PAGE_SIZE);
337337
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
527527
return -ERESTARTSYS;
528528

529529
repeat:
530-
while (pipe_empty(pipe->head, pipe->tail)) {
530+
while (pipe_is_empty(pipe)) {
531531
if (!pipe->writers)
532532
return 0;
533533

@@ -820,7 +820,7 @@ ssize_t splice_to_socket(struct pipe_inode_info *pipe, struct file *out,
820820
if (signal_pending(current))
821821
break;
822822

823-
while (pipe_empty(pipe->head, pipe->tail)) {
823+
while (pipe_is_empty(pipe)) {
824824
ret = 0;
825825
if (!pipe->writers)
826826
goto out;
@@ -968,7 +968,7 @@ static ssize_t do_splice_read(struct file *in, loff_t *ppos,
968968
return 0;
969969

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

974974
if (unlikely(len > MAX_RW_COUNT))
@@ -1080,7 +1080,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
10801080
more = sd->flags & SPLICE_F_MORE;
10811081
sd->flags |= SPLICE_F_MORE;
10821082

1083-
WARN_ON_ONCE(!pipe_empty(pipe->head, pipe->tail));
1083+
WARN_ON_ONCE(!pipe_is_empty(pipe));
10841084

10851085
while (len) {
10861086
size_t read_len;
@@ -1268,7 +1268,7 @@ static int wait_for_space(struct pipe_inode_info *pipe, unsigned flags)
12681268
send_sig(SIGPIPE, current, 0);
12691269
return -EPIPE;
12701270
}
1271-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
1271+
if (!pipe_is_full(pipe))
12721272
return 0;
12731273
if (flags & SPLICE_F_NONBLOCK)
12741274
return -EAGAIN;
@@ -1652,13 +1652,13 @@ static int ipipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
16521652
* Check the pipe occupancy without the inode lock first. This function
16531653
* is speculative anyways, so missing one is ok.
16541654
*/
1655-
if (!pipe_empty(pipe->head, pipe->tail))
1655+
if (!pipe_is_empty(pipe))
16561656
return 0;
16571657

16581658
ret = 0;
16591659
pipe_lock(pipe);
16601660

1661-
while (pipe_empty(pipe->head, pipe->tail)) {
1661+
while (pipe_is_empty(pipe)) {
16621662
if (signal_pending(current)) {
16631663
ret = -ERESTARTSYS;
16641664
break;
@@ -1688,13 +1688,13 @@ static int opipe_prep(struct pipe_inode_info *pipe, unsigned int flags)
16881688
* Check pipe occupancy without the inode lock first. This function
16891689
* is speculative anyways, so missing one is ok.
16901690
*/
1691-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage))
1691+
if (!pipe_is_full(pipe))
16921692
return 0;
16931693

16941694
ret = 0;
16951695
pipe_lock(pipe);
16961696

1697-
while (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
1697+
while (pipe_is_full(pipe)) {
16981698
if (!pipe->readers) {
16991699
send_sig(SIGPIPE, current, 0);
17001700
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
@@ -2897,8 +2897,7 @@ size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
28972897
size = min(size, folio_size(folio) - offset);
28982898
offset %= PAGE_SIZE;
28992899

2900-
while (spliced < size &&
2901-
!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
2900+
while (spliced < size && !pipe_is_full(pipe)) {
29022901
struct pipe_buffer *buf = pipe_head_buf(pipe);
29032902
size_t part = min_t(size_t, PAGE_SIZE - offset, size - spliced);
29042903

@@ -2955,7 +2954,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
29552954
iocb.ki_pos = *ppos;
29562955

29572956
/* Work out how much data we can actually add into the pipe */
2958-
used = pipe_occupancy(pipe->head, pipe->tail);
2957+
used = pipe_buf_usage(pipe);
29592958
npages = max_t(ssize_t, pipe->max_usage - used, 0);
29602959
len = min_t(size_t, len, npages * PAGE_SIZE);
29612960

@@ -3015,7 +3014,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
30153014
total_spliced += n;
30163015
*ppos += n;
30173016
in->f_ra.prev_pos = *ppos;
3018-
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
3017+
if (pipe_is_full(pipe))
30193018
goto out;
30203019
}
30213020

mm/shmem.c

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

34883488
size = min_t(size_t, size, PAGE_SIZE - offset);
34893489

3490-
if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
3490+
if (!pipe_is_full(pipe)) {
34913491
struct pipe_buffer *buf = pipe_head_buf(pipe);
34923492

34933493
*buf = (struct pipe_buffer) {
@@ -3514,7 +3514,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
35143514
int error = 0;
35153515

35163516
/* Work out how much data we can actually add into the pipe */
3517-
used = pipe_occupancy(pipe->head, pipe->tail);
3517+
used = pipe_buf_usage(pipe);
35183518
npages = max_t(ssize_t, pipe->max_usage - used, 0);
35193519
len = min_t(size_t, len, npages * PAGE_SIZE);
35203520

@@ -3601,7 +3601,7 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos,
36013601
total_spliced += n;
36023602
*ppos += n;
36033603
in->f_ra.prev_pos = *ppos;
3604-
if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
3604+
if (pipe_is_full(pipe))
36053605
break;
36063606

36073607
cond_resched();

0 commit comments

Comments
 (0)