Re: two pipe bugfixes

From: Linus Torvalds
Date: Mon Feb 28 2005 - 11:27:53 EST




On Mon, 28 Feb 2005, Andrea Arcangeli wrote:
>
> and it started to return this since 2.6.11-rc:
>
> pipe([3, 4]) = 0
> write(4, "qqqqq", 5) = 5
> select(5, [4], [4], [4], {0, 0}) = 2 (in [4], out [4], left {0,
> 0})
> close(3) = 0
> write(4, "qqqqq", 5) = 5

Ahh, yes. The write merging code doesn't check for no readers.

> IMHO the really wrong thing is that we always set POLLIN (even for
> output filedescriptors that will never allow any data to be read).

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.

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. And python-twisted is just plain bogus.

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.

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

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/