public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Alan J. Wylie" <alan@wylie.me.uk>,
	Thorsten Leemhuis <regressions@leemhuis.info>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
Date: Fri, 13 Jan 2017 21:55:04 +0000	[thread overview]
Message-ID: <20170113215504.GT1555@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20170113204731.GS1555@ZenIV.linux.org.uk>

On Fri, Jan 13, 2017 at 08:47:31PM +0000, Al Viro wrote:
> On Fri, Jan 13, 2017 at 12:32:37PM -0800, Linus Torvalds wrote:
> 
> > Ugh. I still think your patch is butt-ugly, and the index comparisons
> > make me nervous, but..
> 
> No arguments here - 6am on 20-odd hours of uptime is _not_ a good time
> for writing, especially since the data structure needs better documentation
> and probably a couple of inlined helpers.  I'll try to massage the damn
> thing into more readable form.

FWIW, I think it will be more readable if we separate the "advance" and
"truncate" parts like this (warning: not even build-tested).  Comments?

static inline void pipe_truncate(struct iov_iter *i)
{
        struct pipe_inode_info *pipe = i->pipe;
        if (pipe->nrbufs) {
                size_t off = i->iov_offset;
                int idx = i->idx;
                int n;

		n = (pipe->curbuf + pipe->nrbufs - idx) & (pipe->buffers - 1);
                if (off) {
                        pipe->bufs[idx].len = off - pipe->bufs[idx].offset;
                        /* free all after idx; n can't be 0 */
                        idx = next_idx(idx, pipe);
                        n--;
                } else {
                        /* free all _starting_ at idx.
                         * n is 0 when we have nothing to do
                         * *or* when we are truncating full pipe to empty.
                         */
                        if (pipe->nrbufs == pipe->buffers && !n)
                                n = pipe->buffers;
                }
                while (n--) {
                        pipe_buf_release(pipe, &pipe->bufs[idx]);
                        idx = next_idx(idx, pipe);
                        pipe->nrbufs--;
                }
        }
}

static void pipe_advance(struct iov_iter *i, size_t size)
{
        struct pipe_inode_info *pipe = i->pipe;
        if (unlikely(i->count < size))
                size = i->count;
        if (size) {
                struct pipe_buffer *buf;
                size_t off = i->iov_offset, left = size;
                int idx = i->idx;
                if (off) /* make it relative to the beginning of buffer */
                        left += off - pipe->bufs[idx].offset;
                while (1) {
                        buf = &pipe->bufs[idx];
                        if (left <= buf->len)
                                break;
                        left -= buf->len;
                        idx = next_idx(idx, pipe);
                }
                i->idx = idx;
                i->iov_offset = buf->offset + left;
		i->count -= size;
        }
        /* ... and discard everything past that point */
        pipe_truncate(i);
}

  reply	other threads:[~2017-01-13 21:55 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-12 20:26 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn Alan J. Wylie
2017-01-12 20:31 ` Al Viro
2017-01-12 20:38   ` Alan J. Wylie
2017-01-12 22:26 ` Linus Torvalds
2017-01-12 22:37   ` Al Viro
2017-01-12 22:46     ` Al Viro
2017-01-12 23:02       ` Linus Torvalds
2017-01-12 23:14         ` Al Viro
2017-01-12 23:14         ` Linus Torvalds
2017-01-12 23:27           ` Al Viro
2017-01-12 22:46   ` Alan J. Wylie
2017-01-12 22:58     ` Al Viro
2017-01-12 23:28     ` Linus Torvalds
2017-01-13  4:00       ` Al Viro
2017-01-13  7:38         ` Alan J. Wylie
2017-01-13  7:23       ` Alan J. Wylie
2017-01-13  9:33         ` Al Viro
2017-01-13  9:54           ` Alan J. Wylie
2017-01-13 10:20             ` Al Viro
2017-01-13 10:32               ` Alan J. Wylie
2017-01-13 11:25                 ` Al Viro
2017-01-13 11:18               ` Al Viro
2017-01-13 19:33                 ` Linus Torvalds
2017-01-13 20:08                   ` Al Viro
2017-01-13 20:11                     ` Al Viro
2017-01-13 20:32                       ` Linus Torvalds
2017-01-13 20:47                         ` Al Viro
2017-01-13 21:55                           ` Al Viro [this message]
2017-01-13 21:59                             ` Al Viro
2017-01-13 22:13                               ` Al Viro
2017-01-13 22:50                                 ` Al Viro
2017-01-14  0:59                                   ` Linus Torvalds
2017-01-14  1:24                                     ` Al Viro
2017-01-14  1:43                                       ` Al Viro
2017-01-14  1:46                                       ` Linus Torvalds
2017-01-14  1:57                                         ` Al Viro
2017-01-15  0:53                                           ` Al Viro
2017-01-14 13:16                                   ` Alan J. Wylie
2017-01-14 16:29                                     ` Alan J. Wylie
2017-01-14 17:57                                       ` Linus Torvalds
2017-01-13 20:08                   ` Linus Torvalds
2017-01-13 20:16                     ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170113215504.GT1555@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=alan@wylie.me.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=regressions@leemhuis.info \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox