All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@suse.de>
To: Linus Torvalds <torvalds@osdl.org>
Cc: Andrew Morton <akpm@osdl.org>, linux-kernel@vger.kernel.org
Subject: Re: two pipe bugfixes
Date: Mon, 28 Feb 2005 20:04:37 +0100	[thread overview]
Message-ID: <20050228190437.GI8880@opteron.random> (raw)
In-Reply-To: <Pine.LNX.4.58.0502272143500.25732@ppc970.osdl.org>

> > IMHO the really wrong thing is that we always set POLLIN (even for
> > output filedescriptors that will never allow any data to be read).
> 
On Mon, Feb 28, 2005 at 08:25:07AM -0800, Linus Torvalds wrote:
> However, that has always been true. Look at the old code: it would set
> POLLIN for a non-empty pipe for both readers and writers (and do POLLOUT
> for empty pipes both for readers and writers). In fact, your very own
> original strace shows that - it shows "in [4]" even though fd 4 is a
> write-only fd.

Sure, that has always been true, I also wanted to say it wasn't a
mistake of the new code, but just a mistake of the old code that has
seen the light thanks to the recent optimizations.

> The new code does nothing really different. POLLIN is still there for a
> non-empty pipe, just like it was before. It's just that when you have
> multiple buffers, POLLOUT can _also_ be true, since even if you have
> _some_ data in the pipe, you can still do a write of a full PIPE_BUF.
> 
> So the difference is not at all the one you're talking about, and the 
> "bug" you claim to fix was there before too.
> 
> The fact is that if this broke python-twisted, then it just happened to
> work before by mistake. [..]

Yes of course.

> [..] And python-twisted is just plain bogus.

What do you mean with this, could you elaborate? You mean it shouldn't
check for in/out set at the same time? I've no idea why it got confused
by out/in set at the same time, but I guess it could be some
compatibility thing with some other os.

Still my point is that such code should never trigger since pollin
should never be set for an output-pipe-fd.

> That said, I agree with the fact that it's probably not the right thing to
> do, and never was. And if fixing it makes a difference to python-twisted,
> then hey, that's a benefit, but not a reason for the patch.

Sure, I had no idea myself if it was going to work with python-twisted,
because I changed the behaviour compared to the 2.6.9 codebase, but I
tested it and it worked fine as well as the "old 2.6.9" behaviour.

I didn't write the patch to make python-twisted work but only to do
something that would remotely resemble the sus specs and it happened to
fix twisted as well in my testing.

> I don't agree with your patch, though - I don't like your lack of
> parenthesis ;)

;)

  reply	other threads:[~2005-02-28 19:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-02-28  4:25 two pipe bugfixes Andrea Arcangeli
2005-02-28 16:25 ` Linus Torvalds
2005-02-28 19:04   ` Andrea Arcangeli [this message]
2005-02-28 19:22     ` Linus Torvalds
2005-02-28 19:55       ` Andrea Arcangeli
2005-02-28 20:29         ` Linus Torvalds

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=20050228190437.GI8880@opteron.random \
    --to=andrea@suse.de \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.