[PATCH] [2.5] Non-blocking write can block

[PATCH] [2.5] Non-blocking write can block

[P. Benie]

Non-blocking writes to a tty will block if there is a blocking write
waiting for the atomic_write semaphore.

Peter

--- linux-2.5.70/drivers/char/tty_io.c.orig	2003-06-03 21:34:42.000000000 +0100
+++ linux-2.5.70/drivers/char/tty_io.c	2003-06-04 01:40:33.000000000 +0100
@@ -687,8 +687,13 @@
 {
 	ssize_t ret = 0, written = 0;

-	if (down_interruptible(&tty->atomic_write)) {
-		return -ERESTARTSYS;
+	if (file->f_flags & O_NONBLOCK) {
+		if (down_trylock(&tty->atomic_write))
+			return -EAGAIN;
+	}
+	else {
+		if (down_interruptible(&tty->atomic_write))
+			return -ERESTARTSYS;
 	}
 	if ( test_bit(TTY_NO_WRITE_SPLIT, &tty->flags) ) {
 		lock_kernel();

Re: [PATCH] [2.5] Non-blocking write can block

[Christoph Hellwig]

On Wed, Jun 04, 2003 at 01:58:02AM +0100, P. Benie wrote:
> -	if (down_interruptible(&tty->atomic_write)) {
> -		return -ERESTARTSYS;
> +	if (file->f_flags & O_NONBLOCK) {
> +		if (down_trylock(&tty->atomic_write))
> +			return -EAGAIN;
> +	}
> +	else {

The else should be on the same line as the closing brace, else
the patch looks fine.

Re: [PATCH] [2.5] Non-blocking write can block

[Linus Torvalds]

On Wed, 4 Jun 2003, Christoph Hellwig wrote:
> 
> The else should be on the same line as the closing brace, else
> the patch looks fine.

No no no, it's wrong.

If you do something like this, then you also have to teach "select()" 
about this, otherwise you just get busy looping in applications.

In general, we shouldn't do this, unless somebody can show an application 
where it really matters. Taking internal kernel locking into account for 
"blockingness" easily gets quite complicated, and there is seldom any real 
point to it.

Remember: perfect is the enemy of good. I'll happily apply the patch (if 
it also updates the tty poll() functionality), _if_ there is some 
real-world situation where it matters.

		Linus

Re: [PATCH] [2.5] Non-blocking write can block

[P. Benie]

On Wed, 4 Jun 2003, Linus Torvalds wrote:

> No no no, it's wrong.
>
> If you do something like this, then you also have to teach "select()"
> about this, otherwise you just get busy looping in applications.
>
> In general, we shouldn't do this, unless somebody can show an application
> where it really matters.

I wrote the patch to solve a real-world problem with wall(1), which
occasionally gets stuck writing to somebody's tty. I think it's reasonable
for wall to assume that non-blocking writes are non-blocking.

I'll think about how to do the patch correctly.

Peter

Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Timothy Miller]

Christoph Hellwig wrote:
> On Wed, Jun 04, 2003 at 01:58:02AM +0100, P. Benie wrote:
> 
>>-	if (down_interruptible(&tty->atomic_write)) {
>>-		return -ERESTARTSYS;
>>+	if (file->f_flags & O_NONBLOCK) {
>>+		if (down_trylock(&tty->atomic_write))
>>+			return -EAGAIN;
>>+	}
>>+	else {
> 
> 
> The else should be on the same line as the closing brace, else
> the patch looks fine.

I am in general agreement with those who feel we should have a common 
standard for code formatting.  There are particular places where it's 
VERY important to maximize consistency and readability, such as function 
headers.

But when do standards turn into nitpicks?

I personally always write else as you suggest, "} else {", but the way 
the other fellow did it does not in any way hurt readability for me. 
Yes, it does irritate me sometimes when people put the braces and else 
on three different lines, but mostly because it reduces the amount of 
code I can see at one time.  But even then, it doesn't make it any less 
readable to me.

I can see patches getting rejected because they violate function header 
standards.  That would make sense to me.  But if the above patch were to 
be rejected on the basis of the "else", I would be hard pressed to see 
that as a valid justification.

Perhaps it would be good to have an explanation for the relative 
importance of placing braces and else on the same line as compared to 
other formatting standards.

Re: [PATCH] [2.5] Non-blocking write can block

[Alan Cox]

On Mer, 2003-06-04 at 15:35, Linus Torvalds wrote:
> In general, we shouldn't do this, unless somebody can show an application 
> where it really matters. Taking internal kernel locking into account for 
> "blockingness" easily gets quite complicated, and there is seldom any real 
> point to it.

Hanging shutdown is the obvious one. With 2.0/2.2 we had a similar
problem and fixed it.

RE: [PATCH] [2.5] Non-blocking write can block

[Hua Zhong]

We ran into this problem here in an embedded environment. It causes
syslogd to hang and when this happens, everybody who talks to syslogd
hangs. Which means you may not even be able to login. In the end we used
exactly the same fix which seems to work.

I am curious to know the correct fix.

> On Wed, 4 Jun 2003, Christoph Hellwig wrote:
> > 
> > The else should be on the same line as the closing brace, else
> > the patch looks fine.
> 
> No no no, it's wrong.
> 
> If you do something like this, then you also have to teach "select()" 
> about this, otherwise you just get busy looping in applications.
> 
> In general, we shouldn't do this, unless somebody can show an 
> application 
> where it really matters. Taking internal kernel locking into 
> account for 
> "blockingness" easily gets quite complicated, and there is 
> seldom any real 
> point to it.
> 
> Remember: perfect is the enemy of good. I'll happily apply 
> the patch (if 
> it also updates the tty poll() functionality), _if_ there is some 
> real-world situation where it matters.
> 
> 		Linus
> 
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

RE: [PATCH] [2.5] Non-blocking write can block

[Linus Torvalds]

On Wed, 4 Jun 2003, Hua Zhong wrote:
>
> We ran into this problem here in an embedded environment. It causes
> syslogd to hang and when this happens, everybody who talks to syslogd
> hangs. Which means you may not even be able to login. In the end we used
> exactly the same fix which seems to work.
> 
> I am curious to know the correct fix.

[ First off: your embedded syslog problem is fixed by making sure that
  syslog doesn't try to write to a tty that somebody else might be
  blocked. In other words, to me it sounds like a "well, don't do that
  then" schenario, rather than a real kernel problem. ]

[ Secondly, you should all realize that O_NONBLOCK has _never_ meant that 
  the IO can't ever block. Even O_NONBLOCK reads and writes will always 
  block on things like having page faults on the user buffer, and a lot of 
  drivers still use the kernel lock and will block on that. O_NONBLOCK is 
  not an absolute "this is atomic" thing, it's a "don't wait for data if 
  there is none" thing ]

With that in mind, if you feel strongly about this particular path, then I
can only warn you that the correct fix actually looks fairly hard, as far
as I can tell. Yes, the posted patch is a small part of it, but the more
complex side is how to make poll() agree with the write semantics that the 
posted patch changed.

If you have a write() that returns -EAGAIN, and a poll() function that 
says "it's ok to write", any select-loop based application will start 
busy-looping calling poll/write, and use up 100% CPU time.

Which may be acceptable for some users, of course, but what you're doing
with the simple patch is just replacing one bug with another one. And I
personally think the bug you're introducing is the worse one.

But which bug you "prefer" ends up depending entirely on the machine load
and usage - the current behaviour has clearly not ended up in very many
complaints, and even if the patch fixes it for those few people didn't
like the historical behaviour, it may well end up breaking a hell of a lot
more distributions that until now were perfectly happy.

For example: what happens when your real-time application starts
busy-looping due to this?  Right. The system is totally _dead_, since the
application that is busy writing to the tty will never be scheduled.

And yes, something like syslogd could easily be marked high-priority in 
some setup. You do NOT want to make it busy-loop.

As to how to expand the patch to avoid the busy-loop: it's definitely 
non-trivial. Semaphores do not have poll() qualities, and I don't see a 
good way to get them. Something like

   static unsigned int tty_poll(struct file * filp, poll_table * wait)
   {
	struct tty_struct * tty;
	struct semaphore *sem;
	int retval;

	tty = (struct tty_struct *)filp->private_data;
	if (tty_paranoia_check(tty, filp->f_dentry->d_inode->i_rdev, "tty_poll"))
		return 0;

	sem = &tty->atomic_write;
	if (!down_trylock(sem)) {
		poll_wait(filp, sem->wait, wait);
		if (!down_trylock(sem))
			return 0;
	}
	retval = 0;
	if (tty->ldisc.poll)
		retval = tty->ldisc.poll(tty, filp, wait);
	up(sem);
	return retval;
   }

MIGHT work, but as you can see it actually now depends on knowing the 
internals of the semaphore implementation, and quite frankly I don't know 
if it works at all. As a result, I'm not horribly keen on the idea.

And as I tried to explain, I'm also not horribly keen on having a write()  
that doesn't match poll() and can cause busy looping. 

		Linus

RE: [PATCH] [2.5] Non-blocking write can block

[Mike Dresser]

On Wed, 4 Jun 2003, Hua Zhong wrote:

> We ran into this problem here in an embedded environment. It causes
> syslogd to hang and when this happens, everybody who talks to syslogd
> hangs. Which means you may not even be able to login. In the end we used
> exactly the same fix which seems to work.

I get this problem with writing to a remote syslog server, if the remote
syslog server hangs up or crashes no one can login to the machine that is
writing to the syslog server, even when the syslog server comes back.

Mike

Re: [PATCH] [2.5] Non-blocking write can block

[Linus Torvalds]

On 4 Jun 2003, Alan Cox wrote:
>
> On Mer, 2003-06-04 at 15:35, Linus Torvalds wrote:
> > In general, we shouldn't do this, unless somebody can show an application 
> > where it really matters. Taking internal kernel locking into account for 
> > "blockingness" easily gets quite complicated, and there is seldom any real 
> > point to it.
> 
> Hanging shutdown is the obvious one. With 2.0/2.2 we had a similar
> problem and fixed it.

As I tried to point out, the current patch on the table doesn't actually
"fix" anything, in that it can break things even _worse_ than the current
situation.

A much better fix might well be to actually not allow over-long tty writes
at all, and thus avoid the "block out" thing at the source of the problem,
instead of trying to make programs who play nice be the ones that suffer.

If somebody does a 1MB write to a tty, do we actually have any reason to 
try to make it so damn atomic and not return early?

		Linus

RE: [PATCH] [2.5] Non-blocking write can block

[Hua Zhong]

> -----Original Message-----
> From: Linus Torvalds [mailto:torvalds@transmeta.com] 
> Sent: Wednesday, June 04, 2003 10:42 AM
> To: Hua Zhong
> Cc: 'Christoph Hellwig'; 'P. Benie'; 'Kernel Mailing List'
> Subject: RE: [PATCH] [2.5] Non-blocking write can block
> 
> 
> 
> On Wed, 4 Jun 2003, Hua Zhong wrote:
> >
> > We ran into this problem here in an embedded environment. It causes
> > syslogd to hang and when this happens, everybody who talks to
syslogd
> > hangs. Which means you may not even be able to login. In the end we 
> > used exactly the same fix which seems to work.
> > 
> > I am curious to know the correct fix.
> 
> [ First off: your embedded syslog problem is fixed by making sure that
>   syslog doesn't try to write to a tty that somebody else might be
>   blocked. In other words, to me it sounds like a "well, don't do that
>   then" schenario, rather than a real kernel problem. ]

It's hard. The shell might be printing and you cannot prevent that.
 
That said, the main problem was somebody could be stuck in waiting for
tty *forever* and thus everyone who tries to write also hangs.

This particular patch is in 2.4.20 already. There is another patch in
2.4.20 (?) which seems to fix the "main problem" (the n_tty_write_wakeup
function in n_tty.c), but I didn't verify it.

RE: [PATCH] [2.5] Non-blocking write can block

[P. Benie]

On Wed, 4 Jun 2003, Hua Zhong wrote:
> This particular patch is in 2.4.20 already. There is another patch in
> 2.4.20 (?) which seems to fix the "main problem" (the n_tty_write_wakeup
> function in n_tty.c), but I didn't verify it.

Yes - that's because I submitted the patch ages ago.  All that means is
that the distributions are relying on it, not that the patch is correct!

Peter

RE: [PATCH] [2.5] Non-blocking write can block

[Linus Torvalds]

On Wed, 4 Jun 2003, Hua Zhong wrote:
>  
> That said, the main problem was somebody could be stuck in waiting for
> tty *forever* and thus everyone who tries to write also hangs.
> 
> This particular patch is in 2.4.20 already. There is another patch in
> 2.4.20 (?) which seems to fix the "main problem" (the n_tty_write_wakeup
> function in n_tty.c), but I didn't verify it.

Do y ou have that other patch handy? It sounds like that is the real cause 
of the problem, and the patch quoted originally in this thread was a 
(broken) work-around..

		Linus

RE: [PATCH] [2.5] Non-blocking write can block

[P. Benie]

On Wed, 4 Jun 2003, P. Benie wrote:

> On Wed, 4 Jun 2003, Hua Zhong wrote:
> > This particular patch is in 2.4.20 already. There is another patch in
> > 2.4.20 (?) which seems to fix the "main problem" (the n_tty_write_wakeup
> > function in n_tty.c), but I didn't verify it.
>
> Yes - that's because I submitted the patch ages ago.  All that means is
> that the distributions are relying on it, not that the patch is correct!

Sorry Hua, I wasn't reading your mail correctly. Please ignore the above
comment.

Peter

Re: [PATCH] [2.5] Non-blocking write can block

[P. Benie]

On Wed, 4 Jun 2003, Linus Torvalds wrote:
>
> A much better fix might well be to actually not allow over-long tty writes
> at all, and thus avoid the "block out" thing at the source of the problem,
> instead of trying to make programs who play nice be the ones that suffer.
>
> If somebody does a 1MB write to a tty, do we actually have any reason to
> try to make it so damn atomic and not return early?

The problem isn't to do with large writes. It's to do with any sequence of
writes that fills up the receive buffer, which is only 4K for N_TTY. If
the receiving program is suspended, the buffer will fill sooner or later.

I am half-tempted by this style of fix, but I can't help but feel that
we'll discover a huge set of programs that assume short writes never
happen if they aren't playing with signals.

It's also not as easy a fix as it sounds: for blocking writes, we've gone
into into ldisc.write and then in tty->driver->write before we discover
that that we can't write any bytes, by which time we already have the
write semaphore. I suspect that it requires just as much effort to ensure
that this case is handled correctly as it does to stop the non-blocking
write/poll loop.

I compared 2.4.20 and 2.5.70 to see if I could find the patch Hua
referred to. n_tty.c and pty.c look almost the same - I don't think the
patch is in 2.4.20.

Peter

Re: [PATCH] [2.5] Non-blocking write can block

[Linus Torvalds]

On Wed, 4 Jun 2003, P. Benie wrote:
> 
> The problem isn't to do with large writes. It's to do with any sequence of
> writes that fills up the receive buffer, which is only 4K for N_TTY. If
> the receiving program is suspended, the buffer will fill sooner or later.

Well, even then we could just drop the "write_atomic" lock. 

The thing is, I don't know what the tty atomicity guarantees are. I know 
what they are for pipes (quite reasonable), but tty's? 

		Linus

RE: [PATCH] [2.5] Non-blocking write can block

[Hua Zhong]

Oops, yes, that patch is already in 2.5. It got merged in 2.4 sometime
between 2.4.17 and 2.4.20..

> I compared 2.4.20 and 2.5.70 to see if I could find the patch Hua
> referred to. n_tty.c and pty.c look almost the same - I don't 
> think the
> patch is in 2.4.20.
> 
> Peter
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] [2.5] Non-blocking write can block

[P. Benie]

On Wed, 4 Jun 2003, Linus Torvalds wrote:

> On Wed, 4 Jun 2003, P. Benie wrote:
> > The problem isn't to do with large writes. It's to do with any sequence of
> > writes that fills up the receive buffer, which is only 4K for N_TTY. If
> > the receiving program is suspended, the buffer will fill sooner or later.
>
> Well, even then we could just drop the "write_atomic" lock.
>
> The thing is, I don't know what the tty atomicity guarantees are. I know
> what they are for pipes (quite reasonable), but tty's?

We don't have a PIPE_BUF-style atomicity guarantee, even though this would
be quite useful. This lock is only used to prevent simultaneous writes
from being interleaved. I've always assumed that when writes shouldn't be
interleaved, but I can't quote a source for that.

Peter

Re: [PATCH] [2.5] Non-blocking write can block

[Alan Cox]

On Mer, 2003-06-04 at 18:57, Linus Torvalds wrote:
> A much better fix might well be to actually not allow over-long tty writes
> at all, and thus avoid the "block out" thing at the source of the problem,
> instead of trying to make programs who play nice be the ones that suffer.
> 
> If somebody does a 1MB write to a tty, do we actually have any reason to 
> try to make it so damn atomic and not return early?

I would be concerned as to what applications rely in the tty write being done
completely before returning. OTOH I can't see any reason we can't drop the
atomicity part without dropping the 1Mb write will eventually write 1Mbyte
property. That would not seem to be a problem unless POSIX says otherwise ?

Re: [PATCH] [2.5] Non-blocking write can block

[Russell King]

On Wed, Jun 04, 2003 at 08:46:51PM +0100, P. Benie wrote:
> The problem isn't to do with large writes. It's to do with any sequence of
> writes that fills up the receive buffer, which is only 4K for N_TTY. If
> the receiving program is suspended, the buffer will fill sooner or later.

If the tty drivers buffer fills, we don't sleep in tty->driver->write,
but we return zero instead.  If we are in non-blocking mode, and we
haven't written any characters, we return -EAGAIN.  If we have, we
return the number of characters which the tty driver accepted.

However, the problem you are referring to is what happens if you have
a blocking process blocked in write_chan() in n_tty.c, and we have
a non-blocking process trying to write to the same tty.

Reading POSIX, it doesn't seem to be clear about our area of interest,
and I'd even say that it seems to be unspecified.

What are the pipe semantics in this case?  According to my reading of
POSIX write(), if you have a blocked non-blocking writer, a non-blocking
writer should receive EAGAIN.  It would seem sensible to apply the
same rules to terminal devices as well as pipes.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [PATCH] [2.5] Non-blocking write can block

[Davide Libenzi]

On Thu, 5 Jun 2003, Russell King wrote:

> On Wed, Jun 04, 2003 at 08:46:51PM +0100, P. Benie wrote:
> > The problem isn't to do with large writes. It's to do with any sequence of
> > writes that fills up the receive buffer, which is only 4K for N_TTY. If
> > the receiving program is suspended, the buffer will fill sooner or later.
>
> If the tty drivers buffer fills, we don't sleep in tty->driver->write,
> but we return zero instead.  If we are in non-blocking mode, and we
> haven't written any characters, we return -EAGAIN.  If we have, we
> return the number of characters which the tty driver accepted.
>
> However, the problem you are referring to is what happens if you have
> a blocking process blocked in write_chan() in n_tty.c, and we have
> a non-blocking process trying to write to the same tty.
>
> Reading POSIX, it doesn't seem to be clear about our area of interest,
> and I'd even say that it seems to be unspecified.

Given that a problem exist for certain apps, and given that the proposed
fix will *at least* have existing apps to behave funny, couldn't this
implemented as a feature of the fd (default off).
Something like O_REALLYNONBLOCK :)



- Davide

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Greg KH]

On Wed, Jun 04, 2003 at 11:21:41AM -0400, Timothy Miller wrote:
> 
> Perhaps it would be good to have an explanation for the relative 
> importance of placing braces and else on the same line as compared to 
> other formatting standards.

Please read Documentation/CodingStyle.

If you want more justification, read my OLS 2001 paper.

Hope this helps,

greg k-h

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Alex Goddard]

On Fri, 6 Jun 2003, Greg KH wrote:

> On Wed, Jun 04, 2003 at 11:21:41AM -0400, Timothy Miller wrote:
> > 
> > Perhaps it would be good to have an explanation for the relative 
> > importance of placing braces and else on the same line as compared to 
> > other formatting standards.
> 
> Please read Documentation/CodingStyle.
> 
> If you want more justification, read my OLS 2001 paper.

I think you mean 2002.  Good reading.

-- 
Alex Goddard
agoddard@purdue.edu

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Timothy Miller]

Greg KH wrote:
> On Wed, Jun 04, 2003 at 11:21:41AM -0400, Timothy Miller wrote:
> 
>>Perhaps it would be good to have an explanation for the relative 
>>importance of placing braces and else on the same line as compared to 
>>other formatting standards.
> 
> 
> Please read Documentation/CodingStyle.
> 
> If you want more justification, read my OLS 2001 paper.
> 


Well, the coding style you propose isn't exactly the way I do things 
(although it's pretty close).  For instance, I'm not accustomed to using 
a single tab character for indent.  Having more experience with X11 
internals than Linux, you can see where I would get my four-space indent 
habit from.  No matter.  I can easily adapt.

One thing I wanted to mention, however, is that your tongue-in-cheek 
style doesn't help you.  Coding style is something that needs to be 
taken seriously when you're setting standards.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jörn Engel]

On Mon, 9 June 2003 12:24:35 -0400, Timothy Miller wrote:
> 
> One thing I wanted to mention, however, is that your tongue-in-cheek 
> style doesn't help you.  Coding style is something that needs to be 
> taken seriously when you're setting standards.

Coding style is secondary.  It doesn't effect the compiled code.  That
simple.

In the case of the kernel, there is quite a bit of horrible coding
style.  But a working device driver for some hardware is always better
that no working device driver for some hardware, and if enforcing the
coding style more results is scaring away some driver writers, the
style clearly loses.

Jörn

-- 
They laughed at Galileo.  They laughed at Copernicus.  They laughed at
Columbus. But remember, they also laughed at Bozo the Clown.
-- unknown

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Davide Libenzi]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=X-UNKNOWN, Size: 1148 bytes --]

On Mon, 9 Jun 2003, [iso-8859-1] Jörn Engel wrote:

> In the case of the kernel, there is quite a bit of horrible coding
> style.  But a working device driver for some hardware is always better
> that no working device driver for some hardware, and if enforcing the
> coding style more results is scaring away some driver writers, the
> style clearly loses.

There's no such a thing as "horrible coding style", since coding style is
strictly personal. Whoever try to convince you that one style is better
than another one is simply plain wrong. Every reason they will give you to
justify one style can be wiped with other opposite reasons. The only
horrible coding style is to not respect coding standards when you work
inside a project. This is a form of respect for other people working
inside the project itself, give the project code a more professional
look and lower the fatigue of reading the project code. Jumping from 24
different coding styles does not usually help this. I do not believe
professional developers can be scared by a coding style, if this is the
coding style adopted by the project where they have to work in.



- Davide

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Eli Carter]

Davide Libenzi wrote:
> On Mon, 9 Jun 2003, [iso-8859-1] J?rn Engel wrote:
> 
> 
>>In the case of the kernel, there is quite a bit of horrible coding
>>style.  But a working device driver for some hardware is always better
>>that no working device driver for some hardware, and if enforcing the
>>coding style more results is scaring away some driver writers, the
>>style clearly loses.
> 
> 
> There's no such a thing as "horrible coding style", since coding style is
> strictly personal. Whoever try to convince you that one style is better
> than another one is simply plain wrong. Every reason they will give you to
> justify one style can be wiped with other opposite reasons. The only
> horrible coding style is to not respect coding standards when you work
> inside a project.

I beg to differ: http://www0.us.ioccc.org/2001/anonymous.c ;)

Eli
--------------------. "If it ain't broke now,
Eli Carter           \                  it will be soon." -- crypto-gram
eli.carter(a)inet.com `-------------------------------------------------

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Richard B. Johnson]

On Mon, 9 Jun 2003, Eli Carter wrote:

> Davide Libenzi wrote:
> > On Mon, 9 Jun 2003, [iso-8859-1] J?rn Engel wrote:
> >
> >
> >>In the case of the kernel, there is quite a bit of horrible coding
> >>style.  But a working device driver for some hardware is always better
> >>that no working device driver for some hardware, and if enforcing the
> >>coding style more results is scaring away some driver writers, the
> >>style clearly loses.
> >
> >
> > There's no such a thing as "horrible coding style", since coding style is
> > strictly personal. Whoever try to convince you that one style is better
> > than another one is simply plain wrong. Every reason they will give you to
> > justify one style can be wiped with other opposite reasons. The only
> > horrible coding style is to not respect coding standards when you work
> > inside a project.
>
> I beg to differ: http://www0.us.ioccc.org/2001/anonymous.c ;)
>
> Eli

Last I looked, we had a good example in the Buslogic SCSI driver.
However, just in case it's been changed, I submit herewith an
example of real code written by a "professional".

//
//  This is an example of the kind of 'C' code that is being written
//  by so-called experts. It is unreadable, illogical, but it works.
//  I wish I was kidding!  This is the junk I see being written right
//  now by so-called professional programmers!
//  Richard B. Johnson                              rjohnson@analogic.com
//
//
#include<stdio.h>
#define SuccessfulReturnValue 0
typedef int MainReturnType;
typedef int DefaultCounterType;
typedef void NothingWeCareAbout;
typedef const char StringThatIsntGoingToBeModified;
typedef char StringThatCanBeModified;
MainReturnType main(NothingWeCareAbout);
StringThatIsntGoingToBeModified MessageToBeWrittenToTheScreen[]={0x48,0x64,0x6e,0x6f,0x6b,0x25,0x71,0x68,0x7a,0x65,0x6e,0x2a,0x0c};
MainReturnType main(){
    StringThatCanBeModified LocalStringBuffer[sizeof(MessageToBeWrittenToTheScreen)];
    DefaultCounterType CharacterCounter;
    for(CharacterCounter=0;CharacterCounter<sizeof(MessageToBeWrittenToTheScreen);CharacterCounter++)
        LocalStringBuffer[CharacterCounter]=MessageToBeWrittenToTheScreen[CharacterCounter]^CharacterCounter;
    puts(LocalStringBuffer);
    return SuccessfulReturnValue;
}

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Davide Libenzi]

yOn Mon, 9 Jun 2003, Richard B. Johnson wrote:

> Last I looked, we had a good example in the Buslogic SCSI driver.
> However, just in case it's been changed, I submit herewith an
> example of real code written by a "professional".

You know why the code you reported is *wrong* (besides from how
techincally do things) ? Mixing lower and upper case, using long variable
and function names, etc... are simply a matter of personal taste and you
cannot say that such code is "absolutely" wrong. The code is damn wrong
because it violates about 25 sections of the project's defined CodingStyle
document, that's why it is wrong.



- Davide

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jörn Engel]

On Mon, 9 June 2003 11:07:32 -0700, Davide Libenzi wrote:
> 
> You know why the code you reported is *wrong* (besides from how
> techincally do things) ? Mixing lower and upper case, using long variable
> and function names, etc... are simply a matter of personal taste and you
> cannot say that such code is "absolutely" wrong. The code is damn wrong
> because it violates about 25 sections of the project's defined CodingStyle
> document, that's why it is wrong.

Call it as you may.  Whether some style violates more sections of the
CodingStyle than exist in written form or it hurts the taste of 99% of
all developers ever having to tough it, my short form for that is "bad
style".

Point remains, there is a lot of "bad style" and inconsistency in the
kernel.  But fixing all of it and keeping it fixed would result in a
lot of work and maybe a couple of device drivers less.  For what gain?

Jörn

-- 
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Timothy Miller]

Jörn Engel wrote:
> On Mon, 9 June 2003 12:24:35 -0400, Timothy Miller wrote:
> 
>>One thing I wanted to mention, however, is that your tongue-in-cheek 
>>style doesn't help you.  Coding style is something that needs to be 
>>taken seriously when you're setting standards.
> 
> 
> Coding style is secondary.  It doesn't effect the compiled code.  That
> simple.

Agreed.

> In the case of the kernel, there is quite a bit of horrible coding
> style.  But a working device driver for some hardware is always better
> that no working device driver for some hardware, and if enforcing the
> coding style more results is scaring away some driver writers, the
> style clearly loses.

It is a trivial fact that all coding styles are completely arbitrary. 
Yes, there may be many things which are chosen because they make the 
most sense, but there are always numerous choices along the way, all of 
which would be reasonable, that have to be reduced to one.  Some 
philosophers will tell you that all of reality is completely arbitrary 
and made up; of course, they're referring to our perceptions and choices 
moreso than to, say, physics.  Well, what exemplifies arbitrary reality 
more than computer science?  Every last drop of it was invented out of 
whole cloth.  So when you think about it, the C syntax itself is 
arbitrary, and thus even moreso are the coding styles.

But we have a practical goal in mind here.  Not only does something have 
to WORK (compile to working machine code), but our grandchildren, using 
Linux 20.14.6 are going to have to be able to make sense out of what we 
wrote.  Were it not for the fact that Linux is a collaborative project, 
we would not need these standards.

So, yes, while it may seem silly to do it "just because K&R did it that 
way", it is nevertheless a reasonable (albeit arbitrary) choice to make. 
  Someone has to make the choice, enforce it, and make sure that 
everyone understands it.  If there is one style, then it will be easier 
for new people to understand it once they have read the style guide.

Still, it IS nice to have someone produce justification for their 
choices once in a while.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Timothy Miller]

Davide Libenzi wrote:
> On Mon, 9 Jun 2003, [iso-8859-1] J?rn Engel wrote:
> 
> 
>>In the case of the kernel, there is quite a bit of horrible coding
>>style.  But a working device driver for some hardware is always better
>>that no working device driver for some hardware, and if enforcing the
>>coding style more results is scaring away some driver writers, the
>>style clearly loses.
> 
> 
> There's no such a thing as "horrible coding style", since coding style is
> strictly personal. Whoever try to convince you that one style is better
> than another one is simply plain wrong. Every reason they will give you to
> justify one style can be wiped with other opposite reasons. The only
> horrible coding style is to not respect coding standards when you work
> inside a project. This is a form of respect for other people working
> inside the project itself, give the project code a more professional
> look and lower the fatigue of reading the project code. Jumping from 24
> different coding styles does not usually help this. I do not believe
> professional developers can be scared by a coding style, if this is the
> coding style adopted by the project where they have to work in.

Oh, yes, there is most certainly "horrible coding style".  When I was in 
college, I met one CS student after another who really just did not 
belong in CS, and you should have seen the code they wrote.

Imagine a 200 line program which is ALL inside of main().  There is no 
indenting.  Lines of code are broken in random places.  Blank lines are 
inserted randomly.  The variable names chosen are a, b, c, d, e, etc. 
It's impossible to tell which '{' is associated with which '}'.

It's been a while.  I can't remember all of the violations of reason and 
sanity I saw.  I pity the grad students who were faced with grading 
these monstrosities.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Davide Libenzi]

On Mon, 9 Jun 2003, Timothy Miller wrote:

> > There's no such a thing as "horrible coding style", since coding style is
> > strictly personal. Whoever try to convince you that one style is better
> > than another one is simply plain wrong. Every reason they will give you to
> > justify one style can be wiped with other opposite reasons. The only
> > horrible coding style is to not respect coding standards when you work
> > inside a project. This is a form of respect for other people working
> > inside the project itself, give the project code a more professional
> > look and lower the fatigue of reading the project code. Jumping from 24
> > different coding styles does not usually help this. I do not believe
> > professional developers can be scared by a coding style, if this is the
> > coding style adopted by the project where they have to work in.
>
> Oh, yes, there is most certainly "horrible coding style".  When I was in
> college, I met one CS student after another who really just did not
> belong in CS, and you should have seen the code they wrote.


> On Mon, 9 June 2003 11:07:32 -0700, Davide Libenzi wrote:
> >
> > You know why the code you reported is *wrong* (besides from how
> > techincally do things) ? Mixing lower and upper case, using long variable
> > and function names, etc... are simply a matter of personal taste and you
> > cannot say that such code is "absolutely" wrong. The code is damn wrong
> > because it violates about 25 sections of the project's defined CodingStyle
> > document, that's why it is wrong.
>
> Call it as you may.  Whether some style violates more sections of the
> CodingStyle than exist in written form or it hurts the taste of 99% of
> all developers ever having to tough it, my short form for that is "bad
> style".
>
> Point remains, there is a lot of "bad style" and inconsistency in the
> kernel.  But fixing all of it and keeping it fixed would result in a
> lot of work and maybe a couple of device drivers less.  For what gain?

If you try to define a bad/horrible "whatever" in an *absolute* way you
need either the *absolutely* unanimous consent or you need to prove it
using a logical combination of already proven absolute concepts. Since you
missing both of these requirements you cannot say that something is
bad/wrong in an absolute way. You can say though that something is
wrong/bad when dropped inside a given context, and a coding standard might
work as an example. If you try to approach a developer by saying that he
has to use ABC coding standard because it is better that his XYZ coding
standard you're just wrong and you'll have hard time to have him to
understand why he has to use the suggested standard when coding inside the
project JKL. The coding standard gives you the *rule* to define something
wrong when seen inside a given context, since your personal judgement does
not really matter here.



- Davide

RE: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[David Schwartz]

> If you try to define a bad/horrible "whatever" in an *absolute* way you
> need either the *absolutely* unanimous consent or you need to prove it
> using a logical combination of already proven absolute concepts. Since you
> missing both of these requirements you cannot say that something is
> bad/wrong in an absolute way. You can say though that something is
> wrong/bad when dropped inside a given context, and a coding standard might
> work as an example. If you try to approach a developer by saying that he
> has to use ABC coding standard because it is better that his XYZ coding
> standard you're just wrong and you'll have hard time to have him to
> understand why he has to use the suggested standard when coding inside the
> project JKL. The coding standard gives you the *rule* to define something
> wrong when seen inside a given context, since your personal judgement does
> not really matter here.
>
> - Davide

	This is just bad philosophy. You might as well argue that a canvas that's
been randomly pissed on is just as much art as the Mona Lisa. In fact, it's
a worse argument than that because coding styles aim at objective,
measurable goals. Why does consent matter? If some imbecile wants to argue
that it's good to write code that's hard to understand and debug, why should
we care what he has to say? The consent of people whose opinions are
nonsensical is of no value to people who are trying to create rules that
meet their objective requirements.

	Coding styles aim at specific measurable goals. Code should be easy to
understand, extend, and debug. If someone argues code should be hard to
understand, maintain, and debug, we just ignore him. We don't care if he
agrees with us or not because his opinion is obviously (and objectively) of
no value.

	We can measure, for different coding style, how long it takes to find a
bug. We can measure how long it takes a new programmer to get to the point
that he can contribute to the existing code.

	Coding styles are engineering rules. We can validate them based upon the
results they produce. Objective, measureable results.

	DS

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jörn Engel]

On Mon, 9 June 2003 11:58:43 -0700, Davide Libenzi wrote:
> 
> If you try to define a bad/horrible "whatever" in an *absolute* way you
> need either the *absolutely* unanimous consent or you need to prove it
> using a logical combination of already proven absolute concepts. Since you
> missing both of these requirements you cannot say that something is
> bad/wrong in an absolute way. You can say though that something is
> wrong/bad when dropped inside a given context, and a coding standard might
> work as an example. If you try to approach a developer by saying that he
> has to use ABC coding standard because it is better that his XYZ coding
> standard you're just wrong and you'll have hard time to have him to
> understand why he has to use the suggested standard when coding inside the
> project JKL. The coding standard gives you the *rule* to define something
> wrong when seen inside a given context, since your personal judgement does
> not really matter here.

The definition in an absolute way is not the real problem.  The real
problem is that good/bad coding style has more than just one
dimension.  Trying to rate it in just one dimension will almost always
fail.

That said, this discussion appears to have zero impact on the kernel
itself, so it might be time to fade it out.

Jörn

-- 
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jörn Engel]

On Mon, 9 June 2003 14:44:53 -0400, Timothy Miller wrote:
> 
> But we have a practical goal in mind here.  Not only does something have 
> to WORK (compile to working machine code), but our grandchildren, using 
> Linux 20.14.6 are going to have to be able to make sense out of what we 
> wrote.  Were it not for the fact that Linux is a collaborative project, 
> we would not need these standards.

Nice picture.  That implies that coding standards don't matter for
device drivers for some short-lived hardware like drivers/cdrom/ but
do a lot more for core code like mm/.

All right, let's stop beating the grass while there is still a shadow
of the dead horse remaining.

Jörn

-- 
Write programs that do one thing and do it well. Write programs to work
together. Write programs to handle text streams, because that is a
universal interface. 
-- Doug MacIlroy

RE: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Davide Libenzi]

On Mon, 9 Jun 2003, David Schwartz wrote:

> 	This is just bad philosophy.

Actually, that's logic/mathematics. Philosophy is on the other side. We
can say that mathematic/logic compare to philosophy like facts to bullshits.


> been randomly pissed on is just as much art as the Mona Lisa. In fact, it's
> a worse argument than that because coding styles aim at objective,
> measurable goals. Why does consent matter? If some imbecile wants to argue
> that it's good to write code that's hard to understand and debug, why should
> we care what he has to say? The consent of people whose opinions are
> nonsensical is of no value to people who are trying to create rules that
> meet their objective requirements.

A coding style is a very personal thing, you cannot say that the XYZ's
coding style is wrong. Period. Is like saying that your taste for cars is
wrong because you picked up an ABC against a JKL. If you say that XYZ's
coding style is wrong, without dropping it inside a specific context
(like an environment ruled by a coding standard for example), you
are trying to give an absolute judgement of it. Absolute judgements need
either absolutely unanimous consent or they need to be proven using a set
of already proven absolute concepts. You can say that *for you* (for-you
== relative) this is right :

	if (a == b) {
		...
	}

while this is wrong :

	if( a == b )
	{
		...
	}

I might agree with you, that makes two. But there will be for sure someone
that will personally prefer the latter. And this will break the unanimous
consent. Are you able to prove using a set of already proven absolute
concepts that the former is right and the latter is wrong ? The only way
that you have to say that something personal like a coding style is
"wrong" is through a set of rules like a coding standard document. So, to
close the circle, a coding standard document (like the one we have) more
than your very personal judgement, enable you to say that some code is
wrong. If you fail to understand this you will have hard times to
gracefully convince your developers why it is good to use the dictated
coding standard inside professional projects. The "your style sux" is not
generally well accepted by persons with serious attitude problems like
developers.



- Davide

RE: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Nigel Cunningham]

On Tue, 2003-06-10 at 10:55, Davide Libenzi wrote:
> Absolute judgements need either absolutely unanimous consent or they
> need to be proven using a set...

Of course unanimous consent doesn't make a judgement absolute. It just
makes it universally accepted (it might still be wrong or an opinion,
depending on the context).

Regards,

Nigel

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[James Stevenson]

> There's no such a thing as "horrible coding style", since coding style is
> strictly personal. 

yeah i think there is.

GetHandleToChangeLightBulbA
SetHandleToChangeLightBuldA

Need i say more ?

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jesse Pollard]

On Monday 09 June 2003 13:55, Timothy Miller wrote:
> Davide Libenzi wrote:
>
> > There's no such a thing as "horrible coding style", since coding style is
> > strictly personal. Whoever try to convince you that one style is better
> > than another one is simply plain wrong. Every reason they will give you
> > to justify one style can be wiped with other opposite reasons. The only
> > horrible coding style is to not respect coding standards when you work
> > inside a project. This is a form of respect for other people working
> > inside the project itself, give the project code a more professional look
> > and lower the fatigue of reading the project code. Jumping from 24
> > different coding styles does not usually help this. I do not believe
> > professional developers can be scared by a coding style, if this is the
> > coding style adopted by the project where they have to work in.
>
> Oh, yes, there is most certainly "horrible coding style".  When I was in
> college, I met one CS student after another who really just did not
> belong in CS, and you should have seen the code they wrote.
>
> Imagine a 200 line program which is ALL inside of main().  There is no
> indenting.  Lines of code are broken in random places.  Blank lines are
> inserted randomly.  The variable names chosen are a, b, c, d, e, etc.
> It's impossible to tell which '{' is associated with which '}'.
>
> It's been a while.  I can't remember all of the violations of reason and
> sanity I saw.  I pity the grad students who were faced with grading
> these monstrosities.

ummm been there... Actually, after the first 20 it got easy... If I couldn't
read it, it got an "F" (whether it worked or not).

If it could be read with difficulty (and worked) it got a D
If it could be read and worked it got a C
If it could be read and was clear (and worked) it got a B
If it was short, clear, and worked it got an A

And I have met some of the idiots (including Piled higher and Deeper ones) 
that couldn't program their way through a "hello there" program.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Jesse Pollard]

On Monday 09 June 2003 13:58, Davide Libenzi wrote:
[snip]
>
> If you try to define a bad/horrible "whatever" in an *absolute* way you
> need either the *absolutely* unanimous consent or you need to prove it
> using a logical combination of already proven absolute concepts. Since you
> missing both of these requirements you cannot say that something is
> bad/wrong in an absolute way. You can say though that something is
> wrong/bad when dropped inside a given context, and a coding standard might
> work as an example. If you try to approach a developer by saying that he
> has to use ABC coding standard because it is better that his XYZ coding
> standard you're just wrong and you'll have hard time to have him to
> understand why he has to use the suggested standard when coding inside the
> project JKL. The coding standard gives you the *rule* to define something
> wrong when seen inside a given context, since your personal judgement does
> not really matter here.

The coding standards were written by people who said

"Do it this way because 'I' have to read it and understand it to be able to 
maintain it."

Nuff said.

Re: Coding standards. (Was: Re: [PATCH] [2.5] Non-blocking write can block)

[Davide Libenzi]

On Tue, 10 Jun 2003, Jesse Pollard wrote:

> On Monday 09 June 2003 13:58, Davide Libenzi wrote:
> [snip]
> >
> > If you try to define a bad/horrible "whatever" in an *absolute* way you
> > need either the *absolutely* unanimous consent or you need to prove it
> > using a logical combination of already proven absolute concepts. Since you
> > missing both of these requirements you cannot say that something is
> > bad/wrong in an absolute way. You can say though that something is
> > wrong/bad when dropped inside a given context, and a coding standard might
> > work as an example. If you try to approach a developer by saying that he
> > has to use ABC coding standard because it is better that his XYZ coding
> > standard you're just wrong and you'll have hard time to have him to
> > understand why he has to use the suggested standard when coding inside the
> > project JKL. The coding standard gives you the *rule* to define something
> > wrong when seen inside a given context, since your personal judgement does
> > not really matter here.
>
> The coding standards were written by people who said
>
> "Do it this way because 'I' have to read it and understand it to be able to
> maintain it."

The whole sub-thread wasn't talking about democracy in coding styles ;)



- Davide

RE: [PATCH] [2.5] Non-blocking write can block

[Robert White]

(A quick food-for-thought note here...)

I hate to bring it up, but I am fairly certain that this argument is
part-and-parcel of the original AT&T "STREAMS"  (as opposed to "Streams" or
"streams", talk about namespace pollution 8-)interface.

Basically most devices (particularly character and network devices) were
implemented as a pair of queues of messages (one upstream one downstream)
and a writer would compose his entire text into a message (a linked list of
message buffers) which could be atomically placed on the write queue without
any consideration for device internal buffering.

[Between the "head" of the file handle and the actual driver you could
interpose translators and such by forming chains of queues, but that isn't
germane here.]

In such a usage, the writeability of a device is predicated on the
availability of a message buffer from some pool and some (in AT&T's case
some "lame-ass slightly less than Linux'es idea of a") kernel thread does
the pumping from the list to the device logic itself.

Without some sort of "above the driver but below the write" dynamic
buffering you almost inevitably get into a squeeze-the-balloon bug shuffling
situation.  The three bugs already mentioned here are write-interleaving,
odd blocking, and busy waiting.

Basically the peek-ahead on writeability is the relatively simple test: "is
there a free buffer in the pool of any size?" (because writeability is "can
accept at least one character.") and the anti-interleaving warrant is set at
multiples of "smallest buffer pool entry size".

The important thing is that the only lock contention window takes place for
the predictable time of putting the filled buffer onto or getting it off of
the queue (linked list pointer juggling time).

So why did I bring it up?

It seems to me that this can't really be fixed unilaterally at the driver
level without putting in a heck of a lot of scaffolding (e.g. evil STREAMS
8-).

The specific fix, however, might be easy at a fairly high level (line
discipline level?) with a fairly straight-forward linked list of buffers
thing.  With a fixed (or variable sized for that matter) pool of buffers,
the poll() could become a simple look-aside for a buffer well before the
branching internal logic/locking is otherwise referenced.  A
false-positive/race on the poll-to-get-buffer branch remains possible, but
unlikely to loop.

Basically you would be buying the correction in several ways:

1) raise the granularity.  With the warrant for an atomic write raised to
"one buffer" you increase the likely hood that any one operation will get in
and get out all at once.

2) add an insulating layer.  With the buffers rotating in and out via
pointer juggling, a very-short (spinlock) duration locking behavior is
placed between the fast world of the calling processes and the slow world of
serial I/O (and its analogues)

3) dynamic buffering.  Since the writes are called with reasonably sleepable
user context (e.g. enough latitude to do a memory allocate) "extra buffers"
are "always available" (though at the far end of this is a nasty DOR attack
8-) if circumstances or tuning makes such allocation desirable.

4) look aside used to determine write space availability.  (really a "2a"
thought) the list and pool of buffers approach would net you a look-aside
for the question of "can I write" so there is no contention between what the
driver is actually doing and the applicability of the question itself.

5) POSIX (et al) is (if I recall) silent about the size and nature of the
write buffer for a tty device.

uh... but it's just a thought... 8-)

Rob.


P. Benie wrote:

> On Wed, 4 Jun 2003, Linus Torvalds wrote:

> > On Wed, 4 Jun 2003, P. Benie wrote:
> > > The problem isn't to do with large writes. It's to do with any
sequence of
> > > writes that fills up the receive buffer, which is only 4K for N_TTY.
If
> > > the receiving program is suspended, the buffer will fill sooner or
later.
> >
> > Well, even then we could just drop the "write_atomic" lock.
> >
> > The thing is, I don't know what the tty atomicity guarantees are. I know
> > what they are for pipes (quite reasonable), but tty's?

> We don't have a PIPE_BUF-style atomicity guarantee, even though this would
> be quite useful. This lock is only used to prevent simultaneous writes
> from being interleaved. I've always assumed that when writes shouldn't be
> interleaved, but I can't quote a source for that.