Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

any chance of 2.6.0-test*?

273 views
Skip to first unread message

Alan Cox

unread,
Jan 10, 2003, 11:35:49 AM1/10/03
to
On Fri, 2003-01-10 at 16:10, William Lee Irwin III wrote:
> Any specific concerns/issues/wishlist items you want taken care of
> before doing it or is it a "generalized comfort level" kind of thing?
> Let me know, I'd be much obliged for specific directions to move in.

IDE is all broken still and will take at least another three months to
fix - before we get to 'improve'. The entire tty layer locking is terminally
broken and nobody has even started fixing it. Just try a mass of parallel
tty/pty activity . It was problematic before, pre-empt has taken it to dead,
defunct and buried.

Most of the drivers still don't build either.

I think its important that we get to the stage that we can actually say

- It compiles (as close to all the mainstream bits of it as possible)
- The stuff that is destined for the bitbucket is marked in Config and people
agree it should go
- It works (certainly the common stuff)
- Its statistically unlikely to eat your computer
- It passes Cerberus uniprocessor and smp with/without pre-empt

Otherwise everyone wil rapidly decide that ".0-pre" means ".0 as in Windows"
at which point you've just destroyed your testing base.

Given all the new stuff should be in, I'd like to see a Linus the meanie
round of updating for a while which is simply about getting all the 2.4 fixes
and the 2.5 driver compile bugs nailed, and if it doesn't fix a compile bug
or a logic bug it doesn't go in.

No more "ISAPnP TNG" and module rewrites please

Alan

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

Dave Jones

unread,
Jan 10, 2003, 11:35:45 AM1/10/03
to
On Fri, Jan 10, 2003 at 08:10:12AM -0800, William Lee Irwin III wrote:
> Say, I've been having _smashing_ success with 2.5.x on the desktop and
> on big fat highmem umpteen-way SMP (NUMA even!) boxen, and I was
> wondering if you were considering 2.6.0-test* anytime soon.

There's still a boatload of drivers that don't compile,
a metric shitload of bits that never came over from 2.4 after
I stopped doing it circa 2.4.18, a lot of little 'trivial'
patches that got left by the wayside, and a load of 'strange' bits
that still need nailing down (personally, I have two boxes
that won't boot a 2.5 kernel currently (One was pnpbios related,
other needs more investigation), and another that falls on its
face after 10 minutes idle uptime. My p4-ht desktop box is the only one
that runs 2.5 without any problems.

I think we're a way off from a '2.6-test' phase personally,
but instigating a harder 'code freeze' would probably be a
good thing to do[1]

Dave

[1] Exemption granted for the bits not yet brought forward
of course.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

Jeff Garzik

unread,
Jan 10, 2003, 11:48:32 AM1/10/03
to
On Fri, Jan 10, 2003 at 05:19:08PM +0000, Alan Cox wrote:
> No more "ISAPnP TNG" and module rewrites please

Highlighted and seconded...

Shawn Starr

unread,
Jan 10, 2003, 11:48:28 AM1/10/03
to
There will be a new kernel tree that will fit this purpose soon called -xlk
(eXtendable or Extended Linux Kernel). The hope to make it an 'official' like
-ac, -mm tree for stuffing experimental stuff into a post 2.6 (or just before
2.6 goes live) kernel. I will need help in getting this to become a reality
in the coming months to 2.6.

The purpose of the tree is to get experimental code ready for the 2.7 (2.9?)
tree. We want code that will add new drivers / devices and general
improvements to the kernel. The goal is once these are stabilized they can be
submitted to Linus and friends for blessings and inclusion into 2.7 dev
*early* so we won't have a mad rush for features before the next feature
freeze.

>Subject: any chance of 2.6.0-test*?
From: William Lee Irwin III <wli () holomorphy ! com>
>Date: 2003-01-10 16:10:12

>Say, I've been having _smashing_ success with 2.5.x on the desktop and
>on big fat highmem umpteen-way SMP (NUMA even!) boxen, and I was
>wondering if you were considering 2.6.0-test* anytime soon.

>I'd love to get this stuff out for users to hammer on ASAP, and things
>are looking really good AFAICT.

>Any specific concerns/issues/wishlist items you want taken care of
>before doing it or is it a "generalized comfort level" kind of thing?
>Let me know, I'd be much obliged for specific directions to move in.


>Thanks,
>Bill

--
Shawn Starr
UNIX Systems Administrator, Operations
Datawire Communication Networks Inc.
10 Carlson Court, Suite 300
Toronto, ON, M9W 6L2
T: 416-213-2001 ext 179 F: 416-213-2008
shawn...@datawire.net
"The power to Transact" - http://www.datawire.net

Dave Jones

unread,
Jan 10, 2003, 12:08:03 PM1/10/03
to
On Fri, Jan 10, 2003 at 11:39:57AM -0500, Shawn Starr wrote:
> There will be a new kernel tree that will fit this purpose soon called -xlk
> (eXtendable or Extended Linux Kernel). The hope to make it an 'official' like
> -ac, -mm tree for stuffing experimental stuff into a post 2.6 (or just before
> 2.6 goes live) kernel. I will need help in getting this to become a reality
> in the coming months to 2.6.

The effort is really much better spent trying to get to 2.6 first before
worrying about things like 2.7. I hope 2.6 doesn't turn into the
"heres my 2.4+preempt+rmap patchset" monster that we saw six months ago.



> We want code that will add new drivers / devices and general
> improvements to the kernel.

non-core changes (ie, new drivers) still get added during code freeze,
and during 2.6.x, there's no need for a specific tree just for this.
Adding a new driver doesn't (or at least shouldn't) impact any existing
users if done right.



> The goal is once these are stabilized they can be
> submitted to Linus and friends for blessings and inclusion into 2.7 dev
> *early* so we won't have a mad rush for features before the next feature
> freeze.

Nice try. It'll still happen regardless. Bombing Linus with ten zillion
patches when he opens up 2.7.x with "has been tested in 2.6-xyz" isn't
the way to do it. Everything has to happen incrementally, or you end
up with a mess.

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

Dave Jones

unread,
Jan 10, 2003, 12:19:05 PM1/10/03
to
On Fri, Jan 10, 2003 at 05:19:08PM +0000, Alan Cox wrote:

> Most of the drivers still don't build either.

or still lack 2.4 fixes.

> - The stuff that is destined for the bitbucket is marked in Config and people
> agree it should go

What's happening with the OSS drivers ?
I'm still carrying a few hundred KB of changes from 2.4 for those.
I'm not going to spent a day splitting them up, commenting them and pushing
to Linus if we're going to be dropping various drivers.

> - It passes Cerberus uniprocessor and smp with/without pre-empt

I think this should wait until at least some more of the 2.4 changes
come forward. Most of those are security issues and the likes, but there
are a few driver corner cases too.

> Otherwise everyone wil rapidly decide that ".0-pre" means ".0 as in Windows"
> at which point you've just destroyed your testing base.

agreed.

> Given all the new stuff should be in, I'd like to see a Linus the meanie
> round of updating for a while which is simply about getting all the 2.4 fixes
> and the 2.5 driver compile bugs nailed, and if it doesn't fix a compile bug
> or a logic bug it doesn't go in.

Seconded.

> No more "ISAPnP TNG" and module rewrites please

Absolutly. Lets try and get 2.6 out the door _this_ year.

J.A. Magallon

unread,
Jan 10, 2003, 2:11:36 PM1/10/03
to

On 2003.01.10 Linus Torvalds wrote:

>
> On Fri, 10 Jan 2003, Dave Jones wrote:
> >
> > What's happening with the OSS drivers ?
> > I'm still carrying a few hundred KB of changes from 2.4 for those.
> > I'm not going to spent a day splitting them up, commenting them and pushing
> > to Linus if we're going to be dropping various drivers.
>
> I consider them to be old drivers, the same way "hd.c" was. Not
> necessarily useful for most people, but neither was hd.c. And it was
> around for a _long_ time (heh. I needed to check. The config option is
> still there ;)
>
> So I don't see a huge reason to remove them from the sources, but we might
> well make them harder to select by mistake, for example. Right now the
> config help files aren't exactly helpful, and the OSS choice is before the
> ALSA one, which looks wrong.
>
> They should probably be marked deprecated, and if they don't get a lot of
> maintenance, that's fine.
>
> Linus

As there is a CONFIG_EXPERIMENTAL, how about a CONFIG_DEPRECATED for the
opposite edge ?

--
J.A. Magallon <jamag...@able.es> \ Software is like sex:
werewolf.able.es \ It's better when it's free
Mandrake Linux release 9.1 (Cooker) for i586
Linux 2.4.21-pre2-jam2 (gcc 3.2.1 (Mandrake Linux 9.1 3.2.1-2mdk))

Jeff Garzik

unread,
Jan 10, 2003, 3:02:06 PM1/10/03
to
On Fri, Jan 10, 2003 at 07:47:39PM +0100, J.A. Magallon wrote:
> As there is a CONFIG_EXPERIMENTAL, how about a CONFIG_DEPRECATED for the
> opposite edge ?

CONFIG_OBSOLETE already exists for this.

Matthew Wilcox

unread,
Jan 10, 2003, 5:01:36 PM1/10/03
to

Linus Torvalds wrote:
> Note that other architectures have never been an issue for releasing new
> kernels, and that is _particularly_ true of architectures like parisc and
> mips that haven't even _tried_ to track development kernels. In fact, mips
> "anti-maintenance" has often actively discouraged people from even
> bothering to update mips code when adding new features.

Um.. hey! Just because we weren't trying to merge with you during 2.5
for a long time doesn't mean we weren't tracking development. Looking
at our CVS history, we've merged your tree into ours in:

2.5.26
2.5.32
2.5.41
2.5.43
2.5.44
2.5.45
2.5.46
2.5.47
2.5.50
2.5.51
2.5.53
2.5.54

Our outstanding diff vs your tree is about 200k gzipped and it's almost
all drivers. Off the top of my head, I can't think of any core changes
we still need. I don't think we're using any deprecated interfaces
(eg flush_page_to_ram, unlike m68k, mips, mips64, sparc32 & v850).

Of course, I don't consider having working PA-RISC in your tree to be
a prerequisite for release. I just object to being used as an example :-P

--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Andi Kleen

unread,
Jan 11, 2003, 7:30:32 AM1/11/03
to
Alan Cox <al...@lxorguk.ukuu.org.uk> writes:

> On Fri, 2003-01-10 at 16:10, William Lee Irwin III wrote:
> > Any specific concerns/issues/wishlist items you want taken care of
> > before doing it or is it a "generalized comfort level" kind of thing?
> > Let me know, I'd be much obliged for specific directions to move in.
>

> IDE is all broken still and will take at least another three months to
> fix - before we get to 'improve'. The entire tty layer locking is terminally

Can you quickly summarize what is broken with IDE ?

Are just some low level drivers broken or are there some generic
nasty problems.

If it is just some broken low level drivers I guess they can
be marked dangerous or CONFIG_EXPERIMENTAL.

How does it differ from the code that was just merged into 2.4.21pre3
(has the later all the problems fixed?)

> broken and nobody has even started fixing it. Just try a mass of parallel
> tty/pty activity . It was problematic before, pre-empt has taken it to dead,
> defunct and buried.

Can someone shortly describe what is the main problem with TTY?

From what I can see the high level tty code mostly takes lock_kernel
before doing anything.
On reads access to file->private_data is not serialized, but it at
least shouldn't go away because VFS takes care of struct file
reference counting.

The tty_drivers list does seem to need a spinlock, but I guess
just taking lock_kernel in tty_open would fix that for now.

[i didn't look at low level ldiscs]

Any particular test cases that break ?

If yes I would recommend to post them as scripts and their oopses so
that people can start working on them.


The appended untested patch adds some lock_kernel()s that appear to be missing
to tty_io.c. The rest seems to already run under BKL or not access
any global data
(except tty_paranoia_check, but is probably ok with the reference counting
in the VFS)

>
> Most of the drivers still don't build either.

In UP most did last time I tried.
On SMP a lot of problems are caused by the cli removal

My personal (i386) problem list is relatively short.
I use used 2.5.54 on my desktop without any problems (without preempt)

- BIO still oopses when XFS tries replay a log on RAID-0

-Andi

--- linux-2.5.56-work/drivers/char/tty_io.c-o 2003-01-02 05:13:12.000000000 +0100
+++ linux-2.5.56-work/drivers/char/tty_io.c 2003-01-11 13:23:15.000000000 +0100
@@ -1329,6 +1329,8 @@
int major, minor;
struct tty_driver *driver;

+ lock_kernel();
+
/* find a device that is not in use. */
retval = -1;
for ( major = 0 ; major < UNIX98_NR_MAJORS ; major++ ) {
@@ -1340,6 +1342,8 @@
if (!init_dev(device, &tty)) goto ptmx_found; /* ok! */
}
}
+
+ unlock_kernel();
return -EIO; /* no free ptys */
ptmx_found:
set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
@@ -1357,6 +1361,8 @@
#endif /* CONFIG_UNIX_98_PTYS */
}

+ lock_kernel();
+
retval = init_dev(device, &tty);
if (retval)
return retval;
@@ -1389,6 +1395,8 @@
#endif

release_dev(filp);
+
+ unlock_kernel();
if (retval != -ERESTARTSYS)
return retval;
if (signal_pending(current))
@@ -1397,6 +1405,7 @@
/*
* Need to reset f_op in case a hangup happened.
*/
+ lock_kernel();
filp->f_op = &tty_fops;
goto retry_open;
}
@@ -1424,6 +1433,7 @@
nr_warns++;
}
}
+ unlock_kernel();
return 0;
}

@@ -1444,8 +1454,13 @@
if (tty_paranoia_check(tty, filp->f_dentry->d_inode->i_rdev, "tty_poll"))
return 0;

- if (tty->ldisc.poll)
- return (tty->ldisc.poll)(tty, filp, wait);
+ if (tty->ldisc.poll) {
+ int ret;
+ lock_kernel();
+ ret = (tty->ldisc.poll)(tty, filp, wait);
+ unlock_kernel();
+ return ret;
+ }
return 0;

Andi Kleen

unread,
Jan 11, 2003, 8:18:44 AM1/11/03
to
On Sat, Jan 11, 2003 at 02:01:51PM +0100, Russell King wrote:
> > --- linux-2.5.56-work/drivers/char/tty_io.c-o 2003-01-02 05:13:12.000000000 +0100
> > +++ linux-2.5.56-work/drivers/char/tty_io.c 2003-01-11 13:23:15.000000000 +0100
> > @@ -1329,6 +1329,8 @@
> > int major, minor;
> > struct tty_driver *driver;
> >
> > + lock_kernel();
> > +
>
> Deadlock. chrdev_open() calls lock_kernel() and then the fops->open
> method, which is tty_open().

No problem, lock_kernel is recursive and dropped on schedule.

It is very very hard to get a BKL deadlock.

> This one needs deeper review.

I agree, but one has to start somewhere. Please submit any fixes,
perhaps we can take then close these issues for good.

Was looking at n_tty.c now, looks like it has some more race
problems.


-Andi

Russell King

unread,
Jan 11, 2003, 8:05:22 AM1/11/03
to
> --- linux-2.5.56-work/drivers/char/tty_io.c-o 2003-01-02 05:13:12.000000000 +0100
> +++ linux-2.5.56-work/drivers/char/tty_io.c 2003-01-11 13:23:15.000000000 +0100
> @@ -1329,6 +1329,8 @@
> int major, minor;
> struct tty_driver *driver;
>
> + lock_kernel();
> +

Deadlock. chrdev_open() calls lock_kernel() and then the fops->open
method, which is tty_open().

> /* find a device that is not in use. */


> retval = -1;
> for ( major = 0 ; major < UNIX98_NR_MAJORS ; major++ ) {
> @@ -1340,6 +1342,8 @@
> if (!init_dev(device, &tty)) goto ptmx_found; /* ok! */
> }
> }
> +
> + unlock_kernel();
> return -EIO; /* no free ptys */
> ptmx_found:
> set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
> @@ -1357,6 +1361,8 @@
> #endif /* CONFIG_UNIX_98_PTYS */
> }
>
> + lock_kernel();
> +

Deadlock. See chrdev_open() note above.

> retval = init_dev(device, &tty);
> if (retval)
> return retval;
> @@ -1389,6 +1395,8 @@
> #endif
>
> release_dev(filp);
> +
> + unlock_kernel();
> if (retval != -ERESTARTSYS)
> return retval;
> if (signal_pending(current))
> @@ -1397,6 +1405,7 @@
> /*
> * Need to reset f_op in case a hangup happened.
> */
> + lock_kernel();

Deadlock. See chrdev_open() note above.

> filp->f_op = &tty_fops;
> goto retry_open;
> }
> @@ -1424,6 +1433,7 @@
> nr_warns++;
> }
> }
> + unlock_kernel();
> return 0;
> }
>
> @@ -1444,8 +1454,13 @@
> if (tty_paranoia_check(tty, filp->f_dentry->d_inode->i_rdev, "tty_poll"))
> return 0;
>
> - if (tty->ldisc.poll)
> - return (tty->ldisc.poll)(tty, filp, wait);
> + if (tty->ldisc.poll) {
> + int ret;
> + lock_kernel();
> + ret = (tty->ldisc.poll)(tty, filp, wait);
> + unlock_kernel();
> + return ret;
> + }

This one needs deeper review.

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

Alan Cox

unread,
Jan 11, 2003, 8:56:43 AM1/11/03
to
On Sat, 2003-01-11 at 12:27, Andi Kleen wrote:
> Can you quickly summarize what is broken with IDE ?
> Are just some low level drivers broken or are there some generic
> nasty problems.
>
> If it is just some broken low level drivers I guess they can
> be marked dangerous or CONFIG_EXPERIMENTAL.

Low level drivers are basically sorted.

