From 8cefc107ca54c8b06438b7dc9cc08bc0a11d5b98 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 15 Nov 2019 13:30:32 +0000 Subject: pipe: Use head and tail pointers for the ring, not cursor and length Convert pipes to use head and tail pointers for the buffer ring rather than pointer and length as the latter requires two atomic ops to update (or a combined op) whereas the former only requires one. (1) The head pointer is the point at which production occurs and points to the slot in which the next buffer will be placed. This is equivalent to pipe->curbuf + pipe->nrbufs. The head pointer belongs to the write-side. (2) The tail pointer is the point at which consumption occurs. It points to the next slot to be consumed. This is equivalent to pipe->curbuf. The tail pointer belongs to the read-side. (3) head and tail are allowed to run to UINT_MAX and wrap naturally. They are only masked off when the array is being accessed, e.g.: pipe->bufs[head & mask] This means that it is not necessary to have a dead slot in the ring as head == tail isn't ambiguous. (4) The ring is empty if "head == tail". A helper, pipe_empty(), is provided for this. (5) The occupancy of the ring is "head - tail". A helper, pipe_occupancy(), is provided for this. (6) The number of free slots in the ring is "pipe->ring_size - occupancy". A helper, pipe_space_for_user() is provided to indicate how many slots userspace may use. (7) The ring is full if "head - tail >= pipe->ring_size". A helper, pipe_full(), is provided for this. Signed-off-by: David Howells --- fs/pipe.c | 170 +++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 97 insertions(+), 73 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 8a2ab2f974bd..e9b361cb093e 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -43,10 +43,12 @@ unsigned long pipe_user_pages_hard; unsigned long pipe_user_pages_soft = PIPE_DEF_BUFFERS * INR_OPEN_CUR; /* - * We use a start+len construction, which provides full use of the - * allocated memory. - * -- Florian Coosmann (FGC) - * + * We use head and tail indices that aren't masked off, except at the point of + * dereference, but rather they're allowed to wrap naturally. This means there + * isn't a dead spot in the buffer, but the ring has to be a power of two and + * <= 2^31. + * -- David Howells 2019-09-23. + * * Reads with count = 0 should always return 0. * -- Julian Bradfield 1999-06-07. * @@ -285,10 +287,12 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) ret = 0; __pipe_lock(pipe); for (;;) { - int bufs = pipe->nrbufs; - if (bufs) { - int curbuf = pipe->curbuf; - struct pipe_buffer *buf = pipe->bufs + curbuf; + unsigned int head = pipe->head; + unsigned int tail = pipe->tail; + unsigned int mask = pipe->ring_size - 1; + + if (!pipe_empty(head, tail)) { + struct pipe_buffer *buf = &pipe->bufs[tail & mask]; size_t chars = buf->len; size_t written; int error; @@ -321,17 +325,17 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (!buf->len) { pipe_buf_release(pipe, buf); - curbuf = (curbuf + 1) & (pipe->buffers - 1); - pipe->curbuf = curbuf; - pipe->nrbufs = --bufs; + tail++; + pipe->tail = tail; do_wakeup = 1; } total_len -= chars; if (!total_len) break; /* common path: read succeeded */ + if (!pipe_empty(head, tail)) /* More to do? */ + continue; } - if (bufs) /* More to do? */ - continue; + if (!pipe->writers) break; if (!pipe->waiting_writers) { @@ -380,6 +384,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) { struct file *filp = iocb->ki_filp; struct pipe_inode_info *pipe = filp->private_data; + unsigned int head, tail, max_usage, mask; ssize_t ret = 0; int do_wakeup = 0; size_t total_len = iov_iter_count(from); @@ -397,12 +402,15 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) goto out; } + tail = pipe->tail; + head = pipe->head; + max_usage = pipe->ring_size; + mask = pipe->ring_size - 1; + /* We try to merge small writes */ chars = total_len & (PAGE_SIZE-1); /* size of the last buffer */ - if (pipe->nrbufs && chars != 0) { - int lastbuf = (pipe->curbuf + pipe->nrbufs - 1) & - (pipe->buffers - 1); - struct pipe_buffer *buf = pipe->bufs + lastbuf; + if (!pipe_empty(head, tail) && chars != 0) { + struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask]; int offset = buf->offset + buf->len; if (pipe_buf_can_merge(buf) && offset + chars <= PAGE_SIZE) { @@ -423,18 +431,16 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) } for (;;) { - int bufs; - if (!pipe->readers) { send_sig(SIGPIPE, current, 0); if (!ret) ret = -EPIPE; break; } - bufs = pipe->nrbufs; - if (bufs < pipe->buffers) { - int newbuf = (pipe->curbuf + bufs) & (pipe->buffers-1); - struct pipe_buffer *buf = pipe->bufs + newbuf; + + tail = pipe->tail; + if (!pipe_full(head, tail, max_usage)) { + struct pipe_buffer *buf = &pipe->bufs[head & mask]; struct page *page = pipe->tmp_page; int copied; @@ -470,14 +476,19 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) buf->ops = &packet_pipe_buf_ops; buf->flags = PIPE_BUF_FLAG_PACKET; } - pipe->nrbufs = ++bufs; + + head++; + pipe->head = head; pipe->tmp_page = NULL; if (!iov_iter_count(from)) break; } - if (bufs < pipe->buffers) + + if (!pipe_full(head, tail, max_usage)) continue; + + /* Wait for buffer space to become available. */ if (filp->f_flags & O_NONBLOCK) { if (!ret) ret = -EAGAIN; @@ -515,17 +526,19 @@ out: static long pipe_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct pipe_inode_info *pipe = filp->private_data; - int count, buf, nrbufs; + int count, head, tail, mask; switch (cmd) { case FIONREAD: __pipe_lock(pipe); count = 0; - buf = pipe->curbuf; - nrbufs = pipe->nrbufs; - while (--nrbufs >= 0) { - count += pipe->bufs[buf].len; - buf = (buf+1) & (pipe->buffers - 1); + head = pipe->head; + tail = pipe->tail; + mask = pipe->ring_size - 1; + + while (tail != head) { + count += pipe->bufs[tail & mask].len; + tail++; } __pipe_unlock(pipe); @@ -541,21 +554,25 @@ pipe_poll(struct file *filp, poll_table *wait) { __poll_t mask; struct pipe_inode_info *pipe = filp->private_data; - int nrbufs; + unsigned int head = READ_ONCE(pipe->head); + unsigned int tail = READ_ONCE(pipe->tail); poll_wait(filp, &pipe->wait, wait); + BUG_ON(pipe_occupancy(head, tail) > pipe->ring_size); + /* Reading only -- no need for acquiring the semaphore. */ - nrbufs = pipe->nrbufs; mask = 0; if (filp->f_mode & FMODE_READ) { - mask = (nrbufs > 0) ? EPOLLIN | EPOLLRDNORM : 0; + if (!pipe_empty(head, tail)) + mask |= EPOLLIN | EPOLLRDNORM; if (!pipe->writers && filp->f_version != pipe->w_counter) mask |= EPOLLHUP; } if (filp->f_mode & FMODE_WRITE) { - mask |= (nrbufs < pipe->buffers) ? EPOLLOUT | EPOLLWRNORM : 0; + if (!pipe_full(head, tail, pipe->ring_size)) + mask |= EPOLLOUT | EPOLLWRNORM; /* * Most Unices do not set EPOLLERR for FIFOs but on Linux they * behave exactly like pipes for poll(). @@ -679,7 +696,7 @@ struct pipe_inode_info *alloc_pipe_info(void) if (pipe->bufs) { init_waitqueue_head(&pipe->wait); pipe->r_counter = pipe->w_counter = 1; - pipe->buffers = pipe_bufs; + pipe->ring_size = pipe_bufs; pipe->user = user; mutex_init(&pipe->mutex); return pipe; @@ -697,9 +714,9 @@ void free_pipe_info(struct pipe_inode_info *pipe) { int i; - (void) account_pipe_buffers(pipe->user, pipe->buffers, 0); + (void) account_pipe_buffers(pipe->user, pipe->ring_size, 0); free_uid(pipe->user); - for (i = 0; i < pipe->buffers; i++) { + for (i = 0; i < pipe->ring_size; i++) { struct pipe_buffer *buf = pipe->bufs + i; if (buf->ops) pipe_buf_release(pipe, buf); @@ -880,7 +897,7 @@ SYSCALL_DEFINE1(pipe, int __user *, fildes) static int wait_for_partner(struct pipe_inode_info *pipe, unsigned int *cnt) { - int cur = *cnt; + int cur = *cnt; while (cur == *cnt) { pipe_wait(pipe); @@ -955,7 +972,7 @@ static int fifo_open(struct inode *inode, struct file *filp) } } break; - + case FMODE_WRITE: /* * O_WRONLY @@ -975,7 +992,7 @@ static int fifo_open(struct inode *inode, struct file *filp) goto err_wr; } break; - + case FMODE_READ | FMODE_WRITE: /* * O_RDWR @@ -1054,14 +1071,14 @@ unsigned int round_pipe_size(unsigned long size) static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) { struct pipe_buffer *bufs; - unsigned int size, nr_pages; + unsigned int size, nr_slots, head, tail, mask, n; unsigned long user_bufs; long ret = 0; size = round_pipe_size(arg); - nr_pages = size >> PAGE_SHIFT; + nr_slots = size >> PAGE_SHIFT; - if (!nr_pages) + if (!nr_slots) return -EINVAL; /* @@ -1071,13 +1088,13 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) * Decreasing the pipe capacity is always permitted, even * if the user is currently over a limit. */ - if (nr_pages > pipe->buffers && + if (nr_slots > pipe->ring_size && size > pipe_max_size && !capable(CAP_SYS_RESOURCE)) return -EPERM; - user_bufs = account_pipe_buffers(pipe->user, pipe->buffers, nr_pages); + user_bufs = account_pipe_buffers(pipe->user, pipe->ring_size, nr_slots); - if (nr_pages > pipe->buffers && + if (nr_slots > pipe->ring_size && (too_many_pipe_buffers_hard(user_bufs) || too_many_pipe_buffers_soft(user_bufs)) && is_unprivileged_user()) { @@ -1086,17 +1103,21 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) } /* - * We can shrink the pipe, if arg >= pipe->nrbufs. Since we don't - * expect a lot of shrink+grow operations, just free and allocate - * again like we would do for growing. If the pipe currently + * We can shrink the pipe, if arg is greater than the ring occupancy. + * Since we don't expect a lot of shrink+grow operations, just free and + * allocate again like we would do for growing. If the pipe currently * contains more buffers than arg, then return busy. */ - if (nr_pages < pipe->nrbufs) { + mask = pipe->ring_size - 1; + head = pipe->head; + tail = pipe->tail; + n = pipe_occupancy(pipe->head, pipe->tail); + if (nr_slots < n) { ret = -EBUSY; goto out_revert_acct; } - bufs = kcalloc(nr_pages, sizeof(*bufs), + bufs = kcalloc(nr_slots, sizeof(*bufs), GFP_KERNEL_ACCOUNT | __GFP_NOWARN); if (unlikely(!bufs)) { ret = -ENOMEM; @@ -1105,33 +1126,36 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) /* * The pipe array wraps around, so just start the new one at zero - * and adjust the indexes. + * and adjust the indices. */ - if (pipe->nrbufs) { - unsigned int tail; - unsigned int head; - - tail = pipe->curbuf + pipe->nrbufs; - if (tail < pipe->buffers) - tail = 0; - else - tail &= (pipe->buffers - 1); - - head = pipe->nrbufs - tail; - if (head) - memcpy(bufs, pipe->bufs + pipe->curbuf, head * sizeof(struct pipe_buffer)); - if (tail) - memcpy(bufs + head, pipe->bufs, tail * sizeof(struct pipe_buffer)); + if (n > 0) { + unsigned int h = head & mask; + unsigned int t = tail & mask; + if (h > t) { + memcpy(bufs, pipe->bufs + t, + n * sizeof(struct pipe_buffer)); + } else { + unsigned int tsize = pipe->ring_size - t; + if (h > 0) + memcpy(bufs + tsize, pipe->bufs, + h * sizeof(struct pipe_buffer)); + memcpy(bufs, pipe->bufs + t, + tsize * sizeof(struct pipe_buffer)); + } } - pipe->curbuf = 0; + head = n; + tail = 0; + kfree(pipe->bufs); pipe->bufs = bufs; - pipe->buffers = nr_pages; - return nr_pages * PAGE_SIZE; + pipe->ring_size = nr_slots; + pipe->tail = tail; + pipe->head = head; + return pipe->ring_size * PAGE_SIZE; out_revert_acct: - (void) account_pipe_buffers(pipe->user, nr_pages, pipe->buffers); + (void) account_pipe_buffers(pipe->user, nr_slots, pipe->ring_size); return ret; } @@ -1161,7 +1185,7 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) ret = pipe_set_size(pipe, arg); break; case F_GETPIPE_SZ: - ret = pipe->buffers * PAGE_SIZE; + ret = pipe->ring_size * PAGE_SIZE; break; default: ret = -EINVAL; -- cgit v1.2.3 From 6718b6f855a0b4962d54bd625be2718cb820cec6 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 16 Oct 2019 16:47:32 +0100 Subject: pipe: Allow pipes to have kernel-reserved slots Split pipe->ring_size into two numbers: (1) pipe->ring_size - indicates the hard size of the pipe ring. (2) pipe->max_usage - indicates the maximum number of pipe ring slots that userspace orchestrated events can fill. This allows for a pipe that is both writable by the general kernel notification facility and by userspace, allowing plenty of ring space for notifications to be added whilst preventing userspace from being able to pin too much unswappable kernel space. Signed-off-by: David Howells --- fs/fuse/dev.c | 8 ++++---- fs/pipe.c | 10 ++++++---- fs/splice.c | 26 +++++++++++++------------- include/linux/pipe_fs_i.h | 6 +++++- lib/iov_iter.c | 4 ++-- 5 files changed, 30 insertions(+), 24 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index c56011f95a87..423b6c657bf0 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -703,7 +703,7 @@ static int fuse_copy_fill(struct fuse_copy_state *cs) cs->pipebufs++; cs->nr_segs--; } else { - if (cs->nr_segs >= cs->pipe->ring_size) + if (cs->nr_segs >= cs->pipe->max_usage) return -EIO; page = alloc_page(GFP_HIGHUSER); @@ -879,7 +879,7 @@ static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page, struct pipe_buffer *buf; int err; - if (cs->nr_segs >= cs->pipe->ring_size) + if (cs->nr_segs >= cs->pipe->max_usage) return -EIO; err = unlock_request(cs->req); @@ -1341,7 +1341,7 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, if (!fud) return -EPERM; - bufs = kvmalloc_array(pipe->ring_size, sizeof(struct pipe_buffer), + bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer), GFP_KERNEL); if (!bufs) return -ENOMEM; @@ -1353,7 +1353,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->ring_size) { + if (pipe_occupancy(pipe->head, pipe->tail) + cs.nr_segs > pipe->max_usage) { ret = -EIO; goto out; } diff --git a/fs/pipe.c b/fs/pipe.c index e9b361cb093e..69afeab8a73a 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -404,7 +404,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) tail = pipe->tail; head = pipe->head; - max_usage = pipe->ring_size; + max_usage = pipe->max_usage; mask = pipe->ring_size - 1; /* We try to merge small writes */ @@ -571,7 +571,7 @@ pipe_poll(struct file *filp, poll_table *wait) } if (filp->f_mode & FMODE_WRITE) { - if (!pipe_full(head, tail, pipe->ring_size)) + if (!pipe_full(head, tail, pipe->max_usage)) mask |= EPOLLOUT | EPOLLWRNORM; /* * Most Unices do not set EPOLLERR for FIFOs but on Linux they @@ -696,6 +696,7 @@ struct pipe_inode_info *alloc_pipe_info(void) if (pipe->bufs) { init_waitqueue_head(&pipe->wait); pipe->r_counter = pipe->w_counter = 1; + pipe->max_usage = pipe_bufs; pipe->ring_size = pipe_bufs; pipe->user = user; mutex_init(&pipe->mutex); @@ -1150,9 +1151,10 @@ static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg) kfree(pipe->bufs); pipe->bufs = bufs; pipe->ring_size = nr_slots; + pipe->max_usage = nr_slots; pipe->tail = tail; pipe->head = head; - return pipe->ring_size * PAGE_SIZE; + return pipe->max_usage * PAGE_SIZE; out_revert_acct: (void) account_pipe_buffers(pipe->user, nr_slots, pipe->ring_size); @@ -1185,7 +1187,7 @@ long pipe_fcntl(struct file *file, unsigned int cmd, unsigned long arg) ret = pipe_set_size(pipe, arg); break; case F_GETPIPE_SZ: - ret = pipe->ring_size * PAGE_SIZE; + ret = pipe->max_usage * PAGE_SIZE; break; default: ret = -EINVAL; diff --git a/fs/splice.c b/fs/splice.c index 22b0a47a35c0..c521090a0469 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -199,7 +199,7 @@ ssize_t splice_to_pipe(struct pipe_inode_info *pipe, goto out; } - while (!pipe_full(head, tail, pipe->ring_size)) { + while (!pipe_full(head, tail, pipe->max_usage)) { struct pipe_buffer *buf = &pipe->bufs[head & mask]; buf->page = spd->pages[page_nr]; @@ -239,7 +239,7 @@ ssize_t add_to_pipe(struct pipe_inode_info *pipe, struct pipe_buffer *buf) if (unlikely(!pipe->readers)) { send_sig(SIGPIPE, current, 0); ret = -EPIPE; - } else if (pipe_full(head, tail, pipe->ring_size)) { + } else if (pipe_full(head, tail, pipe->max_usage)) { ret = -EAGAIN; } else { pipe->bufs[head & mask] = *buf; @@ -257,7 +257,7 @@ EXPORT_SYMBOL(add_to_pipe); */ int splice_grow_spd(const struct pipe_inode_info *pipe, struct splice_pipe_desc *spd) { - unsigned int max_usage = READ_ONCE(pipe->ring_size); + unsigned int max_usage = READ_ONCE(pipe->max_usage); spd->nr_pages_max = max_usage; if (max_usage <= PIPE_DEF_BUFFERS) @@ -381,7 +381,7 @@ static ssize_t default_file_splice_read(struct file *in, loff_t *ppos, ssize_t res; int i; - if (pipe_full(pipe->head, pipe->tail, pipe->ring_size)) + if (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) return -EAGAIN; /* @@ -698,7 +698,7 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out, .pos = *ppos, .u.file = out, }; - int nbufs = pipe->ring_size; + int nbufs = pipe->max_usage; struct bio_vec *array = kcalloc(nbufs, sizeof(struct bio_vec), GFP_KERNEL); ssize_t ret; @@ -721,9 +721,9 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out, if (ret <= 0) break; - if (unlikely(nbufs < pipe->ring_size)) { + if (unlikely(nbufs < pipe->max_usage)) { kfree(array); - nbufs = pipe->ring_size; + nbufs = pipe->max_usage; array = kcalloc(nbufs, sizeof(struct bio_vec), GFP_KERNEL); if (!array) { @@ -963,7 +963,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd, loff_t pos = sd->pos, prev_pos = pos; /* Don't try to read more the pipe has space for. */ - p_space = pipe->ring_size - + p_space = pipe->max_usage - pipe_occupancy(pipe->head, pipe->tail); read_len = min_t(size_t, len, p_space << PAGE_SHIFT); ret = do_splice_to(in, &pos, pipe, read_len, flags); @@ -1090,7 +1090,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->ring_size)) + if (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) return 0; if (flags & SPLICE_F_NONBLOCK) return -EAGAIN; @@ -1498,13 +1498,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->ring_size)) + if (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) return 0; ret = 0; pipe_lock(pipe); - while (pipe_full(pipe->head, pipe->tail, pipe->ring_size)) { + while (pipe_full(pipe->head, pipe->tail, pipe->max_usage)) { if (!pipe->readers) { send_sig(SIGPIPE, current, 0); ret = -EPIPE; @@ -1584,7 +1584,7 @@ retry: * pipe is empty or the output pipe is full. */ if (pipe_empty(i_head, i_tail) || - pipe_full(o_head, o_tail, opipe->ring_size)) { + pipe_full(o_head, o_tail, opipe->max_usage)) { /* Already processed some buffers, break */ if (ret) break; @@ -1706,7 +1706,7 @@ static int link_pipe(struct pipe_inode_info *ipipe, * output room, break. */ if (pipe_empty(i_head, i_tail) || - pipe_full(o_head, o_tail, opipe->ring_size)) + pipe_full(o_head, o_tail, opipe->max_usage)) break; ibuf = &ipipe->bufs[i_tail & i_mask]; diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index 96158ca80456..44f2245debda 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -32,6 +32,7 @@ struct pipe_buffer { * @wait: reader/writer wait point in case of empty/full pipe * @head: The point of buffer production * @tail: The point of buffer consumption + * @max_usage: The maximum number of slots that may be used in the ring * @ring_size: total number of buffers (should be a power of 2) * @tmp_page: cached released page * @readers: number of current readers of this pipe @@ -50,6 +51,7 @@ struct pipe_inode_info { wait_queue_head_t wait; unsigned int head; unsigned int tail; + unsigned int max_usage; unsigned int ring_size; unsigned int readers; unsigned int writers; @@ -150,9 +152,11 @@ static inline unsigned int pipe_space_for_user(unsigned int head, unsigned int t unsigned int p_occupancy, p_space; p_occupancy = pipe_occupancy(head, tail); - if (p_occupancy >= pipe->ring_size) + if (p_occupancy >= pipe->max_usage) return 0; p_space = pipe->ring_size - p_occupancy; + if (p_space > pipe->max_usage) + p_space = pipe->max_usage; return p_space; } diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 957f8828378a..be2fc5793ddd 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -395,7 +395,7 @@ static size_t copy_page_to_iter_pipe(struct page *page, size_t offset, size_t by i_head++; buf = &pipe->bufs[i_head & p_mask]; } - if (pipe_full(i_head, p_tail, pipe->ring_size)) + if (pipe_full(i_head, p_tail, pipe->max_usage)) return 0; buf->ops = &page_cache_pipe_buf_ops; @@ -528,7 +528,7 @@ static size_t push_pipe(struct iov_iter *i, size_t size, pipe->bufs[iter_head & p_mask].len = PAGE_SIZE; iter_head++; } - while (!pipe_full(iter_head, p_tail, pipe->ring_size)) { + while (!pipe_full(iter_head, p_tail, pipe->max_usage)) { struct pipe_buffer *buf = &pipe->bufs[iter_head & p_mask]; struct page *page = alloc_page(GFP_USER); if (!page) -- cgit v1.2.3 From b667b867344301e24f21d4a4c844675ff61d89e1 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 24 Sep 2019 16:09:04 +0100 Subject: pipe: Advance tail pointer inside of wait spinlock in pipe_read() Advance the pipe ring tail pointer inside of wait spinlock in pipe_read() so that the pipe can be written into with kernel notifications from contexts where pipe->mutex cannot be taken. Signed-off-by: David Howells --- fs/pipe.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 69afeab8a73a..ea134f69a292 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -325,9 +325,14 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (!buf->len) { pipe_buf_release(pipe, buf); + spin_lock_irq(&pipe->wait.lock); tail++; pipe->tail = tail; - do_wakeup = 1; + do_wakeup = 0; + wake_up_interruptible_sync_poll_locked( + &pipe->wait, EPOLLOUT | EPOLLWRNORM); + spin_unlock_irq(&pipe->wait.lock); + kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); } total_len -= chars; if (!total_len) @@ -359,6 +364,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) if (do_wakeup) { wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM); kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); + do_wakeup = 0; } pipe_wait(pipe); } -- cgit v1.2.3 From 8446487feba988a92e7649c60367510f0b0445a8 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 24 Sep 2019 16:07:04 +0100 Subject: pipe: Conditionalise wakeup in pipe_read() Only do a wakeup in pipe_read() if we made space in a completely full buffer. The producer shouldn't be waiting on pipe->wait otherwise. Signed-off-by: David Howells --- fs/pipe.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index ea134f69a292..c16950e36ded 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -328,11 +328,13 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) spin_lock_irq(&pipe->wait.lock); tail++; pipe->tail = tail; - do_wakeup = 0; - wake_up_interruptible_sync_poll_locked( - &pipe->wait, EPOLLOUT | EPOLLWRNORM); + do_wakeup = 1; + if (head - (tail - 1) == pipe->max_usage) + wake_up_interruptible_sync_poll_locked( + &pipe->wait, EPOLLOUT | EPOLLWRNORM); spin_unlock_irq(&pipe->wait.lock); - kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); + if (head - (tail - 1) == pipe->max_usage) + kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); } total_len -= chars; if (!total_len) @@ -361,11 +363,6 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) ret = -ERESTARTSYS; break; } - if (do_wakeup) { - wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM); - kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); - do_wakeup = 0; - } pipe_wait(pipe); } __pipe_unlock(pipe); -- cgit v1.2.3 From a194dfe6e6f6f7205eea850a420f2bc6a1541209 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 20 Sep 2019 16:32:19 +0100 Subject: pipe: Rearrange sequence in pipe_write() to preallocate slot Rearrange the sequence in pipe_write() so that the allocation of the new buffer, the allocation of a ring slot and the attachment to the ring is done under the pipe wait spinlock and then the lock is dropped and the buffer can be filled. The data copy needs to be done with the spinlock unheld and irqs enabled, so the lock needs to be dropped first. However, the reader can't progress as we're holding pipe->mutex. We also need to drop the lock as that would impact others looking at the pipe waitqueue, such as poll(), the consumer and a future kernel message writer. We just abandon the preallocated slot if we get a copy error. Future writes may continue it and a future read will eventually recycle it. Signed-off-by: David Howells --- fs/pipe.c | 51 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 18 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index c16950e36ded..ce77ac0d8901 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -387,7 +387,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) { struct file *filp = iocb->ki_filp; struct pipe_inode_info *pipe = filp->private_data; - unsigned int head, tail, max_usage, mask; + unsigned int head, max_usage, mask; ssize_t ret = 0; int do_wakeup = 0; size_t total_len = iov_iter_count(from); @@ -405,14 +405,13 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) goto out; } - tail = pipe->tail; head = pipe->head; max_usage = pipe->max_usage; mask = pipe->ring_size - 1; /* We try to merge small writes */ chars = total_len & (PAGE_SIZE-1); /* size of the last buffer */ - if (!pipe_empty(head, tail) && chars != 0) { + if (!pipe_empty(head, pipe->tail) && chars != 0) { struct pipe_buffer *buf = &pipe->bufs[(head - 1) & mask]; int offset = buf->offset + buf->len; @@ -441,8 +440,8 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) break; } - tail = pipe->tail; - if (!pipe_full(head, tail, max_usage)) { + head = pipe->head; + if (!pipe_full(head, pipe->tail, max_usage)) { struct pipe_buffer *buf = &pipe->bufs[head & mask]; struct page *page = pipe->tmp_page; int copied; @@ -455,40 +454,56 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) } pipe->tmp_page = page; } + + /* Allocate a slot in the ring in advance and attach an + * empty buffer. If we fault or otherwise fail to use + * it, either the reader will consume it or it'll still + * be there for the next write. + */ + spin_lock_irq(&pipe->wait.lock); + + head = pipe->head; + pipe->head = head + 1; + /* Always wake up, even if the copy fails. Otherwise * we lock up (O_NONBLOCK-)readers that sleep due to * syscall merging. * FIXME! Is this really true? */ - do_wakeup = 1; - copied = copy_page_from_iter(page, 0, PAGE_SIZE, from); - if (unlikely(copied < PAGE_SIZE && iov_iter_count(from))) { - if (!ret) - ret = -EFAULT; - break; - } - ret += copied; + wake_up_interruptible_sync_poll_locked( + &pipe->wait, EPOLLIN | EPOLLRDNORM); + + spin_unlock_irq(&pipe->wait.lock); + kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); /* 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 = copied; + buf->len = 0; buf->flags = 0; if (is_packetized(filp)) { buf->ops = &packet_pipe_buf_ops; buf->flags = PIPE_BUF_FLAG_PACKET; } - - head++; - pipe->head = head; pipe->tmp_page = NULL; + copied = copy_page_from_iter(page, 0, PAGE_SIZE, from); + if (unlikely(copied < PAGE_SIZE && iov_iter_count(from))) { + if (!ret) + ret = -EFAULT; + break; + } + ret += copied; + buf->offset = 0; + buf->len = copied; + if (!iov_iter_count(from)) break; } - if (!pipe_full(head, tail, max_usage)) + if (!pipe_full(head, pipe->tail, max_usage)) continue; /* Wait for buffer space to become available. */ -- cgit v1.2.3 From 7e25a73f1a52b58fc8206557e40d990cd791ad25 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 7 Oct 2019 15:10:06 +0100 Subject: pipe: Remove redundant wakeup from pipe_write() Remove a redundant wakeup from pipe_write(). Signed-off-by: David Howells --- fs/pipe.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index ce77ac0d8901..d7b8d3f22987 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -517,11 +517,6 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) ret = -ERESTARTSYS; break; } - if (do_wakeup) { - wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM); - kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); - do_wakeup = 0; - } pipe->waiting_writers++; pipe_wait(pipe); pipe->waiting_writers--; -- cgit v1.2.3 From 8df441294dd34fb0b16b45f550f55e96db6af4dc Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 7 Oct 2019 16:30:51 +0100 Subject: pipe: Check for ring full inside of the spinlock in pipe_write() Make pipe_write() check to see if the ring has become full between it taking the pipe mutex, checking the ring status and then taking the spinlock. This can happen if a notification is written into the pipe as that happens without the pipe mutex. Signed-off-by: David Howells --- fs/pipe.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index d7b8d3f22987..aba2455caabe 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -463,6 +463,11 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) spin_lock_irq(&pipe->wait.lock); head = pipe->head; + if (pipe_full(head, pipe->tail, max_usage)) { + spin_unlock_irq(&pipe->wait.lock); + continue; + } + pipe->head = head + 1; /* Always wake up, even if the copy fails. Otherwise -- cgit v1.2.3 From cefa80ced57a29179313da7ab3cbb26afb040b6f Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 31 Oct 2019 15:59:24 +0000 Subject: pipe: Increase the writer-wakeup threshold to reduce context-switch count Increase the threshold at which the reader sends a wake event to the writers in the queue such that the queue must be half empty before the wake is issued rather than the wake being issued when just a single slot available. This reduces the number of context switches in the tests significantly, without altering the amount of work achieved. With my pipe-bench program, there's a 20% reduction versus an unpatched kernel. Suggested-by: Konstantin Khlebnikov Signed-off-by: David Howells --- fs/pipe.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index aba2455caabe..9cd5cbef9552 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -324,16 +324,18 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) } if (!buf->len) { + bool wake; pipe_buf_release(pipe, buf); spin_lock_irq(&pipe->wait.lock); tail++; pipe->tail = tail; do_wakeup = 1; - if (head - (tail - 1) == pipe->max_usage) + wake = head - (tail - 1) == pipe->max_usage / 2; + if (wake) wake_up_interruptible_sync_poll_locked( &pipe->wait, EPOLLOUT | EPOLLWRNORM); spin_unlock_irq(&pipe->wait.lock); - if (head - (tail - 1) == pipe->max_usage) + if (wake) kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); } total_len -= chars; -- cgit v1.2.3 From 3c0edea9b29f9be6c093f236f762202b30ac9431 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 1 Nov 2019 20:52:24 +0000 Subject: pipe: Remove sync on wake_ups --- fs/pipe.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/pipe.c') diff --git a/fs/pipe.c b/fs/pipe.c index 9cd5cbef9552..c5e3765465f0 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -332,7 +332,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) do_wakeup = 1; wake = head - (tail - 1) == pipe->max_usage / 2; if (wake) - wake_up_interruptible_sync_poll_locked( + wake_up_locked_poll( &pipe->wait, EPOLLOUT | EPOLLWRNORM); spin_unlock_irq(&pipe->wait.lock); if (wake) @@ -371,7 +371,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to) /* Signal writers asynchronously that there is more room. */ if (do_wakeup) { - wake_up_interruptible_sync_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM); + wake_up_interruptible_poll(&pipe->wait, EPOLLOUT | EPOLLWRNORM); kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); } if (ret > 0) @@ -477,7 +477,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) * syscall merging. * FIXME! Is this really true? */ - wake_up_interruptible_sync_poll_locked( + wake_up_locked_poll( &pipe->wait, EPOLLIN | EPOLLRDNORM); spin_unlock_irq(&pipe->wait.lock); @@ -531,7 +531,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from) out: __pipe_unlock(pipe); if (do_wakeup) { - wake_up_interruptible_sync_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM); + wake_up_interruptible_poll(&pipe->wait, EPOLLIN | EPOLLRDNORM); kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); } if (ret > 0 && sb_start_write_trylock(file_inode(filp)->i_sb)) { -- cgit v1.2.3