Re: two pipe bugfixes

Andrea Arcangeli
2005-02-28T19:08:13Z

> > 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 ;)

;)