The main problems are
- Incorrect locking all over the place
- Incorrect timings on some phases
- Some ioctls can cause crashes due to locking
- ISAPnP IDE doesn't work right now
- Flaws in error recovery paths in certain situations
- Lots of random oopses on boot/remove that were apparently
introduced by the kobject/sysfs people and need chasing
down. (There are some non sysfs ones mostly fixed)
- ide-scsi needs some cleanup to fix switchover ide-cd/scsi
(We can't dump ide-scsi)
- Unregister path has races which cause all the long
standing problems with pcmcia and prevents pci unreg
- PCI IDE driver registration needs busy checks
- PCI layer needs some stuff from 2.4
- PCI layer in 2.4/2.5 needs an IRQ bug fixing
- ACPI doesn't seem to handle compatibility IRQ mode
- We don't handle a few errata (MWDMA on 450NX for example)
- IDE raid hasn't been ported to 2.5 at all yet

Thats off the top of my head right now.

> How does it differ from the code that was just merged into 2.4.21pre3
> (has the later all the problems fixed?)

No, although some don't show up in the same ways in 2.4 - eg the pcmcia
unload race is rare in 2.4 for other reasons. Endianism should all
be cured in 2.4, and I sent that to Linus. The PCI i/o assignment code
in 2.4 is done. I hope to have some of the locking and also the timing
path work Ross Biro has done in for 2.4.22 proper

> > broken and nobody has even started fixing it. Just try a mass of parallel
> > tty/pty activity . It was problematic before, pre-empt has taken it to dead,
> > defunct and buried.
>
> Can someone shortly describe what is the main problem with TTY?
>
> >From what I can see the high level tty code mostly takes lock_kernel
> before doing anything.

Which works really well with all the IRQ paths on it

> On reads access to file->private_data is not serialized, but it at
> least shouldn't go away because VFS takes care of struct file
> reference counting.

The refcounting bugs are/were in the ldisc stuff. I thought the
bluetooth folks (Max and co) fixed that one

If we can lock_kernel the tty layer for now (I'm bothered about
the ldisc end of tht which is IRQ context) then great, tty scaling
is suddenelly a 2.7 problem.

Andi Kleen

unread,
Jan 11, 2003, 9:09:04 AM1/11/03
to
> The main problems are
> - Incorrect locking all over the place

Hmm bad. Is it that hard to fix ?

> - Incorrect timings on some phases

Can't you just take out the timings in that case?
My (not very informed) understanding is:
everything should work with the BIOS timings and generic IDE,
having own timings is just an optimization to squeeze out a bit
more speed.

> - ISAPnP IDE doesn't work right now

Hardly a release show stopper I guess

(ISA support in general being rather broken, but I doubt many people care
still)

> - Flaws in error recovery paths in certain situations
> - Lots of random oopses on boot/remove that were apparently
> introduced by the kobject/sysfs people and need chasing
> down. (There are some non sysfs ones mostly fixed)

I guess the kobject/sysfs stuff could be ripped out if it doesn't
work - it is probably not a "must have" feature.


> - ide-scsi needs some cleanup to fix switchover ide-cd/scsi
> (We can't dump ide-scsi)
> - Unregister path has races which cause all the long
> standing problems with pcmcia and prevents pci unreg

Can't you just disable module unloading for the release ?
(not nice, but has worked for other subsystems like IPv6 too in the past)
Also 2.4 didn't have much problems without modular IDE, 2.6 will
be probably able to tolerate it too.

> - IDE raid hasn't been ported to 2.5 at all yet

Vendor problem ?


>
> > > broken and nobody has even started fixing it. Just try a mass of parallel
> > > tty/pty activity . It was problematic before, pre-empt has taken it to dead,
> > > defunct and buried.
> >
> > Can someone shortly describe what is the main problem with TTY?
> >
> > >From what I can see the high level tty code mostly takes lock_kernel
> > before doing anything.
>
> Which works really well with all the IRQ paths on it

Then 2.0/2.2/2.4 would have been racy too :-) Apparently they worked though.

High level tty code doesn't touch any ttys as far as I can see.

It is done in n_tty.c, which apparently needs a bit of work
(found some races together with Rik there)

> > On reads access to file->private_data is not serialized, but it at
> > least shouldn't go away because VFS takes care of struct file
> > reference counting.
>
> The refcounting bugs are/were in the ldisc stuff. I thought the
> bluetooth folks (Max and co) fixed that one

I was thinking about races with tty_release (= own tty_struct suddenly
going away). VFS would prevent that.


-Andi

Alan Cox

unread,
Jan 11, 2003, 9:48:36 AM1/11/03
to
On Sat, 2003-01-11 at 14:06, Andi Kleen wrote:
> > The main problems are
> > - Incorrect locking all over the place
>
> Hmm bad. Is it that hard to fix ?

Very. Just read the settings/proc code for an example

> > - Incorrect timings on some phases
>
> Can't you just take out the timings in that case?
> My (not very informed) understanding is:
> everything should work with the BIOS timings and generic IDE,
> having own timings is just an optimization to squeeze out a bit
> more speed.

These are timing for phases not drive timings. Drive timings from
the BIOS are also frequently questionable. These have to be fixed
and as boxes get faster it becomes more important they are

> > - Lots of random oopses on boot/remove that were apparently
> > introduced by the kobject/sysfs people and need chasing
> > down. (There are some non sysfs ones mostly fixed)
>
> I guess the kobject/sysfs stuff could be ripped out if it doesn't
> work - it is probably not a "must have" feature.

Without a doubt.

> > - ide-scsi needs some cleanup to fix switchover ide-cd/scsi
> > (We can't dump ide-scsi)
> > - Unregister path has races which cause all the long
> > standing problems with pcmcia and prevents pci unreg
>
> Can't you just disable module unloading for the release ?

Only if I can also nail shut your PCMCIA slot, disallow SATA and remove
some ioctls people use for docking.

> > - IDE raid hasn't been ported to 2.5 at all yet
>
> Vendor problem ?

It needs a rewrite to the new bio layer or rewriting to use the device
mapper, which is a distinct option here.


> > Which works really well with all the IRQ paths on it
>
> Then 2.0/2.2/2.4 would have been racy too :-) Apparently they worked though.

Long ago lock_kernel had meaning against IRQ paths. TTY never caught up.

Linus Torvalds

unread,
Jan 12, 2003, 12:13:05 PM1/12/03
to

On Sun, 12 Jan 2003, Greg KH wrote:
>
> But this is really the first it's been mentioned, I can't see holding up
> 2.6 for this. It's a 2.7 job at the earliest, unless someone wants to
> step up and do it right now...

Hmm.. The tty layer still depends on the kernel lock, and we're clearly
not changing that at this point any more.

I don't see what the fundamental problem is, it sounds like there's just
some part that got the lock yanked, probably as part of the normal VFS
kernel-lock-avoidance patches. Looking at tty_io.c (which pretty much
drives everything else), it does seem to take the lock _most_ of the time,
including read and write time.

The only place that looked like it _really_ didn't get the kernel lock was
apparently tty_open(), which is admittedly a fairly important part of it ;)

Alan, do you have a test-load? Might be worth just renaming "tty_open()"
to "do_tty_open()", and then do a

tty_open(..)
{
lock_kernel();
retval = do_tty_open(..);
unlock_kernel();
return retval;
}

thing.

Quite frankly, very little has changed in the 2.5.x tty layer (serial
stuff, yes, tty no), so its locking should work basically as well as 2.4.x
does. Except for infrastructure changes like the VFS lock removal (where
2.4.x will hold the kernel lock itself over open, 2.5.x won't).

Yeah, preemption has this tendency to show locking problems more easily,
and it could clearly be the case that it's broken in 2.4.x too but
basically impossible to trigger. But the _simple_ explanation is that
tty_open() (and perhaps some other cases like ioctl) just missed the
kernel lock.

Linus

Russell King

unread,
Jan 12, 2003, 12:37:50 PM1/12/03
to
On Sun, Jan 12, 2003 at 09:05:16AM -0800, Linus Torvalds wrote:
> The only place that looked like it _really_ didn't get the kernel lock was
> apparently tty_open(), which is admittedly a fairly important part of it ;)

chrdev_open() kindly takes the BKL for tty_open() so there isn't an issue
here.

There were, however, parts of the code which were using the global-cli(),
and these merely got converted to local_irq_save() without (iirc) too much
auditing. They're all marked with FIXME in my current sources.

One thing that regularly bugs me about our current tty implementation,
however, is the handling of SIGWINCH. This is sent to the process group
rather than the session. Take the following example:

- you're running an xterm, with a shell inside.
- you're running 3 ssh connections, 2 suspended, one running.
- you resize the xterm

The WINCH signal will be sent to just the one foreground ssh connection.

- you suspend the current ssh.

The shell believes that the terminal is the _old_ size.

- you resume one of the old ssh sessions.

Likewise, ssh doesn't believe the terminal has changed size.

My current "workaround" is to re-tweak the xterm size each time I resume
or suspend an old process.

POSIX (at least signal and general terminal interface sections) seems to
be rather quiet on the behaviour of WINCH.

Are there any reasons why we shouldn't send WINCH to the whole session
rather than just the pgrp associated with the tty?

-

Linus Torvalds

unread,
Jan 12, 2003, 2:23:27 PM1/12/03
to

On Sun, 12 Jan 2003, Christoph Hellwig wrote:
>
> 2.5 does hold the BKL on ->open of charater- (and block-) devices.

Ahh, my bad.

> The real problem is that the big irqlock is gone and mingo just replaced
> it with local_irq_save & friends, which is not enough.

Ok, most of them should be fixable with a simple spinlock approach.

If the recursion is too nasty to handle, we could make some tty-specific
recursive spinlock as a stop-gap measure, and mark it as being destined
for the garbage-heap in 2.7.x:

/*
* This isn't even _trying_ to be fast!
*/
struct recursive_spinlock {
spinlock_t lock;
int lock_count;
struct task_struct *lock_owner;
};

static struct recursive_spinlock tty_lock = {
.lock = SPIN_LOCK_UNLOCKED,
.lock_count = 0,
.lock_owner = NULL
};

unsigned long tty_spin_lock(void)
{
unsigned long flags;
struct task_struct *tsk;

local_irq_save(flags);
preempt_disable();
tsk = current;
if (spin_trylock(&tty_lock.lock))
goto got_lock;
if (tsk == tty_lock.lock_owner) {
WARN_ON(!tty_lock.lock_count);
tty_lock.lock_count++;
return flags;
}
spin_lock(&tty_lock.lock);
got_lock:
WARN_ON(tty_lock.lock_owner);
WARN_ON(tty_lock.lock_count);
tty_lock.lock_owner = tsk;
tty_lock.lock_count = 1;
return flags;
}

void tty_spin_unlock(unsigned long flags)
{
WARN_ON(tty_lock.lock_owner != current);
WARN_ON(!tty_lock.lock_count);
if (!--tty_lock.lock_count) {
tty_lock.lock_owner = NULL;
spin_unlock(&tty_lock.lock);
}
preempt_enable();
local_irq_restore(flags);
}

and be done with it.

Anybody willing to test it and see if the above works?

Linus

Rob Wilkens

unread,
Jan 12, 2003, 2:43:16 PM1/12/03
to
Linus,

I'm REALLY opposed to the use of the word "goto" in any code where it's
not needed. OF course, I'm a linux kernel newbie, so I'm in no position
to comment

Let me comment below the relevant code snippet below as to how I would
change it:

On Sun, 2003-01-12 at 14:15, Linus Torvalds wrote:
> if (spin_trylock(&tty_lock.lock))
> goto got_lock;
> if (tsk == tty_lock.lock_owner) {
> WARN_ON(!tty_lock.lock_count);
> tty_lock.lock_count++;
> return flags;
> }
> spin_lock(&tty_lock.lock);
> got_lock:
> WARN_ON(tty_lock.lock_owner);

<etc...>

