History

pipe: remove 'waiting_writers' merging logic

This code is ancient, and goes back to when we only had a single page for the pipe buffers. The exact history is hidden in the mists of time (ie "before git", and in fact predates the BK repository too). At that long-ago point in time, it actually helped to try to merge big back-and-forth pipe reads and writes, and not limit pipe reads to the single pipe buffer in length just because that was all we had at a time. However, since then we've expanded the pipe buffers to multiple pages, and this logic really doesn't seem to make sense. And a lot of it is somewhat questionable (ie "hmm, the user asked for a non-blocking read, but we see that there's a writer pending, so let's wait anyway to get the extra data that the writer will have"). But more importantly, it makes the "go to sleep" logic much less obvious, and considering the wakeup issues we've had, I want to make for less of those kinds of things.

Linus Torvalds committed 6 years ago

Commit a28c8b91 parent f467a6a3 files · +9 −33Browse files

Unified · Split

3 files
  1. fs/pipe.c +5 −14
  2. fs/splice.c +4 −17
  3. include/linux/pipe_fs_i.h +0 −2
fs/pipe.c+5−14
@@ -348,18 +348,11 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) · 347 unmodified lines
348 348
349 if (!pipe->writers)349 if (!pipe->writers)
350 break;350 break;
351 if (!pipe->waiting_writers) {351 if (ret)
352 /* syscall merging: Usually we must not sleep352 break;
353 * if O_NONBLOCK is set, or if we got some data.353 if (filp->f_flags & O_NONBLOCK) {
354 * But if a writer sleeps in kernel space, then354 ret = -EAGAIN;
355 * we can wait for that data without violating POSIX.355 break;
356 */
357 if (ret)
358 break;
359 if (filp->f_flags & O_NONBLOCK) {
360 ret = -EAGAIN;
361 break;
362 }
363 }356 }
364 if (signal_pending(current)) {357 if (signal_pending(current)) {
365 if (!ret)358 if (!ret)
@@ -540,9 +533,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) · 174 unmodified lines
540 wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM);533 wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM);
541 kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);534 kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
542 }535 }
543 pipe->waiting_writers++;
544 pipe_wait(pipe);536 pipe_wait(pipe);
545 pipe->waiting_writers--;
546 537
547 was_empty = pipe_empty(head, pipe->tail);538 was_empty = pipe_empty(head, pipe->tail);
548 }539 }
fs/splice.c+4−17
@@ -559,7 +559,7 @@ static int splice_from_pipe_next(struct pipe_inode_info *pipe, struct splice_des · 558 unmodified lines
559 if (!pipe->writers)559 if (!pipe->writers)
560 return 0;560 return 0;
561 561
562 if (!pipe->waiting_writers && sd->num_spliced)562 if (sd->num_spliced)
563 return 0;563 return 0;
564 564
565 if (sd->flags & SPLICE_F_NONBLOCK)565 if (sd->flags & SPLICE_F_NONBLOCK)
@@ -1098,9 +1098,7 @@ static int wait_for_space(struct pipe_inode_info *pipe, unsigned flags) · 532 unmodified lines
1098 return -EAGAIN;1098 return -EAGAIN;
1099 if (signal_pending(current))1099 if (signal_pending(current))
1100 return -ERESTARTSYS;1100 return -ERESTARTSYS;
1101 pipe->waiting_writers++;
1102 pipe_wait(pipe);1101 pipe_wait(pipe);
1103 pipe->waiting_writers--;
1104 }1102 }
1105}1103}
1106 1104
@@ -1482,11 +1480,9 @@ static int ipipe_prep(struct pipe_inode_info *pipe, unsigned int flags) · 375 unmodified lines
1482 }1480 }
1483 if (!pipe->writers)1481 if (!pipe->writers)
1484 break;1482 break;
1485 if (!pipe->waiting_writers) {1483 if (flags & SPLICE_F_NONBLOCK) {
1486 if (flags & SPLICE_F_NONBLOCK) {1484 ret = -EAGAIN;
1487 ret = -EAGAIN;1485 break;
1488 break;
1489 }
1490 }1486 }
1491 pipe_wait(pipe);1487 pipe_wait(pipe);
1492 }1488 }
@@ -1527,9 +1523,7 @@ static int opipe_prep(struct pipe_inode_info *pipe, unsigned int flags) · 34 unmodified lines
1527 ret = -ERESTARTSYS;1523 ret = -ERESTARTSYS;
1528 break;1524 break;
1529 }1525 }
1530 pipe->waiting_writers++;
1531 pipe_wait(pipe);1526 pipe_wait(pipe);
1532 pipe->waiting_writers--;
1533 }1527 }
1534 1528
1535 pipe_unlock(pipe);1529 pipe_unlock(pipe);
@@ -1751,13 +1745,6 @@ static int link_pipe(struct pipe_inode_info *ipipe, · 215 unmodified lines
1751 i_tail++;1745 i_tail++;
1752 } while (len);1746 } while (len);
1753 1747
1754 /*
1755 * return EAGAIN if we have the potential of some data in the
1756 * future, otherwise just return 0
1757 */
1758 if (!ret && ipipe->waiting_writers && (flags & SPLICE_F_NONBLOCK))
1759 ret = -EAGAIN;
1760
1761 pipe_unlock(ipipe);1748 pipe_unlock(ipipe);
1762 pipe_unlock(opipe);1749 pipe_unlock(opipe);
1763 1750
include/linux/pipe_fs_i.h+0−2
@@ -38,7 +38,6 @@ struct pipe_buffer { · 37 unmodified lines
38 * @readers: number of current readers of this pipe38 * @readers: number of current readers of this pipe
39 * @writers: number of current writers of this pipe39 * @writers: number of current writers of this pipe
40 * @files: number of struct file referring this pipe (protected by ->i_lock)40 * @files: number of struct file referring this pipe (protected by ->i_lock)
41 * @waiting_writers: number of writers blocked waiting for room
42 * @r_counter: reader counter41 * @r_counter: reader counter
43 * @w_counter: writer counter42 * @w_counter: writer counter
44 * @fasync_readers: reader side fasync43 * @fasync_readers: reader side fasync
@@ -56,7 +55,6 @@ struct pipe_inode_info { · 11 unmodified lines
56 unsigned int readers;55 unsigned int readers;
57 unsigned int writers;56 unsigned int writers;
58 unsigned int files;57 unsigned int files;
59 unsigned int waiting_writers;
60 unsigned int r_counter;58 unsigned int r_counter;
61 unsigned int w_counter;59 unsigned int w_counter;
62 struct page *tmp_page;60 struct page *tmp_page;

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t