| Message ID | 20220622041552.737754-12-viro@zeniv.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/44] 9p: handling Rerror without copy_from_iter_full() | expand |
On Wed, 2022-06-22 at 05:15 +0100, Al Viro wrote: > Unlike other copying operations on ITER_PIPE, copy_mc_to_iter() can > result in a short copy. In that case we need to trim the unused > buffers, as well as the length of partially filled one - it's not > enough to set ->head, ->iov_offset and ->count to reflect how > much had we copied. Not hard to fix, fortunately... > > I'd put a helper (pipe_discard_from(pipe, head)) into pipe_fs_i.h, > rather than iov_iter.c - it has nothing to do with iov_iter and > having it will allow us to avoid an ugly kludge in fs/splice.c. > We could put it into lib/iov_iter.c for now and move it later, > but I don't see the point going that way... > > Fixes: ca146f6f091e "lib/iov_iter: Fix pipe handling in _copy_to_iter_mcsafe()" > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > include/linux/pipe_fs_i.h | 9 +++++++++ > lib/iov_iter.c | 15 +++++++++++---- > 2 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h > index cb0fd633a610..4ea496924106 100644 > --- a/include/linux/pipe_fs_i.h > +++ b/include/linux/pipe_fs_i.h > @@ -229,6 +229,15 @@ static inline bool pipe_buf_try_steal(struct pipe_inode_info *pipe, > return buf->ops->try_steal(pipe, buf); > } > > +static inline void pipe_discard_from(struct pipe_inode_info *pipe, > + unsigned int old_head) > +{ > + unsigned int mask = pipe->ring_size - 1; > + > + while (pipe->head > old_head) > + pipe_buf_release(pipe, &pipe->bufs[--pipe->head & mask]); > +} > + > /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual > memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ > #define PIPE_SIZE PAGE_SIZE > diff --git a/lib/iov_iter.c b/lib/iov_iter.c > index 0b64695ab632..2bf20b48a04a 100644 > --- a/lib/iov_iter.c > +++ b/lib/iov_iter.c > @@ -689,6 +689,7 @@ static size_t copy_mc_pipe_to_iter(const void *addr, size_t bytes, > struct pipe_inode_info *pipe = i->pipe; > unsigned int p_mask = pipe->ring_size - 1; > unsigned int i_head; > + unsigned int valid = pipe->head; > size_t n, off, xfer = 0; > > if (!sanity(i)) > @@ -702,11 +703,17 @@ static size_t copy_mc_pipe_to_iter(const void *addr, size_t bytes, > rem = copy_mc_to_kernel(p + off, addr + xfer, chunk); > chunk -= rem; > kunmap_local(p); > - i->head = i_head; > - i->iov_offset = off + chunk; > - xfer += chunk; > - if (rem) > + if (chunk) { > + i->head = i_head; > + i->iov_offset = off + chunk; > + xfer += chunk; > + valid = i_head + 1; > + } > + if (rem) { > + pipe->bufs[i_head & p_mask].len -= rem; > + pipe_discard_from(pipe, valid); > break; > + } > n -= chunk; > off = 0; > i_head++; Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Wed, Jun 22, 2022 at 05:15:20AM +0100, Al Viro wrote: > Unlike other copying operations on ITER_PIPE, copy_mc_to_iter() can > result in a short copy. In that case we need to trim the unused > buffers, as well as the length of partially filled one - it's not > enough to set ->head, ->iov_offset and ->count to reflect how > much had we copied. Not hard to fix, fortunately... > > I'd put a helper (pipe_discard_from(pipe, head)) into pipe_fs_i.h, > rather than iov_iter.c - it has nothing to do with iov_iter and > having it will allow us to avoid an ugly kludge in fs/splice.c. > We could put it into lib/iov_iter.c for now and move it later, > but I don't see the point going that way... > > Fixes: ca146f6f091e "lib/iov_iter: Fix pipe handling in _copy_to_iter_mcsafe()" > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- Does that need a CC: stable@kernel.org # 4.19+ or sm? Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index cb0fd633a610..4ea496924106 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -229,6 +229,15 @@ static inline bool pipe_buf_try_steal(struct pipe_inode_info *pipe, return buf->ops->try_steal(pipe, buf); } +static inline void pipe_discard_from(struct pipe_inode_info *pipe, + unsigned int old_head) +{ + unsigned int mask = pipe->ring_size - 1; + + while (pipe->head > old_head) + pipe_buf_release(pipe, &pipe->bufs[--pipe->head & mask]); +} + /* Differs from PIPE_BUF in that PIPE_SIZE is the length of the actual memory allocation, whereas PIPE_BUF makes atomicity guarantees. */ #define PIPE_SIZE PAGE_SIZE diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 0b64695ab632..2bf20b48a04a 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -689,6 +689,7 @@ static size_t copy_mc_pipe_to_iter(const void *addr, size_t bytes, struct pipe_inode_info *pipe = i->pipe; unsigned int p_mask = pipe->ring_size - 1; unsigned int i_head; + unsigned int valid = pipe->head; size_t n, off, xfer = 0; if (!sanity(i)) @@ -702,11 +703,17 @@ static size_t copy_mc_pipe_to_iter(const void *addr, size_t bytes, rem = copy_mc_to_kernel(p + off, addr + xfer, chunk); chunk -= rem; kunmap_local(p); - i->head = i_head; - i->iov_offset = off + chunk; - xfer += chunk; - if (rem) + if (chunk) { + i->head = i_head; + i->iov_offset = off + chunk; + xfer += chunk; + valid = i_head + 1; + } + if (rem) { + pipe->bufs[i_head & p_mask].len -= rem; + pipe_discard_from(pipe, valid); break; + } n -= chunk; off = 0; i_head++;
Unlike other copying operations on ITER_PIPE, copy_mc_to_iter() can result in a short copy. In that case we need to trim the unused buffers, as well as the length of partially filled one - it's not enough to set ->head, ->iov_offset and ->count to reflect how much had we copied. Not hard to fix, fortunately... I'd put a helper (pipe_discard_from(pipe, head)) into pipe_fs_i.h, rather than iov_iter.c - it has nothing to do with iov_iter and having it will allow us to avoid an ugly kludge in fs/splice.c. We could put it into lib/iov_iter.c for now and move it later, but I don't see the point going that way... Fixes: ca146f6f091e "lib/iov_iter: Fix pipe handling in _copy_to_iter_mcsafe()" Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- include/linux/pipe_fs_i.h | 9 +++++++++ lib/iov_iter.c | 15 +++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-)