I would change it to something like the following (without testing the
code through a compiler or anything to see if it's valid):

if (!(spin_trylock(&tty_lock.lock))){
if (tsk ==tty_lock.lock_owner){
WRAN_ON(!tty_lock.lcok_count);
tty_lock.lock_count++;
return flags;
}
}
WARN_ON(tty_lock.lock_owner);
<etc...>

Am I wrong that the above would do the same thing without generating the
sphagetti code that a goto would give you. Gotos are BAD, very very
bad. Please note also that the two if statements above could probably
even be combined further into one statement by using a short circuit &&
in the if.

If I'm misinterpreting the original code, then forgive me.. I just saw
a goto and gasped. There's always a better option than goto.

-Rob

Rob Wilkens

unread,
Jan 12, 2003, 2:43:21 PM1/12/03
to
Minor change to my original message below.. I left a line out of a code
change suggestion.

oops - Yes, I forgot to add one line here (my point remains the same:
spin_lock(&tty_lock.lock);

David Ford

unread,
Jan 12, 2003, 3:06:44 PM1/12/03
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The horrific response to the use of "goto" is deprecated in favor of
proper use. A function overloaded with gotos probably should be
reworked. But there there is no need to outright avoid the use of
'goto'. if() is simply a conditional test with a goto A or goto B logic.

There is a reason for the implementation of goto. When it all boils
down to it, in assembler it's all a matter of JMP with or without a
condition.

David

Rob Wilkens wrote:

>On Sun, 2003-01-12 at 14:41, Christoph Hellwig wrote:


>
>
>>On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
>>
>>
>>>Linus,
>>>
>>>I'm REALLY opposed to the use of the word "goto" in any code where it's
>>>not needed.
>>>
>>>

>>Who cares?
>>
>>
>>
>
>I do.
>
>-Rob
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE+IcjV74cGT/9uvgsRAtPFAKDATzN7dkRyfRk7WXEDGyYe0oGiqACffj+X
iXkDBQntdGnHeJFKjfmp0Lg=
=l9hb
-----END PGP SIGNATURE-----

David Ford

unread,
Jan 12, 2003, 3:27:33 PM1/12/03
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gotos aren't the root of code evil. Poor design style is and poor
preparation is the root of spaghetti code. Use the right tool for the
job which includes 'goto'. Every teacher that I've met who has an
objection to the use of gotos has been quite naive, often recommends
avoiding pointers, frequently suggests you restrict your strings to 255
characters or less, etc, etc. The oddball extras vary from teacher to
teacher, but the overall impression of the teacher makes me want to
avoid them even with that 10 foot pole.

It's not the fault of the student for being taught wrong, but it is the
fault of the student for ignoring possibile variances of others.
Particularly the "worldly wise"; people who actually write code for the
real world v.s. lectures.

David

Rob Wilkens wrote:

>On Sun, 2003-01-12 at 14:38, Linus Torvalds wrote:
>
>
>>I think goto's are fine
>>
>>
>
>You're a relatively succesful guy, so I guess I shouldn't argue with
>your style.
>
>However, I have always been taught, and have always believed that
>"goto"s are inherently evil. They are the creators of spaghetti code
>(you start reading through the code to understand it (months or years
>after its written), and suddenly you jump to somewhere totally
>unrelated, and then jump somewhere else backwards, and it all gets ugly
>quickly). This makes later debugging of code total hell.
>
>Would it be so terrible for you to change the code you had there to
>_not_ use a goto and instead use something similar to what I suggested?
>Never mind the philosophical arguments, I'm just talking good coding
>style for a relatively small piece of code.
>
>If you want, but comments in your code to meaningfully describe what's
>happening instead of goto labels.
>
>In general, if you can structure your code properly, you should never
>need a goto, and if you don't need a goto you shouldn't use it. It's
>just "common sense" as I've always been taught. Unless you're
>intentionally trying to write code that's harder for others to read.


>
>-Rob
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE+Ic4P74cGT/9uvgsRAt1QAKCb7ryMMG5iBwTefYgDB7HLuDkRngCeNSq/
M/euGkdIdXpv6IZ1Rw9ikEo=
=Yy9i

Valdis.K...@vt.edu

unread,
Jan 12, 2003, 3:27:42 PM1/12/03
to
On Sun, 12 Jan 2003 14:59:57 EST, Rob Wilkens said:

> In general, if you can structure your code properly, you should never
> need a goto, and if you don't need a goto you shouldn't use it. It's
> just "common sense" as I've always been taught. Unless you're
> intentionally trying to write code that's harder for others to read.

Now, it's provable you never *NEED* a goto. On the other hand, *judicious*
use of goto can prevent code that is so cluttered with stuff of the form:

if(...) {
...
die_flag = 1;
if (!die _flag) {...

Pretty soon, you have die_1_flag, die_2_flag, die_3_flag and so on,
rather than 3 or 4 "goto bail_now;".

The real problem is that C doesn't have a good multi-level "break" construct.
On the other hand, I don't know of any language that has a good one - some
allow "break 3;" to break 3 levels- but that's still bad because you get
screwed if somebody adds an 'if' clause....

yoda...@fsmlabs.com

unread,
Jan 12, 2003, 3:44:39 PM1/12/03
to
On Sun, Jan 12, 2003 at 03:18:38PM -0500, Valdis.K...@vt.edu wrote:
> On Sun, 12 Jan 2003 14:59:57 EST, Rob Wilkens said:
>
> > In general, if you can structure your code properly, you should never
> > need a goto, and if you don't need a goto you shouldn't use it. It's
> > just "common sense" as I've always been taught. Unless you're
> > intentionally trying to write code that's harder for others to read.
>
> Now, it's provable you never *NEED* a goto. On the other hand, *judicious*
> use of goto can prevent code that is so cluttered with stuff of the form:
>
> if(...) {
> ...
> die_flag = 1;
> if (!die _flag) {...
>
> Pretty soon, you have die_1_flag, die_2_flag, die_3_flag and so on,
> rather than 3 or 4 "goto bail_now;".
>
> The real problem is that C doesn't have a good multi-level "break" construct.

longjump. Used with good effect in the plan9 code.

Probably takes more coordination than is possible in Linux and has marginal
benefit, but it looks nice.


--
---------------------------------------------------------
Victor Yodaiken
Finite State Machine Labs: The RTLinux Company.
www.fsmlabs.com www.rtlinux.com
1+ 505 838 9109

Linus Torvalds

unread,
Jan 12, 2003, 3:44:48 PM1/12/03
to

On Sun, 12 Jan 2003, Rob Wilkens wrote:
>
> However, I have always been taught, and have always believed that
> "goto"s are inherently evil. They are the creators of spaghetti code

No, you've been brainwashed by CS people who thought that Niklaus Wirth
actually knew what he was talking about. He didn't. He doesn't have a
frigging clue.

> (you start reading through the code to understand it (months or years
> after its written), and suddenly you jump to somewhere totally
> unrelated, and then jump somewhere else backwards, and it all gets ugly
> quickly). This makes later debugging of code total hell.

Any if-statement is a goto. As are all structured loops.

Ans sometimes structure is good. When it's good, you should use it.

And sometimes structure is _bad_, and gets into the way, and using a
"goto" is just much clearer.

For example, it is quite common to have conditionals THAT DO NOT NEST.

In which case you have two possibilities

- use goto, and be happy, since it doesn't enforce nesting

This makes the code _more_ readable, since the code just does what
the algorithm says it should do.

- duplicate the code, and rewrite it in a nesting form so that you can
use the structured jumps.

This often makes the code much LESS readable, harder to maintain,
and bigger.

The Pascal language is a prime example of the latter problem. Because it
doesn't have a "break" statement, loops in (traditional) Pascal end up
often looking like total shit, because you have to add totally arbitrary
logic to say "I'm done now".

Linus

David Ford

unread,
Jan 12, 2003, 3:45:04 PM1/12/03
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Naturally. Evaluate the assembler when it's done. Normally an
unconditional jump takes less cycles than a conditional jump. It's hard
to argue with that. Use the right tool for the job. If the code
expression necessitates a goto within a block of code to keep things
neat and orderly, then do it.

Regarding the if () { ... }, there are no braces around the assembler
code. It's all a matter of linear instructions with jumps. An if() {
... } is a prettiness for our human eyes to organize code.

David

Rob Wilkens wrote:

>On Sun, 2003-01-12 at 14:58, David Ford wrote:
>
>
>>The horrific response to the use of "goto" is deprecated in favor of
>>proper use. A function overloaded with gotos probably should be
>>reworked. But there there is no need to outright avoid the use of
>>'goto'. if() is simply a conditional test with a goto A or goto B logic.
>>
>>
>

>But if the if() is already there, there is no need to have an additional
>goto in the statement, My version may even be faster than linus'
>version because it may save an extra assembler line of code (the beq or
>whatever branch statement generated by the if would do the branch during
>the if rather than after the if was evaluated and got to the first
>statement which was the goto). Of course, compiler optimizatoins
>probably make that particular point pointless to discuss.


>
>
>
>>There is a reason for the implementation of goto. When it all boils
>>down to it, in assembler it's all a matter of JMP with or without a
>>condition.
>>
>>
>

>But in this case, the condition was already there.. My point is that if
>you don't need a goto, you shouldn't use it. Whenever there's an
>obvious way not to use it, don't.


>
>-Rob
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQE+IdFM74cGT/9uvgsRAuMkAJ4nKED1gEVvNKJiYqXg6J73aQgTRQCfWrrJ
zYoHzuMjZ+ikKHCQ1O9E2C4=
=PBlz
-----END PGP SIGNATURE-----

Rob Wilkens

unread,
Jan 12, 2003, 4:01:43 PM1/12/03
to
On Sun, 2003-01-12 at 15:22, Linus Torvalds wrote:
> No, you've been brainwashed by CS people who thought that Niklaus Wirth
> actually knew what he was talking about. He didn't. He doesn't have a
> frigging clue.

Ok, so you disagree with his style is what you're saying. You are free
to disagree, that's your right. We're just from different schools.

> Any if-statement is a goto. As are all structured loops.

"structured" being a key word. I like structure in code. Let's face
it, everyone likes code to be well structured, that's why there's the
"CodingStyle" document -- people like the code to look a certain way so
they can read it later. Without structure, you don't have that

> Ans sometimes structure is good. When it's good, you should use it.

Ok, we agree there.

> And sometimes structure is _bad_, and gets into the way, and using a
> "goto" is just much clearer.

I can't foresee many scenarios where this would be the case. Someone
spoke, for example, about being indented too far and needing to jump out
because there isn't a good "break" mechanism... Well, as it says in the
coding style document -- if you're indented more then 3 levels in,
"you're screwed anyway, and should fix your program".

In the code snippet you put up, I don't see structure as having been bad
and a goto as having been clearer.

> For example, it is quite common to have conditionals THAT DO NOT NEST.

Could someone (Linus, I know your busy, so I won't ask you -- in fact,
I'm amazed you'd bother to write to _me_ at all) please point me to
sample code that illustrates this. I'm trying to envision "conditionals
that do not nest that need goto". As near as I can tell, there must be
some condition that the goto nests all the conditions in otherwise the
goto wouldn't be needed. In some cases, Maybe what's between the goto
and whats gone to should be broken up into a new function (modularized)
and called independently of function that the goto was in.

As someone else pointed out, it's provable that goto isn't needed, and
given that C is a minimalist language, I'm not sure why it was included.

> The Pascal language is a prime example of the latter problem. Because it
> doesn't have a "break" statement, loops in (traditional) Pascal end up
> often looking like total shit, because you have to add totally arbitrary
> logic to say "I'm done now".

But at least the code is "readable" when you do that. Sure it's a
little more work on the part of the programmer. But anyone reading the
code can say "oh, so the loop exits when condition x is met", rather
than digging through the code looking for any place their might be a
goto.

Of course, I'm not about to argue that the kernel should be rewritten
in PASCAL, nor am I interested in reinventing the wheel. We've got a
good system here, let's just keep it good and make it better. (Notice I
say "we" as if I've contributed something to it, whereas really I mean
we including myself as an end-user.)

-Rob

Valdis.K...@vt.edu

unread,
Jan 12, 2003, 4:01:47 PM1/12/03
to
On Sun, 12 Jan 2003 20:32:01 GMT, Sean Neakums <snea...@zork.net> said:
> commence Valdis.K...@vt.edu quotation:

>
> > The real problem is that C doesn't have a good multi-level "break"
> > construct. On the other hand, I don't know of any language that has

> > a good one - some allow "break 3;" to break 3 levels- but that's
> > still bad because you get screwed if somebody adds an 'if'
> > clause....
>
> Perl's facility for labelling blocks and jumping to the beginning or
> end with 'next' and 'last' may be close to what you want, but I don't
> know if it's ever been implemented in a language one could sensibly
> use to write a kernel.

Hmm.. I'm not a Perl wizard.. <reads the docs> Yeah, that's what I meant,
and no, that's not a sane kernel language. ;)

Rob Wilkens

unread,
Jan 12, 2003, 4:10:11 PM1/12/03
to
Mr. Love,

I posted a minor change to the floppy driver which no one commented on
earlier (probably in wrong form)-- three times now (once in unified diff
format).. And you might even say that the message you quoted was kernel
code that I was posting since I posted alternate code to linus' code.

As per whether I post more than the developers here, it's only because I
don't have a job, so I've got more free time than they do. I can't help
it. If they're busier than me, I wouldn't expect them to waste time
writing conversational messages about the kernel.

-Rob

On Sun, 2003-01-12 at 15:31, Robert Love wrote:


> On Sun, 2003-01-12 at 14:34, Rob Wilkens wrote:
>
> > I'm REALLY opposed to the use of the word "goto" in any code where

> > it's not needed. OF course, I'm a linux kernel newbie, so I'm in
> > no position to comment
>
> We use goto's extensively in the kernel. They generate fast, tight
> code.
>
> There is nothing wrong with being a newbie, but there is something wrong
> with voicing ignorance. "Learn first, form opinions later".
>
> When you start posting more than most of the developers here, combined,
> and none of your posts are contributive kernel code, there is a problem.
>
> Robert Love

Matti Aarnio

unread,
Jan 12, 2003, 4:18:03 PM1/12/03
to
On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
> Linus,
>
> I'm REALLY opposed to the use of the word "goto" in any code where it's
> not needed. OF course, I'm a linux kernel newbie, so I'm in no position
> to comment

Bob,

At first, read Documentation/CodingStyle of the kernel.
Then have a look into:

fs/open.c file do_sys_truncate() function.

Explain how you can do that cleanly, understandably, and without
code duplication, or ugly kludges without using those goto ?
(And sticking to coding-style.)

Also take into account, that there the most common execution path
is direct, and exceptions are redirected elsewere.


Original "gotos are evil" comes from days of FORTRAN (and BASIC)
coding, where labels were all numeric, and definitely non-descriptive.
Where nested conditionals didn't exist, etc. Source-code nesting didn't
exist either...


Doing things in moderation will usually produce better code,
than strict sticking into some paradigm or other.


Of course in schools there is definite need for severe whaking of
sense into heads of the young would-be coders. Clean reusable
and maintainable code is _hard_ to produce without any sort of
role-models and rules, especially when kids teach coding for
themselves by looking into other kids code of "look how clever
spaghetti I made" stuff.

When you learn the ropes (rules), and code enough (some years),
you will finally learn where you can relax the rules a bit.
Especially where the rules _must_ be altered to allow some
formerly boo-boo styles.


...


> If I'm misinterpreting the original code, then forgive me.. I just saw
> a goto and gasped. There's always a better option than goto.

Yes there are, but we are coding in C, nor C++ or Java...

> -Rob

/Matti Aarnio

Rob Wilkens

unread,
Jan 12, 2003, 4:33:23 PM1/12/03
to
On Sun, 2003-01-12 at 16:15, Matti Aarnio wrote:
> On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
> > Linus,
> >
> > I'm REALLY opposed to the use of the word "goto" in any code where it's
> > not needed. OF course, I'm a linux kernel newbie, so I'm in no position
> > to comment
>
> Bob,
>
> At first, read Documentation/CodingStyle of the kernel.
> Then have a look into:
>
> fs/open.c file do_sys_truncate() function.
>
> Explain how you can do that cleanly, understandably, and without
> code duplication, or ugly kludges without using those goto ?
> (And sticking to coding-style.)

I've only compiled (and haven't tested this code), but it should be much
faster than the original code. Why? Because we're eliminating an extra
"jump" in several places in the code every time open would be called.
Yes, it's more code, so the kernel is a little bigger, but it should be
faster at the same time, and memory should be less of an issue nowadays.

Here's the patch if you want to apply it (i have only compile tested it,
I haven't booted with it).. This patch applied to the 2.5.56 kernel.

--- open.c.orig 2003-01-12 16:17:01.000000000 -0500
+++ open.c 2003-01-12 16:22:32.000000000 -0500
@@ -100,44 +100,58 @@

error = -EINVAL;
if (length < 0) /* sorry, but loff_t says... */
- goto out;
+ return error;

error = user_path_walk(path, &nd);
if (error)
- goto out;
+ return error;
inode = nd.dentry->d_inode;

/* For directories it's -EISDIR, for other non-regulars - -EINVAL */
error = -EISDIR;
- if (S_ISDIR(inode->i_mode))
- goto dput_and_out;
+ if (S_ISDIR(inode->i_mode)){
+ path_release(&nd);
+ return error;
+ }

error = -EINVAL;
- if (!S_ISREG(inode->i_mode))
- goto dput_and_out;
+ if (!S_ISREG(inode->i_mode)){
+ path_release(&nd);
+ return error;
+ }

error = permission(inode,MAY_WRITE);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }

error = -EROFS;
- if (IS_RDONLY(inode))
- goto dput_and_out;
+ if (IS_RDONLY(inode)){
+ path_release(&nd);
+ return error;
+ }

error = -EPERM;
- if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
- goto dput_and_out;
+ if (IS_IMMUTABLE(inode) || IS_APPEND(inode)){
+ path_release(&nd);
+ return error;
+ }

/*
* Make sure that there are no leases.
*/
error = break_lease(inode, FMODE_WRITE);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }

error = get_write_access(inode);
- if (error)
- goto dput_and_out;
+ if (error){
+ path_release(&nd);
+ return error;
+ }

error = locks_verify_truncate(inode, NULL, length);
if (!error) {
@@ -146,9 +160,7 @@
}
put_write_access(inode);

-dput_and_out:
path_release(&nd);
-out:
return error;

Rik van Riel

unread,
Jan 12, 2003, 4:33:24 PM1/12/03
to
On Sun, 12 Jan 2003, Rob Wilkens wrote:

> However, I have always been taught, and have always believed that
> "goto"s are inherently evil. They are the creators of spaghetti code

If the main flow of the code is through a bunch of hard to trace
gotos and you choose to blame the tool instead of the programmer,
I guess you could blame goto.

However, the goto can also be a great tool to make the code more
readable. The goto statement is, IMHO, one of the more elegant
ways to code exceptions into a C function; that is, dealing with
error situations that don't happen very often, in such a way that
the error handling code doesn't clutter up the main code path.

As an example, you could look at fs/super.c::do_kern_mount()

mnt = alloc_vfsmnt(name);
if (!mnt)
goto out;
sb = type->get_sb(type, flags, name, data);
if (IS_ERR(sb))
goto out_mnt;

Do you see how the absence of the error handling cleanup code
makes the normal code path easier to read ?

regards,

Rik
--
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/ http://guru.conectiva.com/
Current spamtrap: <a href=mailto:"oct...@surriel.com">oct...@surriel.com</a>

Rik van Riel

unread,
Jan 12, 2003, 4:44:00 PM1/12/03
to
On Sun, 12 Jan 2003, Rob Wilkens wrote:
> On Sun, 2003-01-12 at 16:15, Matti Aarnio wrote:

> > At first, read Documentation/CodingStyle of the kernel.
> > Then have a look into:
> >
> > fs/open.c file do_sys_truncate() function.

> - if (S_ISDIR(inode->i_mode))


> - goto dput_and_out;
> + if (S_ISDIR(inode->i_mode)){
> + path_release(&nd);
> + return error;
> + }

[snip same change in a few more changes]

OK, now imagine the dcache locking changing a little bit.
You need to update this piece of (duplicated) code in half
a dozen places in just this function and no doubt in dozens
of other places all over fs/*.c.

From a maintenance point of view, a goto to a single block
of error handling code is easier to maintain.

Emiliano Gabrielli

unread,
Jan 12, 2003, 5:01:04 PM1/12/03
to

<quote who="Rob Wilkens">

> On Sun, 2003-01-12 at 16:40, Rik van Riel wrote:
>> OK, now imagine the dcache locking changing a little bit.
>> You need to update this piece of (duplicated) code in half
>> a dozen places in just this function and no doubt in dozens
>> of other places all over fs/*.c.
>>
>> >From a maintenance point of view, a goto to a single block
>> of error handling code is easier to maintain.
>>
>
> There's no reason, though, that the error handling/cleanup code can't be in an
> entirely separate function, and if speed is needed, there's no reason it can't be an
> "inline" function. Or am I oversimplifying things again?
>
> -Rob
>

you do, if you inline the code and every drive writer use this tecnique the kernel will
be much bigger don't you think ?!?

Makeing a simple function instead is quite slower I think... don't forget that goto are
used only in error recovery routines ...

You can simply build a "stack" of labels .. IMHO this is a great way to be sure of the
right order we are performing cleanup/recovery ...
--
Emiliano Gabrielli

dip. di Fisica
2° Università di Roma "Tor Vergata"

Adam Kropelin

unread,
Jan 12, 2003, 5:04:15 PM1/12/03
to
On Sun, Jan 12, 2003 at 04:27:30PM -0500, Rob Wilkens wrote:
> On Sun, 2003-01-12 at 16:15, Matti Aarnio wrote:
> > On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
> > > Linus,
> > >
> > > I'm REALLY opposed to the use of the word "goto" in any code where it's
> > > not needed. OF course, I'm a linux kernel newbie, so I'm in no position
> > > to comment
> >
> > fs/open.c file do_sys_truncate() function.
> >
> > Explain how you can do that cleanly, understandably, and without
> > code duplication, or ugly kludges without using those goto ?
> > (And sticking to coding-style.)
>
> I've only compiled (and haven't tested this code), but it should be much
> faster than the original code. Why? Because we're eliminating an extra
> "jump" in several places in the code every time open would be called.

Congratulations. You've possibly increased the speed of an error path by
an infintessimal amount at the expense of increasing the size of kernel
image and making the code harder to read and maintain. (I say "possibly"
since with caching effects you may have actually slowed the code down.)

Harder to read: The primary code path is polluted with repetative code
that has no bearing on its primary mission.

Harder to maintain: Add an extra resource allocation near the top and
now you have to hunt out every failure case and manually update them all
(with yet more duplicate code) instead of just amending the cleanup code
at the end of the function.

> error = -EINVAL;
> if (length < 0) /* sorry, but loff_t says... */
> - goto out;
> + return error;

Possibly an improvement, though there are maintenance and consistency
arguments against it.

> /* For directories it's -EISDIR, for other non-regulars - -EINVAL */
> error = -EISDIR;

> - if (S_ISDIR(inode->i_mode))
> - goto dput_and_out;
> + if (S_ISDIR(inode->i_mode)){
> + path_release(&nd);
> + return error;
> + }

Nope. This code is precisely what the on-error-goto method is used
to avoid.

The use of goto in the kernel surprised me the first time I saw it, too.
However, rather than hurry to point out how much more I knew about C
style than the kernel hackers, I stayed quiet and made a learning
experience of it. I suggest you do the same.

--Adam

Oliver Neukum

unread,
Jan 12, 2003, 5:09:16 PM1/12/03
to

> I've only compiled (and haven't tested this code), but it should be much
> faster than the original code. Why? Because we're eliminating an extra
> "jump" in several places in the code every time open would be called.
> Yes, it's more code, so the kernel is a little bigger, but it should be
> faster at the same time, and memory should be less of an issue nowadays.
>
> Here's the patch if you want to apply it (i have only compile tested it,
> I haven't booted with it).. This patch applied to the 2.5.56 kernel.
>
> --- open.c.orig 2003-01-12 16:17:01.000000000 -0500
> +++ open.c 2003-01-12 16:22:32.000000000 -0500
> @@ -100,44 +100,58 @@
>
> error = -EINVAL;
> if (length < 0) /* sorry, but loff_t says... */
> - goto out;
> + return error;

Please don't do such things. The next time locking is changed and a lock
is needed here, some poor guy has to go through that and change all
back to goto.
This may not be applicable here, but as a general rule, don't do it.
I speak from experience.

As for efficiency, that is the compiler's job.

Regards
Oliver

Oliver Neukum

unread,
Jan 12, 2003, 5:13:38 PM1/12/03
to
Am Sonntag, 12. Januar 2003 22:44 schrieb Rob Wilkens:
> On Sun, 2003-01-12 at 16:40, Rik van Riel wrote:
> > OK, now imagine the dcache locking changing a little bit.
> > You need to update this piece of (duplicated) code in half
> > a dozen places in just this function and no doubt in dozens
> > of other places all over fs/*.c.
> >
> > >From a maintenance point of view, a goto to a single block
> >
> > of error handling code is easier to maintain.
>
> There's no reason, though, that the error handling/cleanup code can't be
> in an entirely separate function, and if speed is needed, there's no
> reason it can't be an "inline" function. Or am I oversimplifying things
> again?

Yes. Typical error cleanup looks like:
err_out:
up(&sem);
return err;

Releasing a lock in another function is a crime punished by slow death.
(Some might even resort to voodoo to make sure your shadow suffers
in the beyond.)
It makes code absolutely unreadable.

Rob Wilkens

unread,
Jan 12, 2003, 5:25:58 PM1/12/03
to
On Sun, 2003-01-12 at 16:59, Adam Kropelin wrote:
> Congratulations. You've possibly increased the speed of an error path by
> an infintessimal amount at the expense of increasing the size of kernel
> image and making the code harder to read and maintain. (I say "possibly"
> since with caching effects you may have actually slowed the code down.)

Hey, if the compiler does it's job right, I increased the speed of
something in the kernel. And, as a kernel newbie, I'm proud of that. I
also did it in under 12 minutes (from time stamp of message received to
time stamp of message sent after code compiled and diff'd).

> Harder to read: The primary code path is polluted with repetative code
> that has no bearing on its primary mission.

I thought it was easier to read. For me, I can read "ok, this condition
happens, I fail"... Or "if this other condition happens, I release my
path, then I fail"...

Whereas the "goto out" was very unclear. It made me page down to figure
out what was going on.

That's the whole point.. To just browse the code.. I shouldn't have to
page down to understand what the code right in front of me is doing.
"goto out" is unclear. "retun error" is clear. "path_release" seems
like a relatively plain english function name and I can guess what it
does without knowing exactly what it does. I can also surmise that if I
go beyond a certain point in the function that I need to path_release()
the same way a non-kernel programmer might need to free memory allocated
inside of a function before returning to the calling function.

It really is that simple.

> Harder to maintain: Add an extra resource allocation near the top and
> now you have to hunt out every failure case and manually update them all
> (with yet more duplicate code) instead of just amending the cleanup code
> at the end of the function.

It took me 12 minutes from message received to message sent to update
the entire block of code to handle the new case. How long do you think
it would take to make a minor modification that would only have to do a
portion of what I did? Is it such a burden on the developer to make the
code more readable?

-Rob

Emiliano Gabrielli

unread,
Jan 12, 2003, 5:40:22 PM1/12/03
to

<quote who="Rob Wilkens">

> On Sun, 2003-01-12 at 16:59, Adam Kropelin wrote:
>> Congratulations. You've possibly increased the speed of an error path by an
>> infintessimal amount at the expense of increasing the size of kernel image and
>> making the code harder to read and maintain. (I say "possibly" since with caching
>> effects you may have actually slowed the code down.)
>
> Hey, if the compiler does it's job right, I increased the speed of something in the
> kernel. And, as a kernel newbie, I'm proud of that. I also did it in under 12
> minutes (from time stamp of message received to time stamp of message sent after code
> compiled and diff'd).
>
>> Harder to read: The primary code path is polluted with repetative code that has no
>> bearing on its primary mission.
>
> I thought it was easier to read. For me, I can read "ok, this condition happens, I
> fail"... Or "if this other condition happens, I release my path, then I fail"...
>
> Whereas the "goto out" was very unclear. It made me page down to figure out what was
> going on.
>
> That's the whole point.. To just browse the code.. I shouldn't have to page down to
> understand what the code right in front of me is doing. "goto out" is unclear.
> "retun error" is clear. "path_release" seems like a relatively plain english function
> name and I can guess what it does without knowing exactly what it does.

goto out_path_release is finer to you ?!? :)

> I can also
> surmise that if I go beyond a certain point in the function that I need to
> path_release() the same way a non-kernel programmer might need to free memory
> allocated inside of a function before returning to the calling function.
>
> It really is that simple.
>
>> Harder to maintain: Add an extra resource allocation near the top and now you have
>> to hunt out every failure case and manually update them all (with yet more duplicate
>> code) instead of just amending the cleanup code at the end of the function.
>
> It took me 12 minutes from message received to message sent to update the entire block
> of code to handle the new case. How long do you think it would take to make a minor
> modification that would only have to do a portion of what I did? Is it such a burden
> on the developer to make the code more readable?
>

I think you have no idea of the mole of the linux kernel and the number of daily patches
the mantainers receive ...


I'm also a beginner, and me too at the very first time hated the goto (every one have
told me they are evil !!) but after aving taken a look at the kernel and reading
LinuxDevice Driver I think that the style the linux kernel is coded is cryptic for
beginners, but it is perfect !

It is a wonderful experience to browse the linux kernel sources .. and every time I
didn't understand why a thing was done in such a way .. well I discover later (with
experience) that it was done in the best way it could ever be done !!!


--
Emiliano Gabrielli

dip. di Fisica
2° Università di Roma "Tor Vergata"

Oliver Neukum

unread,
Jan 12, 2003, 5:52:36 PM1/12/03
to
Am Sonntag, 12. Januar 2003 23:22 schrieb Rob Wilkens:

> On Sun, 2003-01-12 at 17:06, Oliver Neukum wrote:
> > Please don't do such things. The next time locking is changed and a lock
> > is needed here, some poor guy has to go through that and change all
> > back to goto.
> > This may not be applicable here, but as a general rule, don't do it.
> > I speak from experience.
> >
> > As for efficiency, that is the compiler's job.
>
> I say "please don't use goto" and instead have a "cleanup_lock" function
> and add that before all the return statements.. It should not be a
> burden. Yes, it's asking the developer to work a little harder, but the
> end result is better code.

It's code that causes added hardship for anybody checking the locking.
It becomes impossible to see whether locks are balanced and turns into
a nightmare as soon as you need error exits from several depths of locking
or with and without memory to be freed.

Please listen to experience.

err_out_buffer:
kfree(buffer);
err_out_nobuffer:
up(&sem);
err_out_nolock:
return err;

is pretty much the only readable construction.

Regards
Oliver

Sean Neakums

unread,
Jan 12, 2003, 6:09:56 PM1/12/03
to
commence Rob Wilkens quotation:

> On Sun, 2003-01-12 at 17:18, Aaron Lehmann wrote:
>> These are usually error conditions. If you inline them, you will have
>> to jump *over* them as part of the normal code path. You don't save
>
> You're wrong. You wouldn't have to jump over them any more than you
> have to jump over the "goto" statement. They would be where the goto
> statement is. Instead of the goto you would have the function.

He said the *normal* path. Jumping to error code is rarely a normal
path. By replacing the gotos with inlined functions, you turn the
fast path into a bunch of jumps over duplicated error code, and the
error path into a straight line.

>> any instructions, and you end up with a kernel which has much more
>> duplicated code and thus thrashes the cache more. It also makes the
>
> If that argument was taken to it's logical conclusion (and I did, in my
> mind just now), no one should add any code the grows the kernel at all.

The interesting thing about taking arguments to their logical
conclusion is that it rarely makes any sense to do so.

--
/ |
[|] Sean Neakums | Questions are a burden to others;
[|] <snea...@zork.net> | answers a prison for oneself.
\ |

Oliver Neukum

unread,
Jan 12, 2003, 6:09:58 PM1/12/03
to
Am Sonntag, 12. Januar 2003 23:34 schrieb Rob Wilkens:
> On Sun, 2003-01-12 at 17:18, Aaron Lehmann wrote:
> > These are usually error conditions. If you inline them, you will have
> > to jump *over* them as part of the normal code path. You don't save
>
> You're wrong. You wouldn't have to jump over them any more than you
> have to jump over the "goto" statement. They would be where the goto
> statement is. Instead of the goto you would have the function.

That exactly is the problem. If they are where the goto would be
they needlessly fill the CPU's pipeline, take up space in L1 and use
up bandwidth on the busses.
A goto is much shorter than a cleanup.

And you would have to jump. Any control structure will result in one
or more conditional jumps in the assembler code (or conditional instructions)

Turning "if (a) goto b;" into a single branch instruction is trivial.
The best the compiler can do with
if (a) {
cleanup();
return err;
}
is putting the conditional code on the end of the function.

So in the best case the compiler can generate almost equivalent code
at a cost of maintainability.

> > any instructions, and you end up with a kernel which has much more
> > duplicated code and thus thrashes the cache more. It also makes the
>
> If that argument was taken to it's logical conclusion (and I did, in my
> mind just now), no one should add any code the grows the kernel at all.

Correct. If you mean that literally, you've grasped an important concept.
You grow the kernel only with good cause.

And you look where you add the code. Additional device drivers don't hurt.
A computed jump is still only one computed jump. Additional code in common
code paths of the core hurts a lot.
For the inner loops of core code there are two considerations, size and
reducing jumps.

Regards
Oliver

Robert Love

unread,
Jan 12, 2003, 6:10:04 PM1/12/03
to
On Sun, 2003-01-12 at 17:22, Rob Wilkens wrote:

> I say "please don't use goto" and instead have a "cleanup_lock" function
> and add that before all the return statements.. It should not be a
> burden. Yes, it's asking the developer to work a little harder, but the
> end result is better code.

No, it is gross and it bloats the kernel. It inlines a bunch of junk
for N error paths, as opposed to having the exit code once at the end.
Cache footprint is key and you just killed it.

Nor is it easier to read.

As a final argument, it does not let us cleanly do the usual stack-esque
wind and unwind, i.e.

do A
if (error)
goto out_a;
do B
if (error)
goto out_b;
do C
if (error)
goto out_c;
goto out;
out_c:
undo C
out_b:
undo B:
out_a:
undo A
out:
return ret;

Now stop this.

Robert Love

Oliver Neukum

unread,
Jan 12, 2003, 6:27:16 PM1/12/03
to
Am Montag, 13. Januar 2003 00:05 schrieb Emiliano Gabrielli:
> <quote who="Rob Wilkens">

>
> > On Sun, 2003-01-12 at 17:43, Oliver Neukum wrote:
> >> It's code that causes added hardship for anybody checking the locking.
> >
> > I can certainly see where it would seem "easier" to match "one lock" to
> > "one unlock" if your troubleshooting a locking issue.
> >
> > "easier" is the motivation behind using goto.. Laziness.. that's all it
> > is.
>
> In italy we say "Non c'è peggior sordo di chi non vuole sentire"

There's no worse fate than that of those who don't want to listen?
(My Italian is bad.)

> Till to this point the thread was useful to beginners becouse of a lot of
> smarty coders explained a point that everyone has heard but that rarely
> knows at deep...
>
> but now Let's ignore him ! He is not a beginner he is a lamer!

Very well. What could be said has been said. But please don't insult him.
Time spent explaining is usually well spent. Perhaps somebody learned.

OK, maybe I have a soft spot.

Regards
Oliver

Oliver Neukum

unread,
Jan 12, 2003, 6:47:12 PM1/12/03
to
Am Montag, 13. Januar 2003 00:11 schrieb Rob Wilkens:
> On Sun, 2003-01-12 at 17:52, Aaron Lehmann wrote:

> > On Sun, Jan 12, 2003 at 05:34:58PM -0500, Rob Wilkens wrote:
> > > You're wrong. You wouldn't have to jump over them any more than you
> > > have to jump over the "goto" statement.
> >
> > The goto is a conditional jump. You propose replacing it with a
> > conditional jump past the error handling code predicated on the
> > opposite condition. Where's the improvement?
>
> The goto is absolutely not a conditional jump. The if that precedes it
> is conditional. The goto is not. The if is already there.

Oh, well.
Apologies first, my assembler is rusty.

if (a == NULL)
goto err_out;

bad compiler ->
tst $D0 ; evaluate a == NULL
bne L1 ; this is the if
bra err_out ; this is the goto
L1:

good compiler ->
tst $D0
beq err_out ; obvious optimisation

Emiliano Gabrielli

unread,
Jan 12, 2003, 6:47:12 PM1/12/03
to

<quote who="Rob Wilkens">

> On Sun, 2003-01-12 at 17:52, Aaron Lehmann wrote:
>> On Sun, Jan 12, 2003 at 05:34:58PM -0500, Rob Wilkens wrote:
>> > You're wrong. You wouldn't have to jump over them any more than you have to jump
>> over the "goto" statement.
>>
>> The goto is a conditional jump. You propose replacing it with a
>> conditional jump past the error handling code predicated on the
>> opposite condition. Where's the improvement?
>
> The goto is absolutely not a conditional jump. The if that precedes it is
> conditional. The goto is not. The if is already there.
>
> -Rob


Oh my God ... really you are the only one on this list not to understand that hi has
mispelled ?!?

Now I understand the reason of your trollness !!

--
Emiliano Gabrielli

dip. di Fisica
2° Università di Roma "Tor Vergata"

Emiliano Gabrielli

unread,
Jan 12, 2003, 6:59:02 PM1/12/03
to

<quote who="Oliver Neukum">

> Am Montag, 13. Januar 2003 00:05 schrieb Emiliano Gabrielli:
>> <quote who="Rob Wilkens">
>>
>> > On Sun, 2003-01-12 at 17:43, Oliver Neukum wrote:
>> >> It's code that causes added hardship for anybody checking the locking.
>> >
>> > I can certainly see where it would seem "easier" to match "one lock" to "one
>> unlock" if your troubleshooting a locking issue.
>> >
>> > "easier" is the motivation behind using goto.. Laziness.. that's all it is.
>>
>> In italy we say "Non c'è peggior sordo di chi non vuole sentire"
>
> There's no worse fate than that of those who don't want to listen? (My Italian is
> bad.)

It is better than my english :-))

>
>> Till to this point the thread was useful to beginners becouse of a lot of smarty
>> coders explained a point that everyone has heard but that rarely knows at deep...
>>
>> but now Let's ignore him ! He is not a beginner he is a lamer!
>
> Very well. What could be said has been said. But please don't insult him. Time spent
> explaining is usually well spent. Perhaps somebody learned.
>

I agree .. my apologises .. I use currently the word "lamer" in IRC, but I don't really
know the true translation.. if it is an insult .. plz adcept my apologises

> OK, maybe I have a soft spot.
>

:-)

> Regards
> Oliver

Emiliano Gabrielli

unread,
Jan 12, 2003, 6:59:04 PM1/12/03
to

Scott Robert Ladd

unread,
Jan 12, 2003, 7:07:13 PM1/12/03
to
Rob Wilken wrote:
> Would it be so terrible for you to change the code you had there to
> _not_ use a goto and instead use something similar to what I suggested?
> Never mind the philosophical arguments, I'm just talking good coding
> style for a relatively small piece of code.
>
> If you want, but comments in your code to meaningfully describe what's
> happening instead of goto labels.

>
> In general, if you can structure your code properly, you should never
> need a goto, and if you don't need a goto you shouldn't use it. It's
> just "common sense" as I've always been taught. Unless you're
> intentionally trying to write code that's harder for others to read.

I've spent some time looking through the kernel source code, getting a feel
for the style and process before attempting to contribute something of my
own. In most ways, the quality of Linux code equals or exceeds that of
commercial products I've worked on. It may not be perfect, but I'd prefer
that the maintainers focus on features and bug fixes, not religious issues.

Your attitude against "goto" is perhaps based upon an excellent but dated
article, "Goto Considered Harmful", written by Edsger W. Dijkstra, and
published by the ACM in 1968. (A recent reprint can be found at
http://www.acm.org/classics/oct95/.) As you can tell from the date, this
article predates modern programming languages and idioms; it comes from a
time when Fortran ruled, and before Fortran 77 provided significant tools
for avoiding spaghetti code.

A "goto" is not, in and of itself, dangerous -- it is a language feature,
one that directly translates to the jump instructions implemented in machine
code. Like pointers, operator overloading, and a host of other "perceived"
evils in programming, "goto" is widely hated by those who've been bitten by
poor programming. Bad code is the product of bad programmers; in my
experience, a poor programmer will write a poor program, regardless of the
availability of "goto."

If you think people can't write spaghetti code in a "goto-less" language, I
can send you some *lovely* examples to disabuse you of that notion. ;)

Used over short distances with well-documented labels, a "goto" can be more
effective, faster, and cleaner than a series of complex flags or other
constructs. The "goto" may also be safer and more intuitive than the
alternative. A "break" is a goto; a "continue" is a "goto" -- these are
statements that move the point of execution explicitly.

That said, I have used exactly two "goto" statements in all the lines of C,
C++, Fortran 95, and (yes) COBOL I've written since leaving BASIC and
Fortran IV behind. In one case, a single "goto" doubled the speed of a
time-critical application; in the other case, "goto" shortens a segment of
code by half and makes the algorithm much clearer. I would not use a goto
willy-nilly for the fun of it -- unless I was entering an obfuscated code
contest ;)

We keep lowering the bar for technical prowess, it seems; if something has
the potential to be used "wrong", high-minded designers remove the offending
syntax rather than find or train competent programmers. This is why Java
removes pointers (among other things) -- it's not that pointers aren't
useful or efficient, it's that they require discipline from programmers.

Just because something is dogma doesn't mean it is absolute truth. If
anything, dogma should be sniffed quite carefully, since it tends to be
rather rank if you get close enough. Removing goto is a religious choice,
not a technical one.

I could draw parallels with idiotic laws in general society, but this
message is already marginal for this list.

..Scott

--
Scott Robert Ladd
Coyote Gulch Productions (http://www.coyotegulch.com)
Professional programming for science and engineering;
Interesting and unusual bits of very free code.

Randy Dunlap

unread,
Jan 12, 2003, 7:57:30 PM1/12/03
to
> On Sun, 2003-01-12 at 14:38, Linus Torvalds wrote:
>> I think goto's are fine
>
> You're a relatively succesful guy, so I guess I shouldn't argue with your
> style.

Good. (although I don't know why I'm replying as this thread
is way overdone....:)

> However, I have always been taught, and have always believed that

> "goto"s are inherently evil. They are the creators of spaghetti code (you


> start reading through the code to understand it (months or years after its
> written), and suddenly you jump to somewhere totally
> unrelated, and then jump somewhere else backwards, and it all gets ugly
> quickly). This makes later debugging of code total hell.
>

> Would it be so terrible for you to change the code you had there to _not_
> use a goto and instead use something similar to what I suggested? Never
> mind the philosophical arguments, I'm just talking good coding style for a
> relatively small piece of code.
>
> If you want, but comments in your code to meaningfully describe what's

or put


> happening instead of goto labels.
>
> In general, if you can structure your code properly, you should never need a
> goto, and if you don't need a goto you shouldn't use it. It's just "common
> sense" as I've always been taught. Unless you're
> intentionally trying to write code that's harder for others to read.

There are goto-less languages, even Algol-like ones.
And OSes can be written in them.
Well, they just have different names for JUMP.

~Randy

Rob Wilkens

unread,
Jan 12, 2003, 7:57:23 PM1/12/03
to
On Sun, 2003-01-12 at 19:03, Scott Robert Ladd wrote:
> I've spent some time looking through the kernel source code, getting a feel
> for the style and process before attempting to contribute something of my
> own.

In part that's why i haven't contributed too much of any actual code
yet. I'm trying to get a feel for the code and the users. I figure the
best way is to join the discussion list and follow the patches going in.


> Your attitude against "goto" is perhaps based upon an excellent but dated
> article, "Goto Considered Harmful", written by Edsger W. Dijkstra, and
> published by the ACM in 1968. (A recent reprint can be found at
> http://www.acm.org/classics/oct95/.) As you can tell from the date, this
> article predates modern programming languages and idioms; it comes from a
> time when Fortran ruled, and before Fortran 77 provided significant tools
> for avoiding spaghetti code.

It only goes to show that the core of computer science hasn't changed
over the years. While some technology changes, the science itself stays
the same. Much like a physicist should know the outdated theories of
relativity by Albert Einstein (made way back in the 20th century as
well) a good computer scientist should appreciate the contributions of
its founders, and have a good grasp of their core contributions. I'm
not claiming to be an expert in either field, however.

Of course, this isn't a computer science mailing list, this is a linux
kernel mailing list, so I apologize for getting off topic. Let's agree
to drop it here.

-Rob

Randy Dunlap

unread,
Jan 12, 2003, 7:57:34 PM1/12/03
to
> On Sun, 12 Jan 2003 14:59:57 EST, Rob Wilkens said:
>
>> In general, if you can structure your code properly, you should never need
>> a goto, and if you don't need a goto you shouldn't use it. It's just
>> "common sense" as I've always been taught. Unless you're
>> intentionally trying to write code that's harder for others to read.
>
> Now, it's provable you never *NEED* a goto. On the other hand, *judicious*
> use of goto can prevent code that is so cluttered with stuff of the form:
>
> if(...) {
> ...
> die_flag = 1;
> if (!die _flag) {...
>
> Pretty soon, you have die_1_flag, die_2_flag, die_3_flag and so on, rather
> than 3 or 4 "goto bail_now;".

Right.

> The real problem is that C doesn't have a good multi-level "break"
> construct. On the other hand, I don't know of any language that has a good
> one - some allow "break 3;" to break 3 levels- but that's still bad because
> you get screwed if somebody adds an 'if' clause....


The one that I used in a previous life was like so. No "while"
or "for" constructs, only "do thisloop forever" with conditionals
all being explicitly coded inside the loop(s). All based on:
do [loopname] [forever];
{block};
end [loopname];

with {block} possibly containing "undo [loopname]".
An unnamed undo just terminates the innermost loop.
Named undo's can be used to terminate any loop level.

~Randy

Randy Dunlap

unread,
Jan 12, 2003, 8:10:07 PM1/12/03
to
>> Harder to read: The primary code path is polluted with repetative code
>> that has no bearing on its primary mission.
>
> I thought it was easier to read. For me, I can read "ok, this condition
> happens, I fail"... Or "if this other condition happens, I release my path,
> then I fail"...

over and over, with additions each time?

> Whereas the "goto out" was very unclear. It made me page down to figure out
> what was going on.
>
> That's the whole point.. To just browse the code.. I shouldn't have to page
> down to understand what the code right in front of me is doing. "goto out"
> is unclear. "retun error" is clear. "path_release" seems like a relatively
> plain english function name and I can guess what it does without knowing

> exactly what it does. I can also surmise that if I go beyond a certain


> point in the function that I need to path_release() the same way a
> non-kernel programmer might need to free memory allocated inside of a
> function before returning to the calling function.

So you choose to agree with your lessons that goto is bad, but
ignore the other lesson that functions shouldn't have multiple
return paths? Or didn't have that lesson?

Michael Kingsbury

unread,
Jan 12, 2003, 8:10:08 PM1/12/03
to
> Say, I've been having _smashing_ success with 2.5.x on the desktop and
> on big fat highmem umpteen-way SMP (NUMA even!) boxen, and I was
> wondering if you were considering 2.6.0-test* anytime soon.
>
> I'd love to get this stuff out for users to hammer on ASAP, and things
> are looking really good AFAICT.
>
> Any specific concerns/issues/wishlist items you want taken care of
> before doing it or is it a "generalized comfort level" kind of thing?
> Let me know, I'd be much obliged for specific directions to move in.

As long as I can't unzip the latest, do a quick

'make xconfig' w/o changing the defaults,

followed by

'make bzImage',

and actually get the sucker to compile, I'd say go for it. But there's
still too much breakage. And until that breakage is fixed, you won't get
the next wave of people (users) who are curious to poke around with it.
(some of us don't care to take half a day just to figure out what to do to make something compile because
there's problems in the source.)

(Yes, I realize I have to customize the kernel some. But at the very
least the defaults shouldn't cause it to fall flat on its face.)

-mike

David Lang

unread,
Jan 12, 2003, 8:42:26 PM1/12/03
to
> I've only compiled (and haven't tested this code), but it should be much
> faster than the original code. Why? Because we're eliminating an extra
> "jump" in several places in the code every time open would be called.
> Yes, it's more code, so the kernel is a little bigger, but it should be
> faster at the same time, and memory should be less of an issue nowadays.

Rob, one thing you may not have noticed since you haven't been following
the list for a while is that with the current generation of computers size
frequently translates directly into speed and a lot of the time honored
performance tricks that trade size for fewer commands executed end up
being losses.

this can be seen by compiling code with -O2 and with -Os and frequently
the -Os will actually be faster.

This is becouse not all memory is equal, main memory is very slow compared
to the CPU cache, so code that is slightly larger can cause more cache
misses and therefor be slower, even if significantly fewer commands are
executed.

in addition frequently the effect isn't direct (i.e. no noticable
difference on the code you are changing, but instead the change makes
other code slower as it gets evicted from the cache)

unfortunantly while this effect is known the rules of when to optimize for
space and when to optimize for fewer cpu cycles for code execution are not
clear and vary from CPU to CPU frequently within variations of the same
family)

if you google for -Os you should find one of the several discussions on
the list in the last year on the subject.

David Lang

Nuno Monteiro

unread,
Jan 12, 2003, 8:55:06 PM1/12/03
to
On 13.01.03 01:21 Rob Wilkens wrote:
>
> I didn't have a lesson about multiple return paths.. I actually before
> posting checked out the coding style document..
>
> :/usr/src/linux-2.5.56/Documentation# grep -i return CodingStyle
>
> Returned nothing.. There was no mention of using return in the middle
> or end of a function in the coding style document, though perhaps there
> should be.
>

You dont realistically expect to learn how to program reading
Documentation/CodingStyle, do you? CodingStyle merely serves as general
guidelines of what is considered good practice among kernel development,
no more no less. If you dont know how to code in the first place (which
you have proved spectacularly, by now), you're SOL.

Go educate yourself and stop trolling. Somewhere, along the bucketloads
of vomit you insisted in throwing here, you asked who were the other
top-rank trolls -- sincerely, and as far as I can remember, you're ahead
by a fair margin. Not even ime...@trustix.co.id with his delireous
"single user linux" crap came anywhere near you, although he was, too,
very resistant to being clued in.

See things from this end -- when 99% of the people tell you you're wrong,
consider the possibility you are _actually_ wrong, and not the other way
around.

Now, off to ~/.killfile you go.


Regards,


Nuno

Rob Wilkens

unread,
Jan 12, 2003, 9:27:01 PM1/12/03
to
On Sun, 2003-01-12 at 20:51, Nuno Monteiro wrote:
> You dont realistically expect to learn how to program reading
> Documentation/CodingStyle, do you? CodingStyle merely serves as general

No, but I would expect rules of kernel coding to be in there. For
example, we were talking about using return mid-function. I always
considerred that perfectly OK. Others were saying that's a no-no. If
it's a no-no, it should be documented. That is, if it's a no-no. That
may be a situational thing though.

> See things from this end -- when 99% of the people tell you you're wrong,
> consider the possibility you are _actually_ wrong, and not the other way
> around.

99% of the people are running a microsoft based OS (It has 95% of the
market in terms of sales, but factor in pirated copies and your figure
will get close), of the 0.5% running linux, 0.025% of them are probably
subscribed to this mailing list. Of that, probably 5% read it
regularly. Now, of those 5% (of 0.25% of 0.5%), I'm probably getting
5-10 people reacting negatively (the rest are neutral reactions or no
reactions), and claiming to speak (in a rather -- shall we say --
arrogant and self-important manor) for the list as a whole.

> Now, off to ~/.killfile you go.

Yeah, yeah, should've done it before..

-Rob

p.s. Yes, all statistics are made up on the spot for comical purposes
only and are only half-way intended to represent the real world. p.p.s.
given those percentages, 5-10 people may be a lot percentagewise, I
don't know, I haven't done the math.

Marcelo Pacheco

unread,
Jan 12, 2003, 9:56:38 PM1/12/03
to
>
>
>99% of the people are running a microsoft based OS (It has 95% of the
>market in terms of sales, but factor in pirated copies and your figure
>will get close), of the 0.5% running linux, 0.025% of them are probably
>subscribed to this mailing list. Of that, probably 5% read it
>regularly. Now, of those 5% (of 0.25% of 0.5%), I'm probably getting
>5-10 people reacting negatively (the rest are neutral reactions or no
>reactions), and claiming to speak (in a rather -- shall we say --
>arrogant and self-important manor) for the list as a whole.
>
No Rob, most people don't bother responding because they read the FAQ
for the list and they realize that this is waste of bandwidth of a lot
of people that are important to Linux. You're wasting their times and
this is why this should stop NOW.

You're soo arrogant you don't want to see how much YOU'RE wrong ?
See, I'm a computer geek that started at 11, that learned everything
from the practical viewpoint (first assembly then Basic then structured
language) and even going through structured programming classes 7 years
latter they still managed to get brainwashed to the point that just now
I realize that goto's are really a good thing, and that it should be
used as standard practice (college has been 10+ years ago). So open your
mind to the fact that you were brainwashed, and the fact that they
brainwash so many people that aren't smart enough to question them, that
everything is taken at face value.

And Linux is not a hobbist thing anymore. From the point a lot of people
trust their businesses to Linux like I do everyday (I'm MIS with about
12 Penguins, with about 30 more Microshits to be converted to Penguins),
then you should respect people here. Me and I'm sure a lot others read
this list daily, and they don't want to spend their time on this thread.
I apreciate very much the work of everybody here, considering that they
could probably make a hell of a lot more money through other ways, but
they decided to go the open source way, and the whole world's society is
benefiting from their work, instead of mostly benefiting big, greedy
companies like microshit.

By the way, you're not being paid my Microshit or other companies that
have a lot to benefit from stalling Linux development to troll this
list, aren't you ?

Marcelo Pacheco

Jochen Striepe

unread,
Jan 12, 2003, 10:15:22 PM1/12/03
to
Hi,

On 12 Jan 2003, Rob Wilkens wrote:
>
> [...] Now, of those 5% (of 0.25% of 0.5%), I'm probably getting


> 5-10 people reacting negatively (the rest are neutral reactions or no

> reactions), [...]

You don't think people might be silent because they think everything
relevant was said by those 5-10 people?

Please do assume long-time kernel hackers *know* what they do. Asking
of course is OK, but please do not consider anything they do plain
wrong just because you don't understand after some seconds of thinking.
If you imply they're too dumb to do their job, you *will* get negative
reactions, like it or not.

If you think there's not enough documentation, try the web (including
the mailing list archive) or some kernel hacking books. If some things
are *not* documented at all (that is, you don't find the proper
documentation), ask and write some documentation yourself or ask
yourself were to get a grip on techniques your not familiar with.


Thanks,

Jochen.

--
The sequence is important. People just learning to pee sometimes
put the steps in the wrong order, with frustrating results.
Maybe a checklist would be useful to those just starting out.
-- Brien K. Meehan; Msg-ID <tkolgcf...@news.supernews.com>

Rob Wilkens

unread,
Jan 12, 2003, 10:15:36 PM1/12/03
to
On Sun, 2003-01-12 at 21:00, Ryan Anderson wrote:

> On Sun, Jan 12, 2003 at 05:51:57PM -0500, Rob Wilkens wrote:
> > "easier" is the motivation behind using goto.. Laziness.. that's all it
> > is.
>
> Have you ever read "Programming Perl"?
>
> Laziness is a virtue. (At least I think that's the right book -
> possibly the 3rd edition.)

Actually, it's one of the few books I actually still own (the perl
one).. Laziness is indeed a virtue.. One I live my life by. Of
course, that's why I haven't gotten off my ass all day and have been
doing nothing but reading/writing mails from this list. :-0 If the
list accepted binaries, I'd take a photo of my room and workspace, and
you'd get the impression that I was one of the laziest people you've
ever met. Granted, one of my computer science professors (not one of
the greatest professors I ever had) said that the best programmers in
the world are the laziest sons of bitches you'll ever meet, and I think
I may qualify (as a lazy son of a bitch, not necessarily as a great
programmer).

Aww.. Heck.. Go ahead and use goto all you want. I'm not in a position
to stop anyone anyway.

-Rob

Paul Mackerras

unread,
Jan 12, 2003, 10:38:49 PM1/12/03
to
Alan Cox writes:

> The pcmcia stuff crashes erratically with the old IDE the bugs have not
> changed but the problem now shows up far more often. With a 2Gb type II
> PCMCIA IDE disk I get a crash about 1 in 3 ejects now.

I'm using the patch below, which makes sure that ide_release (in
ide-cs.c) runs in process context not interrupt context. The specific
problem I saw was that ide_unregister calls various devfs routines
that don't like being called in interrupt context, but there may well
be other thing ide_unregister does that aren't safe in interrupt
context.

I am also hitting another problem on my powerbook that I would like
your advice on. The thing is that my powerbook (a 500MHz G4 titanium
powerbook) has 3 IDE interfaces, but only the first two have anything
attached. The pmac IDE driver goes through and initializes all three,
including setting hwif->selectproc for each one. Then, when I plug in
a CF card in the pcmcia/cardbus slot, ide_register_hw goes and looks
for a free ide_hwifs[] entry. Slot 2 appears to be available because
there are no devices attached, and so it uses that. In the process of
setting up ide_hwifs[2], it sets hwif->io_ports but it doesn't reset
selectproc (or hwif->OUTB etc., AFAICS). The result is that it calls
the pmac IDE selectproc, which crashes (since we munged hwif->io_ports
without telling it).

I have "fixed" the problem for now by adding a call to
init_hwif_data(index) in ide_register_hw just before the first
memcpy. I'd be interested to know what the right fix is. :)

Regards,
Paul.

diff -urN linux-2.4/drivers/ide/legacy/ide-cs.c pmac/drivers/ide/legacy/ide-cs.c
--- linux-2.4/drivers/ide/legacy/ide-cs.c 2002-12-19 11:50:35.000000000 +1100
+++ pmac/drivers/ide/legacy/ide-cs.c 2003-01-13 14:02:31.000000000 +1100
@@ -92,10 +92,11 @@
int ndev;
dev_node_t node;
int hd;
+ struct tq_struct rel_task;
} ide_info_t;

static void ide_config(dev_link_t *link);
-static void ide_release(u_long arg);
+static void ide_release(void *arg);
static int ide_event(event_t event, int priority,
event_callback_args_t *args);

@@ -136,9 +137,8 @@
if (!info) return NULL;
memset(info, 0, sizeof(*info));
link = &info->link; link->priv = info;
+ INIT_TQUEUE(&info->rel_task, ide_release, link);

- link->release.function = &ide_release;
- link->release.data = (u_long)link;
link->io.Attributes1 = IO_DATA_PATH_WIDTH_AUTO;
link->io.Attributes2 = IO_DATA_PATH_WIDTH_8;
link->io.IOAddrLines = 3;
@@ -187,6 +187,7 @@
static void ide_detach(dev_link_t *link)
{
dev_link_t **linkp;
+ ide_info_t *info = link->priv;
int ret;

DEBUG(0, "ide_detach(0x%p)\n", link);
@@ -197,9 +198,10 @@
if (*linkp == NULL)
return;

- del_timer(&link->release);
- if (link->state & DEV_CONFIG)
- ide_release((u_long)link);
+ if (link->state & DEV_CONFIG) {
+ schedule_task(&info->rel_task);
+ flush_scheduled_tasks();
+ }

if (link->handle) {
ret = CardServices(DeregisterClient, link->handle);
@@ -209,7 +211,7 @@

/* Unlink, free device structure */
*linkp = link->next;
- kfree(link->priv);
+ kfree(info);

} /* ide_detach */

@@ -381,7 +383,7 @@
cs_failed:
cs_error(link->handle, last_fn, last_ret);
failed:
- ide_release((u_long)link);
+ ide_release(link);

} /* ide_config */

@@ -393,12 +395,15 @@

======================================================================*/

-void ide_release(u_long arg)
+static void ide_release(void *arg)
{
- dev_link_t *link = (dev_link_t *)arg;
+ dev_link_t *link = arg;
ide_info_t *info = link->priv;

- DEBUG(0, "ide_release(0x%p)\n", link);
+ if (!(link->state & DEV_CONFIG))
+ return;
+
+ DEBUG(0, "ide_do_release(0x%p)\n", link);

if (info->ndev) {
/* FIXME: if this fails we need to queue the cleanup somehow
@@ -435,6 +440,7 @@
event_callback_args_t *args)
{
dev_link_t *link = args->client_data;
+ ide_info_t *info = link->priv;

DEBUG(1, "ide_event(0x%06x)\n", event);

@@ -442,7 +448,7 @@
case CS_EVENT_CARD_REMOVAL:
link->state &= ~DEV_PRESENT;
if (link->state & DEV_CONFIG)
- mod_timer(&link->release, jiffies + HZ/20);
+ schedule_task(&info->rel_task);
break;
case CS_EVENT_CARD_INSERTION:
link->state |= DEV_PRESENT | DEV_CONFIG_PENDING;

yoda...@fsmlabs.com

unread,
Jan 12, 2003, 10:38:52 PM1/12/03
to

See Brian Kernighan's explanation in 1981. Still good.

http://bit.csc.lsu.edu/tutorial/ten-commandments/bwk-on-pascal.html


On Sun, Jan 12, 2003 at 12:22:26PM -0800, Linus Torvalds wrote:


>
> On Sun, 12 Jan 2003, Rob Wilkens wrote:
> >
> > However, I have always been taught, and have always believed that
> > "goto"s are inherently evil. They are the creators of spaghetti code
>

> No, you've been brainwashed by CS people who thought that Niklaus Wirth
> actually knew what he was talking about. He didn't. He doesn't have a
> frigging clue.


>
> > (you start reading through the code to understand it (months or years
> > after its written), and suddenly you jump to somewhere totally
> > unrelated, and then jump somewhere else backwards, and it all gets ugly
> > quickly). This makes later debugging of code total hell.
>

> Any if-statement is a goto. As are all structured loops.
>
> Ans sometimes structure is good. When it's good, you should use it.
>
> And sometimes structure is _bad_, and gets into the way, and using a
> "goto" is just much clearer.
>
> For example, it is quite common to have conditionals THAT DO NOT NEST.
>
> In which case you have two possibilities
>
> - use goto, and be happy, since it doesn't enforce nesting
>
> This makes the code _more_ readable, since the code just does what
> the algorithm says it should do.
>
> - duplicate the code, and rewrite it in a nesting form so that you can
> use the structured jumps.
>
> This often makes the code much LESS readable, harder to maintain,
> and bigger.
>
> The Pascal language is a prime example of the latter problem. Because it
> doesn't have a "break" statement, loops in (traditional) Pascal end up
> often looking like total shit, because you have to add totally arbitrary
> logic to say "I'm done now".
>
> Linus


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

--
---------------------------------------------------------
Victor Yodaiken
Finite State Machine Labs: The RTLinux Company.
www.fsmlabs.com www.rtlinux.com
1+ 505 838 9109

Rob Wilkens

unread,
Jan 12, 2003, 10:47:44 PM1/12/03
to
On Sun, 2003-01-12 at 21:51, Marcelo Pacheco wrote:
> No Rob, most people don't bother responding because they read the FAQ
> for the list and they realize that this is waste of bandwidth of a lot
> of people that are important to Linux. You're wasting their times and
> this is why this should stop NOW.

Actually, we were talking in private e-mail, so if we were discussing
the FAQ, you would've continued this there, but since you've decided to
bring this discussion out here, I'll continue it here for now..

I have unlimitted bandwidth, as do many others. IF they don't they
shouldn't subscribe to this list, they should subscribe to the newsgroup
and pick-and-choose by sbject/thread.

> You're soo arrogant you don't want to see how much YOU'RE wrong ?

I'm perfectly willing to see how much I'm wrong. No one has yet shown
me that I'm wrong, though. I'd be perfectly willing to see it if it
were shown.

> See, I'm a computer geek that started at 11, that learned everything

I have a head start on you then, I started around age 8. TI-99/4A & the
BASIC programming language back then (I even had a voice box for the
computer so I was writing programs that could talk which I thought was
the coolest thing.) Moved on to the Commodore 64, 128, then moved into
the PC world kind of late, in 1990 I bought an XT 10MHz machine that
happenned to work with the commodore 128 monitor, and since then have
incrementally upgrade part by part to my current machine (haven't
upgraded in three years).

> mind to the fact that you were brainwashed, and the fact that they
> brainwash so many people that aren't smart enough to question them, that
> everything is taken at face value.

If I'm questioning people on this list, what makes you think I wasn't
smart enough to question them then?? Think about that one for a second?

> And Linux is not a hobbist thing anymore. From the point a lot of people
> trust their businesses to Linux like I do everyday (I'm MIS with about
> 12 Penguins, with about 30 more Microshits to be converted to Penguins),

Serverwise or backofficewise, penguins have their place. On the
desktop, penguins have to learn to adapt to a different climate. That's
off topic for the kernel list though.

> then you should respect people here. Me and I'm sure a lot others read
> this list daily, and they don't want to spend their time on this thread.

Then don't read it, skip over to the next message and move on with
life. As someone else put it, life is too short.

> I apreciate very much the work of everybody here, considering that they
> could probably make a hell of a lot more money through other ways, but
> they decided to go the open source way, and the whole world's society is
> benefiting from their work, instead of mostly benefiting big, greedy
> companies like microshit.

Few would be doing it if there weren't SOME benefit for themselves.
That benefit _may_ be a better system for themselves.. Some may be the
intellectual satisfaction of a job well done.. Some may work for
companies like red hat or suse.. Some may work for companies that want
to replace old UNIX systems with cheaper PC hardware that does the same
thing... Everyone has a motivating factor.

> By the way, you're not being paid my Microshit or other companies that
> have a lot to benefit from stalling Linux development to troll this
> list, aren't you ?

The answer to this is that I'm not in any way shape or form employed by
any company what so ever at present. I'm considering part time work,
but will not say more about that because the last time I mentioned
something about an employer on a list where people thought I was a troll
(alt.support.depression.manic around 2000 when I was unmedicated) they
were sending faxes and calling the office where I worked at with
complaints. I'd rather not forewarn any employer that I'm a little
off-center, though I guess it wouldn't hurt my chances of eventually
getting on social security disability which would cover my bills.

-Rob

Ryan Anderson

unread,
Jan 12, 2003, 10:58:09 PM1/12/03
to
On Sun, Jan 12, 2003 at 08:21:50PM -0500, Rob Wilkens wrote:
> :/usr/src/linux-2.5.56/Documentation# grep -i return CodingStyle
>
> Returned nothing..

As annoying as this thread is, there are occassional valid points.

A first pass at an addition:

--- local/Documentation/CodingStyle.orig 2002-11-19 03:02:32.000000000 -0500
+++ local/Documentation/CodingStyle 2003-01-12 22:44:39.000000000 -0500
@@ -264,3 +264,27 @@

Remember: if another thread can find your data structure, and you don't
have a reference count on it, you almost certainly have a bug.
+
+ Chapter 9: Error handling
+
+Error handling in functions needs to follow a few simple rules. If the
+function has allocated resources (irqs, memory), taken a reference,
+grabbed locks, etc, all of those allocations must be reversed when
+returning an error.
+
+In functions that depend on several allocations, the preferred way to
+return the error is with with the use of a few "goto"s that point at an
+error block at the end of the function, after the normal, successful
+return.
+
+Several consecutive lables can be used, reversing the order of the
+allocations. For example, if memory is allocated, a lock taken, and
+an irq activated, the error labels might be labeled "err_noirq",
+"err_nolock", "err_nomem", in order. The final step would be to
+return the error.
+
+The reason for this style is very simple: Multiple return paths from
+the same block of code are extremely confusing, and make verification
+that locks are balanced, memory is properly freed, and that reference
+counts are not leaked very difficult.
+


--

Ryan Anderson
sometimes Pug Majere

Greg KH

unread,
Jan 13, 2003, 1:26:58 AM1/13/03
to
On Sun, Jan 12, 2003 at 11:15:52AM -0800, Linus Torvalds wrote:
>
> Anybody willing to test it and see if the above works?

Hm, I just did, and it seems to work for me. I ran a bunch of different
usb-serial drivers through some tests, but these worked for me without
this patch. I don't know what stress tests Alan was running to show up
a problem.

Anyway, here's a patch with your new lock, if you want to apply it.

thanks,

greg k-h


diff -Nru a/drivers/char/tty_io.c b/drivers/char/tty_io.c
--- a/drivers/char/tty_io.c Sun Jan 12 22:27:42 2003
+++ b/drivers/char/tty_io.c Sun Jan 12 22:27:42 2003
@@ -159,6 +159,58 @@
extern void tx3912_rs_init(void);
extern void hvc_console_init(void);

+
+/*
+* This isn't even _trying_ to be fast!
+*/
+struct recursive_spinlock {
+ spinlock_t lock;
+ int lock_count;
+ struct task_struct *lock_owner;
+};
+
+static struct recursive_spinlock tty_lock = {
+ .lock = SPIN_LOCK_UNLOCKED,
+ .lock_count = 0,
+ .lock_owner = NULL
+};
+
+unsigned long tty_spin_lock(void)
+{
+ unsigned long flags;
+ struct task_struct *tsk;
+
+ local_irq_save(flags);
+ preempt_disable();
+ tsk = current;
+ if (spin_trylock(&tty_lock.lock))
+ goto got_lock;
+ if (tsk == tty_lock.lock_owner) {
+ WARN_ON(!tty_lock.lock_count);
+ tty_lock.lock_count++;
+ return flags;
+ }
+ spin_lock(&tty_lock.lock);
+got_lock:
+ WARN_ON(tty_lock.lock_owner);
+ WARN_ON(tty_lock.lock_count);
+ tty_lock.lock_owner = tsk;
+ tty_lock.lock_count = 1;
+ return flags;
+}
+
+void tty_spin_unlock(unsigned long flags)
+{
+ WARN_ON(tty_lock.lock_owner != current);
+ WARN_ON(!tty_lock.lock_count);
+ if (!--tty_lock.lock_count) {
+ tty_lock.lock_owner = NULL;
+ spin_unlock(&tty_lock.lock);
+ }
+ preempt_enable();
+ local_irq_restore(flags);
+}
+
static struct tty_struct *alloc_tty_struct(void)
{
struct tty_struct *tty;
@@ -460,7 +512,7 @@
{
unsigned long flags;

- local_irq_save(flags); // FIXME: is this safe?
+ flags = tty_spin_lock();
if (tty->ldisc.flush_buffer)
tty->ldisc.flush_buffer(tty);
if (tty->driver.flush_buffer)
@@ -468,7 +520,7 @@
if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) &&
tty->ldisc.write_wakeup)
(tty->ldisc.write_wakeup)(tty);
- local_irq_restore(flags); // FIXME: is this safe?
+ tty_spin_unlock(flags);
}

wake_up_interruptible(&tty->write_wait);
@@ -1926,7 +1978,7 @@
fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
tty->flip.buf_num = 0;

- local_irq_save(flags); // FIXME: is this safe?
+ flags = tty_spin_lock();
tty->flip.char_buf_ptr = tty->flip.char_buf;
tty->flip.flag_buf_ptr = tty->flip.flag_buf;
} else {
@@ -1934,13 +1986,13 @@
fp = tty->flip.flag_buf;
tty->flip.buf_num = 1;

- local_irq_save(flags); // FIXME: is this safe?
+ flags = tty_spin_lock();
tty->flip.char_buf_ptr = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
tty->flip.flag_buf_ptr = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
}
count = tty->flip.count;
tty->flip.count = 0;
- local_irq_restore(flags); // FIXME: is this safe?
+ tty_spin_unlock(flags);

tty->ldisc.receive_buf(tty, cp, fp, count);

Matti Aarnio

unread,
Jan 13, 2003, 4:47:47 AM1/13/03
to
On Sun, Jan 12, 2003 at 09:19:28PM -0500, Rob Wilkens wrote:
> On Sun, 2003-01-12 at 20:51, Nuno Monteiro wrote:
> > You dont realistically expect to learn how to program reading
> > Documentation/CodingStyle, do you? CodingStyle merely serves as general
>
> No, but I would expect rules of kernel coding to be in there. For
> example, we were talking about using return mid-function. I always
> considerred that perfectly OK. Others were saying that's a no-no. If
> it's a no-no, it should be documented. That is, if it's a no-no. That
> may be a situational thing though.


That document is short for several reasons.

- It has been written for particular needs, and updated rarely

- Most long-time kernel coders really don't need detailed guidelines,
although newcomers (like yourself) apparently want something..

Things you need to understand include at least:

- Linux kernel is one of the most complicated codes that gcc compiler
is ever put to work on, and over the years we have unearthed tons of
optimizer bugs. Many of somewhat odd looking coding details do
originate from circumventions of those compiler bugs.

- Advanced optimizer hinting features, like unlikely() attribute
are very new in (this) compiler, and while they in theory move the
"unlikely" codes out of the fast-path, such things have been buggy
in the past, and we are worried of bug effects...

- Why code in a manner, where the optimizer needs to work very hard to
produce fast code, when simplified structures, like those with 'goto'
in them, can result the same with much less effort ?

- Knowing effects of those optimizations requires profiling, and studying
produced assembly code on different target systems.

- Linux is multi-CPU-architecture kernel, looking at it via "intel-only,
lattest Pentium-XIV" viewpoint is not going to cover all cases. Making
changes that bloat code will make system that much harder to fit into
smaller embedded machines.

- Sometimes you need to code differently to achieve higher performance
at different processors. See drivers/md/xor.c and header files
it includes. Especially all include/asm-*/xor.h files, including
include/asm-generic/xor.h ... (That shows to you how different
approaches work at different systems. What works fast in register-
rich processors does not necessarily be fast in register-poor things,
like i386 series.)

- Adding function classifiers like "static inline" do change rules
somewhat. (Especially things like "mid-function return".)

- Larger code, either with manually replicated exit cleanup (which
are pain to maintain, when some large-scale change is needed), or
via excessive use of "static inline", will pressure caches, and
makes it more likely to drop soon to be again needed code (or data)
from cache. Cases where cache-miss costs your tens, or hundreds
of processor clock cycles that is really important question.

- Explaining things like GCC asm() statement syntaxes isn't a thing
for Linux kernel documentation. (Those vary from target processor
to another!)


I repeat:

To understand effects of various coding details, you need to study produced
assembly code, and keep in mind that in modern processors a branch out of
straight-line execution path (after optimizations) is expensive (in time),
and cache-misses are even more expensive.

Then you need to benchmark those codes with different loads (where
applicable) to find out what caches affect them -- and when you must
bypass caches (they are limited resource, after all) in order to give
overall improved system performance, when your code does not cause
actively used code or data to be evicted from caches unnecessarily.


> -Rob

/Matti Aarnio

Horst von Brand

unread,
Jan 13, 2003, 5:57:19 AM1/13/03
to
Linus Torvalds <torv...@transmeta.com> said:
> On 12 Jan 2003, Robert Love wrote:

> > On Sun, 2003-01-12 at 15:22, Linus Torvalds wrote:
> >
> > > No, you've been brainwashed by CS people who thought that Niklaus
> > > Wirth actually knew what he was talking about. He didn't. He
> > > doesn't have a frigging clue.
> >
> > I thought Edsger Dijkstra coined the "gotos are evil" bit in his
> > structured programming push?

> Yeah, he did, but he's dead, and we shouldn't talk ill of the dead. So
> these days I can only rant about Niklaus Wirth, who took the "structured
> programming" thing and enforced it in his languages (Pascal and Modula-2),
> and thus forced his evil on untold generations of poor CS students who had
> to learn langauges that weren't actually useful for real work.

You did not live through the horror that was FORTRAN IV, I learned
programming in it (no structure except DO, a primitive for). Sure, Pascal
strictures are overkill, but in a sense they were needed to implant a bit
of structure into the mess that usually came before.

> (Yeah, yeah, most _practical_ versions of Pascal ended up having all the
> stuff necessary to break structure, but as you may be able to tell, I was
> one of the unwashed masses who had to write in "standard Pascal" in my
> youth. I'm scarred for life).

And you do break structure only when necessary, so the dosis of Pascal did
you some good after all ;-)
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513

Helge Hafting

unread,
Jan 13, 2003, 6:09:36 AM1/13/03
to
Rob Wilkens wrote:

> > Any if-statement is a goto. As are all structured loops.
>

> "structured" being a key word. I like structure in code. Let's face
> it, everyone likes code to be well structured, that's why there's the
> "CodingStyle" document -- people like the code to look a certain way so
> they can read it later. Without structure, you don't have that
>
If you like "structure" so much, consider the fact that gotos are
used in a very structured way in the kernel. Books won't list
goto as a "structured keyword" but that don't prevent
it from being used in a structured and diciplined manner.

"Avoid gotos" is only a rule of thumb, for those that don't
know the finer points. Gotos have a enormous potential for abuse,
but it isn't abused in the kernel because it is used by experts who
limit themselves to only using it right. (Well, most of the time, but
that isn't what you complain about. :-)

One cannot expect a newbie to get this right while learning the
language,
so beginners get a "don't use gotos" rule. And therefore it takes time
before they learn to read code containing gotos too - even _good_ code.

Please note that rules of thumb aren't absolute. There is another
well known - don't play with fire. Generally good, but some people
have to do a lot of it anyway in order to design better fire
extinguishers
and train firefighters.

> > Ans sometimes structure is good. When it's good, you should use it.
>

> Ok, we agree there.


>
> > And sometimes structure is _bad_, and gets into the way, and using a
> > "goto" is just much clearer.
>

> I can't foresee many scenarios where this would be the case. Someone
> spoke, for example, about being indented too far and needing to jump out
> because there isn't a good "break" mechanism... Well, as it says in the
> coding style document -- if you're indented more then 3 levels in,
> "you're screwed anyway, and should fix your program".

Sometimes using gotos makes things clearer, _for those that have some
experience with this particular way of using gotos_.

_Of course_ it isn't the clearest to _you_, because you, as a newbie,
have noe training or experience in this. Get experience, learn to
recognise the very limited use of goto in the kernel, and you'll
probably agree.

Consider someone who takes a C course where they don't teach "switch"
because "if" can do the same thing. He would consider "switch"
unnecessary. And hard to read, due to lack of experience. And just
like
you, he would urge people to not use switch because _he_ finds it harder
to read. That doesn't make avoiding switch the right thing to do
though.

The same applies to goto. It is a useful thing - if used right.
[...]
> As someone else pointed out, it's provable that goto isn't needed, and
> given that C is a minimalist language, I'm not sure why it was included.

Again, switch is as unnecessary, yet nobody wants to remove it. And
yes,
switch can be abused too (replace all ifs with switch for example.)
The rule against goto is a beginners simplification, because the real
rules for using goto are harder to express. They are wasted on people
who struggle with "for" and pointers - and then the beginners course
ends.

> > The Pascal language is a prime example of the latter problem. Because it
> > doesn't have a "break" statement, loops in (traditional) Pascal end up
> > often looking like total shit, because you have to add totally arbitrary
> > logic to say "I'm done now".
>

> But at least the code is "readable" when you do that.

Readable for who, is the question. Some limited use of goto don't take
that long to learn. I used to simply think "urgh, a goto" but didn't
complain - it wasn't _my_ kernel. Now I understand that kind of code
and approve. And I shudder thinking about some code I've written for
pay - doing ten allocations in a row, testing each, and freeing
everything in a nested fashion upon possible failure.

> Sure it's a
> little more work on the part of the programmer.
Now, if we can avoid that extra work _without_ harm, then it is
certainly worth it, no?

> But anyone reading the
> code can say "oh, so the loop exits when condition x is met", rather
> than digging through the code looking for any place their might be a
> goto.

What you describe is the problem with undiciplined use of goto. Kernel
code don't use gotos in any arbitrary way. Check, and you'll see
that 99% of it it is very diciplined, using gotos in one
well-defined way. Much of it is explicitly coded function-internal
"exceptions", a structured concept newer than C.

(Yyou can surely find some bad examples too. Either hacks with a
special reason, or things that really ought to be cleaned up.)

> Of course, I'm not about to argue that the kernel should be rewritten
> in PASCAL, nor am I interested in reinventing the wheel. We've got a
> good system here, let's just keep it good and make it better.

Making it better is interesting. Note that performance sometimes
overrules style, and yes - there are cases where goto wins
on raw performance. And that might pay off if it is a very
often executed piece of code.

Also, avoiding goto at all cost is extra work for programmers
_every time_. Learning to use gotos "right" (I.e. advanced use of C)
is one-time learning and then it is free - everytime.

The use of gotos may be a learning curve for every kernel newbie, but
so is understanding the VFS or the VM system or any other big
complicated piece. Wether or not that piece uses gotos. :-)

Helge Hafting

Eric W. Biederman

unread,
Jan 13, 2003, 6:13:25 AM1/13/03
to
Linus Torvalds <torv...@transmeta.com> writes:

> On 12 Jan 2003, Robert Love wrote:
> > On Sun, 2003-01-12 at 15:22, Linus Torvalds wrote:
> >
> > > No, you've been brainwashed by CS people who thought that Niklaus
> > > Wirth actually knew what he was talking about. He didn't. He
> > > doesn't have a frigging clue.
> >
> > I thought Edsger Dijkstra coined the "gotos are evil" bit in his
> > structured programming push?
>

Actually Edsger Dijkstra wrote a paper entitled "Goto considered Harmful"
He never called them evil. And the title was more for shock value to
grab peoples attention. For the beginners Dijkstra did not
distinguish between goto, break, return, and continue.

You can find many of his papers at:
http://www.cs.utexas.edu/users/EWD/

What he was after was simple and maintainable code, and from
everything I have read, I think he would have no major problems with
the Linux kernel code.

In the category of maintainability, and simplicity Dijkstra made
certain the first hardware implementation of an interrupt handler he
worked with had the ability to completely save the interrupt state so
he could later return to the interrupted code. To make writing to a
printer a non-realtime task he invented the semaphore.

> Yeah, he did, but he's dead, and we shouldn't talk ill of the dead.

Linus I think if he was alive you would have enjoyed talking to him,
your view points seem to mesh quite well.

> So
> these days I can only rant about Niklaus Wirth, who took the "structured
> programming" thing and enforced it in his languages (Pascal and Modula-2),
> and thus forced his evil on untold generations of poor CS students who had
> to learn langauges that weren't actually useful for real work.

Now that is something worth criticizing!

Though standard Pascal was not unusable because of the lack of a goto,
that was the one feature it actually had. But I suspect a lot of
teachers failed to mention it.

>
> (Yeah, yeah, most _practical_ versions of Pascal ended up having all the
> stuff necessary to break structure, but as you may be able to tell, I was
> one of the unwashed masses who had to write in "standard Pascal" in my
> youth. I'm scarred for life).


Well you seem to be coping well. Just a little Pascal phobia there...

Eric

Terje Eggestad

unread,
Jan 13, 2003, 9:39:14 AM1/13/03
to
On man, 2003-01-13 at 12:09, Eric W. Biederman wrote:
> Linus Torvalds <torv...@transmeta.com> writes:
>
> > On 12 Jan 2003, Robert Love wrote:
> > > On Sun, 2003-01-12 at 15:22, Linus Torvalds wrote:
> > >
> > > > No, you've been brainwashed by CS people who thought that Niklaus
> > > > Wirth actually knew what he was talking about. He didn't. He
> > > > doesn't have a frigging clue.
> > >
> > > I thought Edsger Dijkstra coined the "gotos are evil" bit in his
> > > structured programming push?
> >
>
> Actually Edsger Dijkstra wrote a paper entitled "Goto considered Harmful"
> He never called them evil. And the title was more for shock value to
> grab peoples attention. For the beginners Dijkstra did not
> distinguish between goto, break, return, and continue.
>
> You can find many of his papers at:
> http://www.cs.utexas.edu/users/EWD/
>

http://www.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF
to be exact. Reading it you can tell exactly how much of an
mathematician Dijkstra really was. At these times It's best to keep in
mind a quote:

"I used to understand the Theory of Relativity, but then the
mathematicians got hold of it."
-- Albert Einstein

> What he was after was simple and maintainable code, and from
> everything I have read, I think he would have no major problems with
> the Linux kernel code.
>

Well, in the formentioned paper he made the case that while - do, and
do-while are superfluous; we should all use recursion instead.
Out of respect of the dead, I'm not going to say what I think of that.

Since C don't allow goto beyond it's function, most of what was
problematic of goto's aren't legal anyway.
But I submit that those that think goto's are evil never had to deal
with smp locks.

Most goto's I've seen deal with error handling, and I guess much could
be "solved/hidden" with a "on_return { };" clause, but...
An on_return would be a simplified try/trow/catch that is limited to the
function.
IMHO: a trow that is catch'ed somewhere up the call stack, is just as
much an evil as goto.

> > So
> > these days I can only rant about Niklaus Wirth, who took the "structured
> > programming" thing and enforced it in his languages (Pascal and Modula-2),
> > and thus forced his evil on untold generations of poor CS students who had
> > to learn langauges that weren't actually useful for real work.
>
> Now that is something worth criticizing!
>

Considering that students are bottled on Java these day, they're still
learning a language unusable for real work.

> >
> > (Yeah, yeah, most _practical_ versions of Pascal ended up having all the
> > stuff necessary to break structure, but as you may be able to tell, I was
> > one of the unwashed masses who had to write in "standard Pascal" in my
> > youth. I'm scarred for life).
>
>
> Well you seem to be coping well. Just a little Pascal phobia there...
>
> Eric
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majo...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
_________________________________________________________________________

Terje Eggestad mailto:terje.e...@scali.no
Scali Scalable Linux Systems http://www.scali.com

Olaf Helsets Vei 6 tel: +47 22 62 89 61 (OFFICE)
P.O.Box 150, Oppsal +47 975 31 574 (MOBILE)
N-0619 Oslo fax: +47 22 62 89 51
NORWAY
_________________________________________________________________________

Richard B. Johnson

unread,
Jan 13, 2003, 10:23:39 AM1/13/03
to
On Sun, 12 Jan 2003, Rob Wilkens wrote:

> Linus,
>
> I'm REALLY opposed to the use of the word "goto" in any code where it's
> not needed. OF course, I'm a linux kernel newbie, so I'm in no position
> to comment
>
I'm sure Linus can hold his own on this one, but this has become
a FAQ or FSB (Frequently stated bitch).


Programming 101:
All known microprocessors execute instructions called
"op-codes". The logic of program-flow is implemented
by testing condition "flags" and jumping somewhere else,
based upon the condition. These jumps are "GOTOs".

To implement logic, you have three, and only three choices.
You can jump to some code that's earlier in the instruction
stream, you can jump to some code that's later in the
instruction-stream, or you can just keep executing the
current instruction stream.

Assemblers generate op-codes that programmers program.
They are not allowed to change anything a programmer inputs.
For all practical purposes, they simply change programmer-
defined names to processor-specific op-codes.

Compilers have more latitude. Compilers, including 'C' compilers
attempt to convert logic that a programmer defines to op-codes.
Since a compiler needs to generate code that will give the
correct logic, regardless of the previous logic or the following
logic, it is constrained in its code generation capabilities in
a manner that an assembly-language programmer would not necessarily
be. However, an astute programmer can, using the tools provided
by the compiler, including 'goto', cause code-generation in which
the usual case allows the processor to continue executing the
current execution-stream without jumping.

When the processor is forced to jump on a condition, this usually
costs CPU cycles. An astute kernel programmer may spend hours
mucking with a single piece of code trying to make it execute as
fast as possible. When a new-be comes along and complains about
the methods used, he is going to get his feet wet real quickly!

Here is an example of how some junior instructors in college
teach...
if(...){
{
if(...){
{
if(...){
{

They just don't get it. When you get to something that's not
true you can't readily "get out". Further, the normal program-
flow ends up with all those "ifs". The above code should be:

if(!...) goto get_out;
if(!...) goto get_out;
if(!...) goto get_out;
{do something()...}
get_out:

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

Terje Eggestad

unread,
Jan 13, 2003, 10:47:48 AM1/13/03
to
1) newbies has made the very same "minor" suggestion before; and had
their heads summarily chopped of.
2) It's not a minor suggestion.
3) and more to the point, you proved your selv why it's a dumb idea to
remove them, when you had to correct you own code 2 minutes after you
posted it.

-------------------- begin quote ---------------------------------
On Sun, 2003-01-12 at 14:34, Rob Wilkens wrote:
> I would change it to something like the following (without testing the
> code through a compiler or anything to see if it's valid):
>
> if (!(spin_trylock(&tty_lock.lock))){
> if (tsk ==tty_lock.lock_owner){
> WRAN_ON(!tty_lock.lcok_count);
> tty_lock.lock_count++;
> return flags;
> }
> }
oops - Yes, I forgot to add one line here (my point remains the same:
spin_lock(&tty_lock.lock);
> WARN_ON(tty_lock.lock_owner);
> <etc...>
>
-------------------- end quote ---------------------------------

OF all the reasons to use goto, spinlocks are alone reason enough. By
omitting the very little line you did
you created a race condition. Now I'm assuming that you know what a race
condition is.
IF YOU DON'T: DO NOT ASK! L-O-O-K I-T U-P !!!!

Now since races are the worst kind of problems, in that they causes
seemingly random data corruption, or of you forget a spinunlock you get
a hung machine, minimize them is of paramount importance.

Considering that doing kernel development is hard enough, new
development is almost always done on uni processors kernels that do only
one thing at the time. Then when you base logic is OK, you move to a
SMP, which means (adding and) debugging you spin locks.

Considering that fucking up spin locks are prone to corrupting your
machine, one very simple trick to makeing fewer mistakes to to have one,
and only one, unlock for every lock.

In order to achive that you write your function to have one entry point
(which all C functions have) and one exit point, thus one and only one
return.

In order to do that you can either concoct a set of if-then-else-orelse
logic that is incomprehensible, or you use goto!


Now you may look at Linus's code you buggified, and point out that the
lock and unlock are not in the same function, then read Linus's original
post CAREFULLY! And UNDERSTAND how these functions are used.


On søn, 2003-01-12 at 21:03, Rob Wilkens wrote:
> On Sun, 2003-01-12 at 14:53, Tomas Szepe wrote:
> > > [ro...@optonline.net]
> > >
> > > Am I wrong that the above would do the same thing without generating the
> > > sphagetti code that a goto would give you. Gotos are BAD, very very
> > > bad.
> >
> > Whom do I pay to have this annoying clueless asshole shot?
> > OH MY GOD, I really can't take any more.
>
> Mail filters are a wonderful thing, use them on me and you want have to
> have me shot. We'll both be happier in the end.
>
> What exactly is it that you can't take? I made a minor suggestion in
> the above message, very minor, and only a suggestion -- one that was
> free to be ignorred. There are over 300 messages a day on this list,
> why do the few ones from _me_ bother you.
>
> -Rob


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

--
_________________________________________________________________________

Terje Eggestad mailto:terje.e...@scali.no
Scali Scalable Linux Systems http://www.scali.com

Olaf Helsets Vei 6 tel: +47 22 62 89 61 (OFFICE)
P.O.Box 150, Oppsal +47 975 31 574 (MOBILE)
N-0619 Oslo fax: +47 22 62 89 51
NORWAY
_________________________________________________________________________

-

Terje Eggestad

unread,
Jan 13, 2003, 11:06:45 AM1/13/03
to
On man, 2003-01-13 at 01:08, Rob Wilkens wrote:
> On Sun, 2003-01-12 at 18:57, Emiliano Gabrielli wrote:
> > Ok, my apologises... what I would say is "why don't you sit for a moment and think about
> > what a lot of skilled coder said you ???"
>
> Believe me when I say, I'm learning a lot from these discussions. Hey,
I'm sure
> that's what I'm here for. And that's what the list is about in some
> respect (discussion).
THat's NOT why we're here. People are on this list to do work, not to be
teaching assistants. That's why You're considered disrespectful.

> Call me a troll if you want, call me a cockroach
> if it makes you feel better. I don't care. But in the end, I'm saying
> what I think is right, and when I'm wrong, I'm learning the whys. If I
> still think I'm right, I'm saying so, and people explain vividly exactly
> why it is that I'm wrong, and believe it or not, I do learn. Sometimes
> I learn at a different rate than others, but that's only because I have
> a different background and a different set of learning disabilities.

Jens Axboe

unread,
Jan 13, 2003, 11:28:36 AM1/13/03
to
On Mon, Jan 13 2003, Terje Eggestad wrote:
> On man, 2003-01-13 at 16:49, Jens Axboe wrote:

> > On Mon, Jan 13 2003, Terje Eggestad wrote:
> > > Considering that doing kernel development is hard enough, new
> > > development is almost always done on uni processors kernels that do only
> > > one thing at the time. Then when you base logic is OK, you move to a
> > > SMP, which means (adding and) debugging you spin locks.
> >
> > Goto's aside, I find the above extremely bad advise. You should _always_
> > develop with smp and for smp from the very start, or you will most
> > likely not get it right later on. With preempt, this becomes even more
> > important.
>
> You should, and I do, *design* with smp in mind, and I throw in
> smplock/unlonk as I go, but I tend to make first runs on a UP.
>
> I see your point on preemt, though.
> You do first runs on SMP?

Always, if for nothing else than the benefit of a better debugging
environment.

> > > Considering that fucking up spin locks are prone to corrupting your
> > > machine, one very simple trick to makeing fewer mistakes to to have one,
> > > and only one, unlock for every lock.
> >

> > Taking a spin lock twice will hard lock the machine, however on smp you
> > will typically have the luxury of an nmi watchdog which will help you
> > solve this quickly. Double unlock will oops immediately if you run with
> > spin lock debugging (you probably should, if you are developing kernel
> > code).
>
> I have the console on a serial port, and a terminal server. With kdb,
> you can enter the kernel i kdb even when deadlocked.

Even if spinning with interrupt disabled?

--
Jens Axboe

Terje Eggestad

unread,
Jan 13, 2003, 11:28:39 AM1/13/03
to
On man, 2003-01-13 at 16:49, Jens Axboe wrote:
> On Mon, Jan 13 2003, Terje Eggestad wrote:
> > Considering that doing kernel development is hard enough, new
> > development is almost always done on uni processors kernels that do only
> > one thing at the time. Then when you base logic is OK, you move to a
> > SMP, which means (adding and) debugging you spin locks.
>
> Goto's aside, I find the above extremely bad advise. You should _always_
> develop with smp and for smp from the very start, or you will most
> likely not get it right later on. With preempt, this becomes even more
> important.

You should, and I do, *design* with smp in mind, and I throw in
smplock/unlonk as I go, but I tend to make first runs on a UP.

I see your point on preemt, though.
You do first runs on SMP?

> > Considering that fucking up spin locks are prone to corrupting your


> > machine, one very simple trick to makeing fewer mistakes to to have one,
> > and only one, unlock for every lock.
>
> Taking a spin lock twice will hard lock the machine, however on smp you
> will typically have the luxury of an nmi watchdog which will help you
> solve this quickly. Double unlock will oops immediately if you run with
> spin lock debugging (you probably should, if you are developing kernel
> code).

I have the console on a serial port, and a terminal server. With kdb,
you can enter the kernel i kdb even when deadlocked.

TJ

--
_________________________________________________________________________

Terje Eggestad mailto:terje.e...@scali.no
Scali Scalable Linux Systems http://www.scali.com

Olaf Helsets Vei 6 tel: +47 22 62 89 61 (OFFICE)
P.O.Box 150, Oppsal +47 975 31 574 (MOBILE)
N-0619 Oslo fax: +47 22 62 89 51
NORWAY
_________________________________________________________________________

-

Terje Eggestad

unread,
Jan 13, 2003, 11:53:43 AM1/13/03
to
On man, 2003-01-13 at 17:26, Jens Axboe wrote:
> On Mon, Jan 13 2003, Terje Eggestad wrote:
> > On man, 2003-01-13 at 16:49, Jens Axboe wrote:
> > > On Mon, Jan 13 2003, Terje Eggestad wrote:
> > > > Considering that doing kernel development is hard enough, new
> > > > development is almost always done on uni processors kernels that do only
> > > > one thing at the time. Then when you base logic is OK, you move to a
> > > > SMP, which means (adding and) debugging you spin locks.
> > >
> > > Goto's aside, I find the above extremely bad advise. You should _always_
> > > develop with smp and for smp from the very start, or you will most
> > > likely not get it right later on. With preempt, this becomes even more
> > > important.
> >
> > You should, and I do, *design* with smp in mind, and I throw in
> > smplock/unlonk as I go, but I tend to make first runs on a UP.
> >
> > I see your point on preemt, though.
> > You do first runs on SMP?
>
> Always, if for nothing else than the benefit of a better debugging
> environment.
>
> > > > Considering that fucking up spin locks are prone to corrupting your
> > > > machine, one very simple trick to makeing fewer mistakes to to have one,
> > > > and only one, unlock for every lock.
> > >
> > > Taking a spin lock twice will hard lock the machine, however on smp you
> > > will typically have the luxury of an nmi watchdog which will help you
> > > solve this quickly. Double unlock will oops immediately if you run with
> > > spin lock debugging (you probably should, if you are developing kernel
> > > code).
> >
> > I have the console on a serial port, and a terminal server. With kdb,
> > you can enter the kernel i kdb even when deadlocked.
>
> Even if spinning with interrupt disabled?

Haven't painted myself into that corner yet. Doubt it, very much.

Linus Torvalds

unread,
Jan 13, 2003, 1:08:11 PM1/13/03
to

On 13 Jan 2003, Alan Cox wrote:
>
> Lots of serial activity (standard PC serial ports) with carrier drops
> present and random oopses appear.

Well, that would kind of match the locking place - the hangup might well
race with IO activity on another CPU. That's exactly the sequence that was
protected by the global irq lock.

> I tried duplicating it with pty/tty traffic on a dual PPro200 and
> suprisingly that did the same.

Just regular IO on its own shouldn't trigger this, I _think_ (no hangup
event).

Although I can actually imagine the "flush_to_ldisc()" racing with itself
on a pty (with the master flushing the slave as the slave flushes itself),
so maybe it could actually happen.. But if your pty tests closed the
master and forced hangups that way, the race would be more likely.

Do you still have the pty stress-test program?

> Ages ago I chased serial bugs down by doing data transfers between two
> PC's while one of them was strobing the carrier up and down on the test
> PC with varying frequencies

Yeah, that hangup path is one of the nastier tty events, and it's also oen
that doesn't get much testing in many "normal" loads.

Linus

Benjamin Herrenschmidt

unread,
Jan 13, 2003, 1:34:53 PM1/13/03
to
On Mon, 2003-01-13 at 15:59, Alan Cox wrote:

> On Mon, 2003-01-13 at 03:33, Paul Mackerras wrote:
> > I'm using the patch below, which makes sure that ide_release (in
> > ide-cs.c) runs in process context not interrupt context. The specific
> > problem I saw was that ide_unregister calls various devfs routines
> > that don't like being called in interrupt context, but there may well
> > be other thing ide_unregister does that aren't safe in interrupt
> > context.
>
> The ide release code isnt safe in any context.

Yup, though Paul's patch is a first step in the right way as I don't
think anybody sane plan to fix the IDE release code to make it interrupt
safe ;) At least I don't...



> > I have "fixed" the problem for now by adding a call to
> > init_hwif_data(index) in ide_register_hw just before the first
> > memcpy. I'd be interested to know what the right fix is. :)
>

> The code is currently written on the basis that people don't mangle
> free interfaces (the free up code restores stuff which I grant is
> kind of ass backwards). It also needs serious review and 2.5 testing
> before I'd want to move it to the right spot.

The code is indeed strangely backward, I had to deal with that in 2.4
for the PowerBook hotswap CD bay and will soon have to review that for
2.4.21-pre & 2.5.

> Also note that freeing the IDE can fail. If it fails then the code
> should probably be a lot smarter. Right now it 'loses' the interface.
> Really it should set a timer and try again. It might also want to
> add a null iops (out does nothing in returns FFFFFFFF) to stop
> further I/O cycles.

Yup, this have been a problem for me too, as ide_unregister for example
fails with a mounted fs. So the user had effectively removed the drive
from the bay, but I couldn't free the interface... nasty. Especially if
the user then plugs some different IDE device in the bay, the kernel
will get completely confused.

Andrew McGregor

unread,
Jan 13, 2003, 2:36:37 PM1/13/03
to
I believe I may have seen it on 2.5.53 just surfing on an Alcatel
Speedtouch DSL modem (using PPPoATM) while compiling in a background
gnome-terminal (and probably running a few other things, maybe including
top, in terminals). I'll try to confirm that.

Andrew

--On Monday, January 13, 2003 18:33:31 +0000 Alan Cox
<al...@lxorguk.ukuu.org.uk> wrote:

> On Mon, 2003-01-13 at 16:35, Linus Torvalds wrote:


>> On Sun, 12 Jan 2003, Greg KH wrote:
>> >
>> > Anyway, here's a patch with your new lock, if you want to apply it.
>>

>> I'd like to have some verification (or some test-suite) to see whether
>> it makes any difference at all before applying it.
>>
>> Alan, what's your load?


>
> Lots of serial activity (standard PC serial ports) with carrier drops

> present and random oopses appear. I've seen ppp oopses too but don't know
> if they are related. I tried duplicating it with pty/tty traffic on a dual


> PPro200 and suprisingly that did the same.
>

> Ages ago I chased serial bugs down by doing data transfers between two
> PC's while one of them was strobing the carrier up and down on the test
> PC with varying frequencies
>

> I've not had time to try that paticular abuse again alas.
>
> Alan

Valdis.K...@vt.edu

unread,
Jan 13, 2003, 6:38:55 PM1/13/03
to
On Mon, 13 Jan 2003 17:49:12 EST, Zwane Mwaikambo said:
> On Mon, 13 Jan 2003, Jens Axboe wrote:
>
> > > It uses NMI's to break into the debugger, so it would also work with
> > > interrupts disabled and spinning on a lock, the same is also true for
> > > kgdb.
> >
> > But still requiring up-apic, or smp with apic, right?
>
> Well UP with Local APIC will suffice. So that works on a lot of i686
> machines.

This mean those of us that have a 'local_apic_kills_bios' flag in dmi_scan.c
are still out of luck, correct?

Bob Taylor

unread,
Jan 13, 2003, 7:05:38 PM1/13/03
to
In message <m14r8dg...@frodo.biederman.org>, Eric W.
Biederman writes:

[snip]

> Though standard Pascal was not unusable because of the lack of a goto,
> that was the one feature it actually had. But I suspect a lot of
> teachers failed to mention it.

If my memory serves me, Niklaus Wirth invented Pascal as a
teaching language not meant for production work. Modula, I
think, was meant for production work. I don't care for either
also.

[snip]

Bob

--
+---------------------------------------------------------------+
| Bob Taylor Email: brta...@sanfelipe.com.mx |
|---------------------------------------------------------------|
| Like the ad says, at 300 dpi you can tell she's wearing a |
| swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you |
| can tell it's painted on. I suppose at 2400 dpi you can tell |
| if the paint is giving her a rash. (So says Joshua R. Poulson)|
+---------------------------------------------------------------+

James H. Cloos Jr.

unread,
Jan 13, 2003, 8:45:12 PM1/13/03
to
[re OSS vs ALSA audio drivers]

Linus> So I don't see a huge reason to remove them from the sources,
Linus> but we might well make them harder to select by mistake, for
Linus> example. Right now the config help files aren't exactly helpful,
Linus> and the OSS choice is before the ALSA one, which looks wrong.

Linus> They should probably be marked deprecated, and if they don't
Linus> get a lot of maintenance, that's fine.

Like this?

Also available from:

bk://cloos.bkbits.net/alsa-oss

Chan...@1.966, 2003-01-13 20:05:11-05:00, cl...@lugabout.jhcloos.org
Move ALSA before OSS

sound/Kconfig | 26 ++++++++++++--------------
arch/i386/defconfig | 10 +++++-----
arch/ia64/defconfig | 10 +++++-----
arch/parisc/defconfig | 8 ++++----
arch/sparc64/defconfig | 10 +++++-----
5 files changed, 31 insertions(+), 33 deletions(-)


diff -Nru a/sound/Kconfig b/sound/Kconfig
--- a/sound/Kconfig Mon Jan 13 20:32:29 2003
+++ b/sound/Kconfig Mon Jan 13 20:32:29 2003
@@ -1,20 +1,6 @@
# sound/Config.in
#

-menu "Open Sound System"
- depends on SOUND!=n
-
-config SOUND_PRIME
- tristate "Open Sound System"
- depends on SOUND
- help
- Say 'Y' or 'M' to enable Open Sound System drivers.
-
-source "sound/oss/Kconfig"
-
-endmenu
-
-
menu "Advanced Linux Sound Architecture"
depends on SOUND!=n

@@ -42,3 +28,15 @@

endmenu

+menu "Open Sound System"
+ depends on SOUND!=n
+
+config SOUND_PRIME
+ tristate "Open Sound System (DEPRECATED)"
+ depends on SOUND
+ help
+ Say 'Y' or 'M' to enable Open Sound System drivers.
+
+source "sound/oss/Kconfig"
+
+endmenu
diff -Nru a/arch/i386/defconfig b/arch/i386/defconfig
--- a/arch/i386/defconfig Mon Jan 13 20:32:29 2003
+++ b/arch/i386/defconfig Mon Jan 13 20:32:29 2003
@@ -839,11 +839,6 @@
CONFIG_SOUND=y

#
-# Open Sound System
-#
-# CONFIG_SOUND_PRIME is not set
-
-#
# Advanced Linux Sound Architecture
#
CONFIG_SND=y
@@ -922,6 +917,11 @@
# ALSA USB devices
#
# CONFIG_SND_USB_AUDIO is not set
+
+#
+# Open Sound System
+#
+# CONFIG_SOUND_PRIME is not set

#
# USB support
diff -Nru a/arch/ia64/defconfig b/arch/ia64/defconfig
--- a/arch/ia64/defconfig Mon Jan 13 20:32:29 2003
+++ b/arch/ia64/defconfig Mon Jan 13 20:32:29 2003
@@ -657,6 +657,11 @@
CONFIG_SOUND=y

#
+# Advanced Linux Sound Architecture
+#
+# CONFIG_SND is not set
+
+#
# Open Sound System
#
CONFIG_SOUND_PRIME=y
@@ -679,11 +684,6 @@
# CONFIG_SOUND_VIA82CXXX is not set
# CONFIG_SOUND_OSS is not set
# CONFIG_SOUND_TVMIXER is not set
-
-#
-# Advanced Linux Sound Architecture
-#
-# CONFIG_SND is not set

#
# USB support
diff -Nru a/arch/parisc/defconfig b/arch/parisc/defconfig
--- a/arch/parisc/defconfig Mon Jan 13 20:32:29 2003
+++ b/arch/parisc/defconfig Mon Jan 13 20:32:29 2003
@@ -643,14 +643,14 @@
CONFIG_SOUND=y

#
-# Open Sound System
+# Advanced Linux Sound Architecture
#
-# CONFIG_SOUND_PRIME is not set
+# CONFIG_SND is not set

#
-# Advanced Linux Sound Architecture
+# Open Sound System
#
-# CONFIG_SND is not set
+# CONFIG_SOUND_PRIME is not set

#
# USB support
diff -Nru a/arch/sparc64/defconfig b/arch/sparc64/defconfig
--- a/arch/sparc64/defconfig Mon Jan 13 20:32:29 2003
+++ b/arch/sparc64/defconfig Mon Jan 13 20:32:29 2003
@@ -710,11 +710,6 @@
CONFIG_SOUND=m

#
-# Open Sound System
-#
-# CONFIG_SOUND_PRIME is not set
-
-#
# Advanced Linux Sound Architecture
#
CONFIG_SND=m
@@ -777,6 +772,11 @@
#
CONFIG_SND_SUN_AMD7930=m
CONFIG_SND_SUN_CS4231=m
+
+#
+# Open Sound System
+#
+# CONFIG_SOUND_PRIME is not set

#
# USB support

Brian Tinsley

unread,
Jan 14, 2003, 2:14:43 AM1/14/03
to
What's happened to Rob??? He's been unusually quiet today! Too many
tranquilizers? He must have passed out face down on his keyboard and
the drool shorted it out ;)

Bruce Harada wrote:

>On Mon, 13 Jan 2003 13:08:42 +0000
>Dave Jones <da...@codemonkey.org.uk> wrote:
>
>
>
>>On Sun, Jan 12, 2003 at 02:34:54PM -0500, Rob Wilkens wrote:
>>
>>Wow, one week later, and this would tie in with the fourth anniversary
>>of someone else[2] making an ass of himself on this issue[1]
>>
>> Dave
>>
>>[1] http://www.uwsg.iu.edu/hypermail/linux/kernel/9901.2/0939.html
>>[2] Complete with quaint 'cool' l33t handle. How 90s! *bows head in shame*
>>
>>
>
>So does that mean that we can look forward to Rob being a very productive
>leading Linux developer in four years time?
>
>Naaaah.... ;)


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

--

Brian Tinsley
Chief Systems Engineer
Emageon

Scott Robert Ladd

unread,
Jan 15, 2003, 10:10:35 AM1/15/03
to
Kai Henningsen wrote:
> Well ... he did have some nice ideas. Unfortunately, they usually don't
> get palatable until someone else has worked on them.

I actually *liked* Modula-2... but I haven't used it since the very early
90s. I guess I didn't like it *that* much, huh? ;)

> If you look into Wirth's books and see that the example code in there ...
>
> * uses one(!) space indentation
> * routinely puts several statements on one line

I've got a copy of the classic "Algorithms + Data Structures = Programs"; in
it, Wirth uses four-space indentation but a proportional font. The style
isn't all that bad. Hell, I've got worse stuff in some of *my* books --
oops, shouldn't have typed that... ;)

One-space indents may very well be an artifact of idiot copy editors, and
not the author.

> * typically uses one- or two-character variable names

There is one instance when one/two-character variable names make sense:
mathematical code that directly implements numericla algorithms from the
text. In such a case, short variable names correspond directly to standard
notation; using longer names would actually obscure the correspondence
between text and code. I also don't see the point of using "array_index"
over a plain old traditional "i" in a loop.

Rarely is any coding "rule" absolute. The point is clarity; if a "goto" or
one-character identifier make sense, use'em.

For those who growl -- I think this kind of discussion *has* value in the
kernel mailing list. Kernel newbies and such can learn a great deal from
rational, calm debates among experts; if they learn, their contributions to
the kernel will be better.

Of course, note the "rational" and "calm" above, which does not apply to the
Stallman debate... ;)

I enjoy the implementation debates; they give me a better idea of where the
kernel is going, so I can figure out where to stick my oar in the waters.

--
Scott Robert Ladd
Coyote Gulch Productions (http://www.coyotegulch.com)
Professional programming for science and engineering;
Interesting and unusual bits of very free code.

Marcelo Pacheco

unread,
Jan 15, 2003, 10:22:14 AM1/15/03
to
>
>
>For those who growl -- I think this kind of discussion *has* value in the
>kernel mailing list. Kernel newbies and such can learn a great deal from
>rational, calm debates among experts; if they learn, their contributions to
>the kernel will be better.
>
>Of course, note the "rational" and "calm" above, which does not apply to the
>Stallman debate... ;)
>
>I enjoy the implementation debates; they give me a better idea of where the
>kernel is going, so I can figure out where to stick my oar in the waters.
>
Yes, but how many times this discussion already happened ?
I have nothing against the discussion happening ... As long as it
doesn't happen a couple of times a year. That's what a list archive is
for. The latest generation of Internet users, including some latest
generation developers are in general LAZY about looking up archives,
they want to post the question now and get an answer, while they could
have gotten the answer without bothering anyone. Sure, it takes one a
little more effort to search the archives, but that's one person's
effort, while posting one message wastes hundreds (perhaps thousands) of
people's time.
And also, the FAQ for the list states it quite clearly that the people
doing usefull work here is only really interested in proof that
something is better, performance and/or space wise, so if someone has an
opinion, first go prove it on some piece of the kernel, the post their
opinions along with the results to the list. That's the way to go.
Maintenability is important, after speed, memory usage and portability.
That's sound engineering, instead of C.S. dogmas.

Marcelo Pacheco

Kevin Puetz

unread,
Jan 15, 2003, 10:46:51 PM1/15/03
to
Valdis.K...@vt.edu wrote:

> On Sun, 12 Jan 2003 14:59:57 EST, Rob Wilkens said:
>
>> In general, if you can structure your code properly, you should never
>> need a goto, and if you don't need a goto you shouldn't use it. It's
>> just "common sense" as I've always been taught. Unless you're
>> intentionally trying to write code that's harder for others to read.
>
> Now, it's provable you never *NEED* a goto. On the other hand,
> *judicious* use of goto can prevent code that is so cluttered with stuff
> of the form:
>
> if(...) {
> ...
> die_flag = 1;
> if (!die _flag) {...
>
> Pretty soon, you have die_1_flag, die_2_flag, die_3_flag and so on,
> rather than 3 or 4 "goto bail_now;".
>
> The real problem is that C doesn't have a good multi-level "break"
> construct. On the other hand, I don't know of any language that has a good
> one - some allow "break 3;" to break 3 levels- but that's still bad
> because you get screwed if somebody adds an 'if' clause....

perl has a pretty nice one... you can label each construct to which break is
applicable (ie, loop or whatever) then say "break foo" to escape that
struture (and any others you happen to be inside.

Folkert van Heusden

unread,
Jan 18, 2003, 8:10:48 AM1/18/03
to
> Now, it's provable you never *NEED* a goto. On the other hand,
*judicious*
> use of goto can prevent code that is so cluttered with stuff of the form:
> if(...) {
> ...
> die_flag = 1;
> if (!die _flag) {...
>
> Pretty soon, you have die_1_flag, die_2_flag, die_3_flag and so on,
> rather than 3 or 4 "goto bail_now;".

There's always the construction:

for(;;)
{
/* do something */

if (something_failed)
break;

/* do something */

if (something_failed)
break;

...

break; /* the final break */
}

etc.

0 new messages