USB changes for 2.5.34

This includes the previous USB changes I sent out for 2.5.33 but never made it into your tree.

Pull from: bk://linuxusb.bkbits.net/linus-2.5

thanks,

greg k-h

 drivers/usb/core/hcd.c             |   21 +-
 drivers/usb/core/inode.c           |   43 +++--
 drivers/usb/host/ehci-dbg.c        |  141 +++++++++-------
 drivers/usb/host/ehci-hcd.c        |  117 +++++++++----
 drivers/usb/host/ehci-hub.c        |   13 +
 drivers/usb/host/ehci-q.c          |  107 +++++-------
 drivers/usb/host/ehci-sched.c      |   32 +--
 drivers/usb/host/ehci.h            |   24 ++
 drivers/usb/host/ohci-dbg.c        |  256 ++++++++++++++++++++++++++----
 drivers/usb/host/ohci-hcd.c        |   79 ++++-----
 drivers/usb/host/ohci-mem.c        |   27 ---
 drivers/usb/host/ohci-q.c          |  314 +++++++++++++++++++------------------
 drivers/usb/host/ohci.h            |   27 +--
 drivers/usb/host/uhci-hcd.c        |  187 ++++++++++------------
 drivers/usb/host/uhci-hcd.h        |   43 ++++-
 drivers/usb/image/scanner.c        |    6 
 drivers/usb/media/se401.c          |  128 +++++++--------
 drivers/usb/media/se401.h          |    1 
 drivers/usb/net/kaweth.c           |    2 
 drivers/usb/net/pegasus.h          |    6 
 drivers/usb/net/usbnet.c           |    5 
 drivers/usb/storage/protocol.c     |   69 ++++++--
 drivers/usb/storage/transport.c    |    5 
 drivers/usb/storage/unusual_devs.h |   14 +
 drivers/usb/storage/usb.c          |    8 
 drivers/usb/storage/usb.h          |    1 
 26 files changed, 1018 insertions(+), 658 deletions(-)

-----

[email protected], 2002-09-09 14:21:49-07:00, [email protected]
[PATCH] USB: se401 driver update

 drivers/usb/media/se401.c |  128 +++++++++++++++++++++++-----------------------
 drivers/usb/media/se401.h |    1 
 2 files changed, 65 insertions(+), 64 deletions(-)

------

[email protected], 2002-09-09 13:59:18-07:00, [email protected]
[PATCH] [PATCH 2.5.33+] ohci and iso-in

I added a bug in 2.5.23 when cleaning up something that was broken ... it wasn't broken in quite the way I had thought at the time!

This fixes a problem some folk have reported recently with ISO-IN, by masking a common non-error outcome.

Please merge to Linus' tree, on top of the one patch you already have queued. Thanks to Nemosoft for such quick turnaround on testing!

 drivers/usb/host/ohci-q.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)

------

[email protected], 2002-09-09 13:58:51-07:00, [email protected]
[PATCH] [PATCH] (repost) fix for big endian machines in scanner.c

This patch fixes a problem with big endian machines and scanner drivers which use the SCANNER_IOCTL_CTRLMSG ioctl. The big endian to little endian swap was done twice, resulting in a no-op.

 drivers/usb/image/scanner.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

------

[email protected], 2002-09-09 13:58:32-07:00, [email protected]
[PATCH] USB storage: abort bug fix

Also, have you sent in the one-line fix I found for the abort bug? Andries found that it cured his BUG_ON problem. In case you didn't save a copy of it, I've included it below.

 drivers/usb/storage/transport.c |    3 +++
 1 files changed, 3 insertions(+)

------

[email protected], 2002-09-05 08:40:44-07:00, [email protected]
[PATCH] usbnet, add YOPY device IDs

A now-happy Yopy user sent me these IDs.

 drivers/usb/net/usbnet.c |    5 +++++
 1 files changed, 5 insertions(+)

------

[email protected], 2002-09-05 08:40:25-07:00, [email protected]
[PATCH] two byte offset for kaweth

this is the two byte offset patch to kaweth for 2.5 to prevent MIPS crashing and speed up other arches.

 drivers/usb/net/kaweth.c |    2 ++
 1 files changed, 2 insertions(+)

------

[email protected], 2002-09-05 08:33:20-07:00, [email protected]
USB: storage driver: replace show_trace() with BUG()

 drivers/usb/storage/transport.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

------

[email protected], 2002-09-04 17:35:36-07:00, [email protected]
[PATCH] Feiya 5-in-1 Card Reader

I have a USB 5-in-1 Card Reader, that will read CF and SM and SD/MMC. Under Linux it appears as three SCSI devices.
For today, the report is on the CF part.

The CF part works fine under ordinary usb-storage SCSI simulation, with one small problem: 8 and 32 MB cards, that are detected as having 15872 and 63488 sectors by other readers, are detected as having 15873 and 63489 sectors by this Feiya reader (0x090c / 0x1132).
In the good old days probably nobody would have noticed, but these days the partition reading code also wants to read the last sector. This results in the SCSI code taking the device off line:

[USB storage does a READ_10, which fails since the sector is past the end of the disk. Then it tries a READ_6 and nothing ever happens, probably because the device does not support READ_6. Then the error handler does an abort which triggers some bugs in scsiglue.c and transport.c, then the error handler does a device reset, then a host reset, then a bus reset, and finally the device is taken offline.]

The patch below does not address any bugs in the SCSI error code (a big improvement would be just to rip it all out - this error code never achieves anything useful but has crashed many a machine) and does not fix the USB code either.
It just adds a flag to the unusual_devices section mentioning that this device (my revision is 1.00) has this bug.

Without the patch the kernel crashes, or insmod usb-storage hangs. With the patch the CF part of the device works perfectly.

(Another change is to only print "Fixing INQUIRY data" when really something is changed, not when the data was OK already.)

Andries

 drivers/usb/storage/protocol.c     |   69 ++++++++++++++++++++++++++++---------
 drivers/usb/storage/unusual_devs.h |    7 +++
 drivers/usb/storage/usb.h          |    1 
 3 files changed, 62 insertions(+), 15 deletions(-)

------

[email protected], 2002-09-04 12:08:14-07:00, [email protected]
USB: clean up the error path in create_special_files() for usbfs

Thanks to David Brownell for pointing out the problem here.

 drivers/usb/core/inode.c |   42 ++++++++++++++++++++++++++++++++----------
 1 files changed, 32 insertions(+), 10 deletions(-)

------

[email protected], 2002-09-04 11:16:26-07:00, [email protected]
[PATCH] 2.5.32-usb

This patch appears not to be in 2.5.32, but applies cleanly.

The following patch fixes 3 problems in USB:

1. Don't pci_map buffers when we know we're not going to pass them to a device.

This was first noticed on ARM (no surprises here); the root hub code, rh_call_control(), placed data into the buffer and then called usb_hcd_giveback_urb(). This function called pci_unmap_single() on this region which promptly destroyed the data that rh_call_control() had placed there. This lead to a corrupted device descriptor and the "too many configurations" message.

2. If controller->hcca is NULL, don't try to dereference it.

3. If we free the root hub (in ohci-hcd.c or uhci-hcd.c), don't leave a dangling pointer around to trip us up in usb_disconnect(). EHCI appears to get this right.

 drivers/usb/core/hcd.c      |   21 +++++++++++----------
 drivers/usb/host/ohci-dbg.c |    3 ++-
 drivers/usb/host/ohci-hcd.c |    3 ++-
 drivers/usb/host/uhci-hcd.c |    1 +
 4 files changed, 16 insertions(+), 12 deletions(-)

------

[email protected], 2002-09-04 11:04:10-07:00, [email protected]
USB: remove __NO_VERSION__

Thanks to Rusty "trivial" Russell

 drivers/usb/core/inode.c |    1 -
 1 files changed, 1 deletion(-)

------

[email protected], 2002-09-04 09:54:02-07:00, [email protected]
[PATCH] USB: pegasus driver patch

one more adapter, changed company name and forgotten flag

 drivers/usb/net/pegasus.h |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

------

[email protected], 2002-09-04 09:21:00-07:00, [email protected]
[PATCH] ohci-hcd endpoint scheduling, driverfs

This patch cleans up some messy parts of this driver, and was pleasantly painless.

- gets rid of ED dma hashtables
* less memory needed
* also less (+faster) code
* ... rewrites all ED scheduling ops, they now use cpu addresses, like EHCI and UHCI do already

- simplifies ED scheduling (no dma hashtables)
* control and bulk lists are now doubly linked
* periodic tree still singly linked; driver uses a new CPU view "shadow" of the hardware framelist
* previous periodic code was cryptic, almost read-only
* simpler tree code for EDs with {branch,period}

- bugfixes periodic scheduling
* when CONFIG_USB_BANDWIDTH, checks per-frame load against the limit; no more dodgey accounting
* handles iso period != 1; interrupt and iso schedule EDs with the same routine (HW sees special TDs)
* credit usbfs with bandwidth for endpoints, not URBs

- adds driverfs output (when CONFIG_USB_DEBUG)
* resembles EHCI: 'async' (control+bulk) and 'periodic' (interrupt+iso) files show schedules
* shows only queue heads (EDs) just now (*)

- has minor text and code cleanups, etc

Now that this logic has morphed into more comprehensible form, I know what to borrow into the EHCI code!

(*) It shows TDs on the td_list, but this patch won't
put them there. A queue fault handling update will.

 drivers/usb/host/ohci-dbg.c |  253 +++++++++++++++++++++++++++++++-----
 drivers/usb/host/ohci-hcd.c |   67 ++++-----
 drivers/usb/host/ohci-mem.c |   27 ---
 drivers/usb/host/ohci-q.c   |  308 +++++++++++++++++++++++---------------------
 drivers/usb/host/ohci.h     |   27 +--
 5 files changed, 425 insertions(+), 257 deletions(-)

------

[email protected], 2002-09-04 09:18:15-07:00, [email protected]
[PATCH] PATCH: usb-storage: fix software eject

This patch fixes the recently broken software eject of media. At least, it should. I'm back to having compile problems again, but the fix should be pretty self-evident.

 drivers/usb/storage/usb.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletion(-)

------

[email protected], 2002-09-04 09:16:59-07:00, [email protected]
[PATCH] Re: updated ehci patch ...

* keep watchdog on shorter leash, and just do
standard irq processing when it barks. this
means I can use a somewhat iffy vt8235 mobo.

* updates to the driverfs debug output, including using S_IRUGO so anyone can gawk.

* some updates, mostly to use a new hcd_to_bus(), so this version also compiles on a (slightly patched) 2.4.20-pre5 kernel. (*)

 drivers/usb/host/ehci-dbg.c   |  141 +++++++++++++++++++++++-------------------
 drivers/usb/host/ehci-hcd.c   |   53 +++++++++------
 drivers/usb/host/ehci-hub.c   |   13 ++-
 drivers/usb/host/ehci-q.c     |    9 +-
 drivers/usb/host/ehci-sched.c |    6 -
 drivers/usb/host/ehci.h       |   22 ++++++
 6 files changed, 150 insertions(+), 94 deletions(-)

------

[email protected], 2002-09-04 09:16:29-07:00, [email protected]
[PATCH] ehci locking

I've been chasing problems on a KT333 based system, with the 8253 southbridge and EHCI 1.0 (!), and this fixes at least some of them:

- locking updates:
* a few routines weren't protected right
* less irqsave thrashing for schedule lock

- adds a watchdog timer that should fire when the STS_IAA interrupt seems to be missing.

- gives ports back to companion UHCI/OHCI on rmmod

- re-enables faulted QH only after all its completion callbacks have done their work

- removes an oops I've seen when usb-storage unlinks stuff. (it seemed confused about error handling, but that's not a reason to oops.)

- minor cleanup: deadcode rm, etc

Right now the watchdog just barks, and that mechanism might go away (or into the shared hcd code). Sometimes the issue it reports seems to clear up by itself, but sometimes not...

 drivers/usb/host/ehci-hcd.c   |   64 ++++++++++++++++++++++-----
 drivers/usb/host/ehci-q.c     |   98 +++++++++++++++++-------------------------
 drivers/usb/host/ehci-sched.c |   26 ++++-------
 drivers/usb/host/ehci.h       |    2 
 4 files changed, 105 insertions(+), 85 deletions(-)

------

[email protected], 2002-09-04 09:06:34-07:00, [email protected]
[PATCH] Lexar USB CF Reader

Two weeks ago I sent this patch to the listed USB storage maintainer ([email protected]) and have not yet heard back. The attached patch adds support for the Lexar USB CF Reader identified by id_product 0xb002, version 0x0113 (which is the version I have). This patch is against the 2.4.19 kernel, sorry if this is the wrong address to send this stuff to. Thanks.

 drivers/usb/storage/unusual_devs.h |    7 +++++++
 1 files changed, 7 insertions(+)

------

[email protected], 2002-09-04 09:06:07-07:00, [email protected]
[PATCH] pci_free_consistent on ohci initialisation failure

The trace at the end of the message shows the init failure.

 drivers/usb/host/ohci-hcd.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

------

[email protected], 2002-09-04 09:05:37-07:00, [email protected]
[PATCH] Re: [patch 2.5.31-bk5] uhci, misc

This patch has some small UHCI bugfixes

- on submit error, frees memory and (!) returns error code
- root hub should disconnect only once
- pci pool code shouldn't be given GFP_DMA
- uses del_timer_sync(), which behaves on SMP, not del_timer()

and cleanups:

- use container_of
- doesn't replicate so much hcd state
- no such status -ECONNABORTED
- uses bus_name in procfs, not "hc0", "hc1" etc

 drivers/usb/host/uhci-hcd.c |  101 ++++++++++++++++----------------------------
 drivers/usb/host/uhci-hcd.h |    8 ---
 2 files changed, 38 insertions(+), 71 deletions(-)

------

[email protected], 2002-09-04 09:05:07-07:00, [email protected]
[PATCH] uhci, doc + cleanup

Another UHCI patch. I'm sending this since Dan said he was going to start teaching "uhci-hcd" how to do control and interrupt queueing, and this may help. Granted it checks out (I didn't test the part that has a chance to break, though it "looks right"), I think it should get merged in at some point. What it does:

- updates and adds some comments/docs
- gets rid of a "magic number" calling convention, instead passing an explicit flag UHCI_PTR_DEPTH or UHCI_PTR_BREADTH (self-doc :)
- deletes bits of unused/dead code
- updates the append-to-qh code:
* start using list_for_each() ... clearer than handcrafted loops, and it prefetches too. Lots of places should get updated to do this, IMO.
* re-orders some stuff to fix a sequencing problem
* adds ascii-art to show how the urb queueing is done (based on some email Johannes sent me recently)

That sequencing problem is that when splicing a QH between A and B, it currently splices A-->QH before QH-->B ... so that if the HC is looking at that chunk of schedule at that time, everything starting at B will be ignored during the rest of that frame. (Since the QH is initted to have UHCI_PTR_TERM next, stopping the schedule scan.)

I said "problem" not "bug" since in the current code it would probably (what does that "PIIX bug" do??) just reduce control/bulk throughput. That's because the logic is only appending towards the end of each frame's schedule, where the FSBR loopback kicks in.

 drivers/usb/host/uhci-hcd.c |   85 ++++++++++++++++++++++++--------------------
 drivers/usb/host/uhci-hcd.h |   35 +++++++++++++++++-
 2 files changed, 82 insertions(+), 38 deletions(-)

------

Greg, please don't do this

that BUG() thing is _way_ out of line, and has killed a few of my machines several times for no good reason. It actively hurts debuggability, because the machine is totally dead after it, and the whole and ONLY point of BUG() messages is to help debugging and make it clear that we can't handle something.

In this case, we _can_ handle it, and we're much better off with a machine that works and that you can look up the messages with than killing it.

Rule of thumb: BUG() is only good for something that never happens and that we really have no other option for (ie state is so corrupt that continuing is deadly).

Linus

Sorry, Matt told me to add it, I didn't realize the background. Should I leave it as show_trace(), or just remove it? Do you want me to send you another changeset to put it back?

thanks,

greg k-h

I've just excluded it, with a comment to never _ever_ kill the machine unless there is a major reason for it.

We might want to add some "weaker" form of BUG_ON() for sanity checks that aren't life-threatening (ie a "CHECK(a == b)" kind of debug facility) that would be prettier than doing printk's and show_trace(), and that would also be easier to disable for production kernels (not that I personally much believe in disabling debugging like that - if it really isn't needed it should be removed, not disabled).

Linus

I'd have thought you may well want the reverse. If the user didnt pick the kernel debugging, don't die on software check option you want to blow up. If they are debugging or its < 2.6.0-rc1 you want it to show the stack and keep going

No, thanks. If it's fatal, it's a BUG(), and you blow regardless.

If it isn't fatal, there is no excuse for blowing up. EVER. That's _especially_ true for some random user who didn't ask for, and can't handle debugging. If it's useful information that the developer believes he wants, it shouldn't be conditional at all.

You definitely want to keep going regardless. A BUG() that takes out the machine is just not useful, because users who aren't ready to debug it can't even make any reports except "it stops" (which this one did if you were under X - the machine was just _dead_).

Basically, with the amount of locking we have, a BUG() or a blow-up just about anywhere is lethal. Most sequences (especially in drivers, but inside filesystems etc too) tend to hold spinlocks etc that just makes it a bad idea to BUG() out unless you really really have to, since the machine is not likely to survive and be able to write good reports to disk etc at pretty much any point.

(It used to be that you could take a fault just about anywhere except for in interrupt handlers, and Linux would try its damndest to clean up and continue as if nothing had happened. Those days are sadly gone, and trapping and depending on killing the process seldom works well any more).

On the whole, it's a lot better to just print out a message (and call traces are often very useful) and continue. That's not always possible, of course, and a lot of BUG() and BUG_ON() cases are perfectly valid simply because sometimes there isn't anything you can do except kill the machine and try to inform the user.

I think the historical kernel behaviour ("trap and kill and continue" historically worked so fine for _both_ major bugs and for "random sanity test" cases) has caused us to be a bit lazy about this sometimes.

Linus

Note that from an implementation standpoint I suspect that a "trap and continue" thing can easily be pretty much exactly as the current BUG() with a flag somewhere, say in the "third byte" of the "ud2" instruction.

That would also make it easy to dynamically change the behaviour (ie some people might want to explicitly make even the "warnings" fatal - a kernel version of -Werror), and the implementation should be trivial:

#define TRAP_INSTRUCTION( lethal )	\
	__asm__ __volatile__( "ud2\n"	\
			"\t.byte %0\n"	\
			"\t.word %c1\n" \
			"\t.long %c2\n" \
			: :"i" (lethal), "i" (__LINE__), "i" (__FILE__))

and then you have

	#define BUG()	TRAP_INSTRUCTION(1)
	#define WARN()	TRAP_INSTRUCTION(0)

or something like that (where the non-lethal version just increments eip by 9 to jump over the extended ud2 and the information pointers).

Linus

There were a few BUG_ON calls in scsi_glue.c and in transport.c that killed machines. For transport.c my source has

        if (len != srb->request_bufflen) {
                printk("USB transport.c: len=%d srb->request_bufflen=%d\n",
                       len, srb->request_bufflen);
#if 0
                show_trace(NULL);
#endif
        }

(show_trace was not defined here; I think the call should be deleted).

With some luck Alan's fix means that the ones in usb_stor_abort_transport() and [us_]release() are not triggered anymore.

Andries

show_trace isn't exported for modules, and (even worse) isn't even implemented on all architectures, IIRC.
- Nicholas

That's right.

Create an arch-independent `dump_stack()' function. So we don't need to do

#ifdef CONFIG_X86
	show_stack(0);		/* No prototype in scope! */

#endif

any more.

The whole dump_stack() implementation is delegated to the architecture. If it doesn't provide one, there is a default do-nothing library function.

 arch/alpha/kernel/traps.c |    5 +++++
 arch/cris/kernel/traps.c  |    6 +++++-
 arch/i386/kernel/traps.c  |    8 ++++++++
 fs/buffer.c               |    4 +---
 include/linux/kernel.h    |    2 ++
 kernel/ksyms.c            |    3 +++
 lib/Makefile              |    2 +-
 lib/dump_stack.c          |   13 +++++++++++++
 8 files changed, 38 insertions(+), 5 deletions(-)

@@ -171,6 +171,11 @@ void show_stack(unsigned long *sp)
 	dik_show_trace(sp);
 }
 
+void dump_stack(void)
+{
+	show_stack(NULL);
+}
+
 void
 die_if_kernel(char * str, struct pt_regs *regs, long err, unsigned long *r9_15)
 {

@@ -230,8 +230,12 @@ watchdog_bite_hook(struct pt_regs *regs)
 #endif	
 }
 
-/* This is normally the 'Oops' routine */
+void dump_stack(void)
+{
+	show_stack(NULL);
+}
 
+/* This is normally the 'Oops' routine */
 void 
 die_if_kernel(const char * str, struct pt_regs * regs, long err)
 {

@@ -207,6 +207,14 @@ void show_stack(unsigned long * esp)
 	show_trace(esp);
 }
 
+/*
+ * The architecture-independent backtrace generator
+ */
+void dump_stack(void)
+{
+	show_stack(0);
+}
+
 void show_registers(struct pt_regs *regs)
 {
 	int i;

@@ -61,10 +61,8 @@ void __buffer_error(char *file, int line
 		return;
 	enough++;
 	printk("buffer layer error at %s:%d\n", file, line);
-#ifdef CONFIG_X86
 	printk("Pass this trace through ksymoops for reporting\n");
-	show_stack(0);
-#endif
+	dump_stack();
 }
 EXPORT_SYMBOL(__buffer_error);
 

@@ -96,6 +96,8 @@ extern const char *print_tainted(void);
 #define TAINT_FORCED_MODULE		(1<<1)
 #define TAINT_UNSAFE_SMP		(1<<2)
 
+extern void dump_stack(void);
+
 #if DEBUG
 #define pr_debug(fmt,arg...) \
 	printk(KERN_DEBUG fmt,##arg)

@@ -609,3 +609,6 @@ EXPORT_SYMBOL(pidhash);
 #if defined(CONFIG_SMP) && defined(__GENERIC_PER_CPU)
 EXPORT_SYMBOL(__per_cpu_offset);
 #endif
+
+/* debug */
+EXPORT_SYMBOL(dump_stack);

@@ -0,0 +1,13 @@
+/*
+ * Provide a default dump_stack() function for architectures
+ * which don't implement their own.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+void dump_stack(void)
+{
+	printk(KERN_NOTICE
+		"This architecture does not implement dump_stack()\n");
+}

@@ -12,7 +12,7 @@ export-objs := cmdline.o dec_and_lock.o 
 	       crc32.o rbtree.o radix-tree.o
 
 obj-y := errno.o ctype.o string.o vsprintf.o brlock.o cmdline.o \
-	 bust_spinlocks.o rbtree.o radix-tree.o
+	 bust_spinlocks.o rbtree.o radix-tree.o dump_stack.o
 
 obj-$(CONFIG_RWSEM_GENERIC_SPINLOCK) += rwsem-spinlock.o
 obj-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o

.

Is there a reason for not calling it "backtrace()" ?

Daniel

In retrospect, no. But it's called dump_stack() in 2.4 now.

-

So? If it is a problem for people, fix it. Or remove the damn call. It _still_ isn't valid to kill a machine for a non-fatal error.

We're not Windows. We don't take GFP's for random reasons. I'm not interested in what some people call "hardening", but I _am_ interested in a system that is rock solid and works even when it doesn't necessarily expect to.

Linus

Linus, normally I would agree with you on this, but...

(1) We can't always handle it. The current code implements a heuristic, which is not always correct. When it gets it wrong....

(2) show_trace() was proving ineffective in bringing the bad commands to light so they might be fixed

(3) Given that we think we've fixed all of these bad initiators, the BUG() really is something that should never happen.

Matt

Matthew Dharm                              Home: [email protected] 
Maintainer, Linux USB Mass Storage Driver

I need a computer?
					-- Customer
User Friendly, 2/19/1998

Short and sweet:

If I have a BUG() that I need to remove and replace with a printk() (which I did) in order to even be able to debug the dang thing, then that BUG() is a mistake. No ifs, buts or maybe's accepted. The one and _only_ point of BUG() is to help debugging, and if it doesn't, then it should not be there.

Longer:

You can't have it both ways.

Either it really never happens.

Or it _does_ happen, in which case you want sane debuggable output.

In the latter case, BUG() is the wrong thing to do, exactly because it tends to bring the system down. In the first case it obviously doesn't matter and isn't even relevant, and the best thing to do would be to remove it altogether. In neither case is BUG() useful.

I'm personally in X 99% of the time except for the reasonably rare case when I'm chasing down some bug I know I can reproduce and I want the kernel to have access to the console.

And I doubt I'm alone in that. I suspect most people who use Linux in any interesting situation (and no, I don't think servers are very interesting from most standpoints) tend to do this. Agreed?

That means that the BUG() output won't even be _visible_ to 99% of all cases. And if it causes the machine to hang, it is now not only invisible, it cannot even be gotten hold of some other way.

Which basically means that a driver that uses BUG() for something that isn't certain to be fatal anyway is a BUGGY driver.

If my X session is hung, that's already bad (most people will give up at that point). If I can't even log in remotely, that's _disastrous_, since now the BUG() causes me to not to be able to debug it at all. The BUG() basically made itself irrlevant.

The fact is, BUG() is almost always the wrong thing to do. And it's almost _guaranteed_ to be the wrong thing to do in a driver, since a driver won't know what locks the rest of the kernel is holding, _and_ since it's almost always possible for a driver to try to return an error code instead.

In short:

Either you want debugging (in which case BUG() is the wrong thing to do), or you don't want debugging (in which case BUG() is the wrong thing to do). You can choose either, but in neither case is BUG() acceptable.

BUG() is fine for _fundamental_ problems, where you don't have any other choice, and where the machine really is effectively dead anyway. If the VM notices that it's lists are corrupt, that's a BUG() thing. We don't have much choice. If the scheduler notices that it's running on another CPU than it thought it was running on, that's a BUG() thing.

Now, the kernel problem may be that BUG() is a bit too easy to use, and the alternatives are not. We should fix that. But we shouldn't fix it by using BUG() in places where it definitely doesn't belong.

Linus

There are probably good exceptions to this rule, like any rule.

For example, if the request queue (or some other fundamental internal data structure) is found to be corrupted, I couldn't really fault a driver for BUG()'ing out on it. It's not as if the driver could sanely even do an end_request() in that case.

But even broken hardware is not a reason for a BUG(). For example, if the driver notices that some part of the controller is hung hard (ie provable hardware problem), the last thing it should do is to bring the system down. It should fail the IO, and maybe turn itself off, but let the system continue otherwise.

One of the best things I ever did from a debuggability standpoint was to almost never use panic() in the base kernel, and make various kernel page faults etc just try to kill the offender and go on.

Sometimes that "try to continue" approach ends up being nasty too (the problem repeats and you end up with a dead machine that scrolls infinite bug reports off the screen, making them really hard to catch), but on the whole it tends to make things much easier to debug ("oops, I just lost my floppy drive, let's save the messages on the harddisk and reboot").

Linus

Or in even shorter sound bite format: "Just say no to BUG()s."

:)

Well, the thing is, BUG() _is_ sometimes useful. It's a dense and very convenient way to say that something catastrophic happened.

And actually, outside of drivers and filesystems you can often know (or control) the number of locks the surrounding code is holding, and then a BUG() may not be as lethal. At which point the normal "oops and kill the process" action is clearly fine - the machine is still perfectly usable.

(In fact, on UP a BUG() tends to be quite usable just about anywhere except in an interrupt handler: there may be some local locks like directory semaphores etc that are held and not released, but _most_ of the time the machine is quite usable. SMP really does make things harder to debug even quite apart from the races it introduces. Sad.)

Linus

I know you probably don't like the name, but all over the kernel people are using BUG() as ASSERT()... so why not create what people want?

IMO we should have ASSERT() and OHSHIT(), the latter being the true meaning and current implementation of BUG(), the former being used when the machine is still useable.

Jeff

From: Jeff Garzik <[email protected]>
Date: Tue, 10 Sep 2002 13:16:03 -0400

IMO we should have ASSERT() and OHSHIT(),

I fully support the addition of an OHSHIT() macro. :-)

Oh, please no. We'd end up with endless asserts in the networking layer, just because David would find it amusing.

I can just see it now - code bloat hell.

And no, I still don't like ASSERT().

I think the approach should clearly spell what the trouble level is:

	DEBUG(x != y, "x=%d, y=%d\n", x, y);

	WARN(x != y, "crap happens: x=%d y=%d\n", x, y);

	FATAL(x != y, "Aiee: x=%d y=%d\n", x, y);

where the DEBUG one gets compiled out normally (or has some nice per-file way of being enabled/disabled - a perfect world would expose the on/off in devicefs as a per-file entity when kernel debugging is on), WARN continues but writes a message (and normally does _not_ get compiled out), and FATAL is like our current BUG_ON().

All would print out the filename and line number, the message, and the backtrace.

Linus

hehe :)

I like, that would allow me to kill per-file debug code in some of my net drivers...

Jeff

Which still leaves the question, does it really make sense for FATAL/BUG to forcibly kill the machine? If the bug is truly fatal, presumably the machine kills itself in short order anyway, otherwise we might have a shot at recording the situation. A more useful distinction might be in terms of risk of damaging filesystems (or perhaps hardware) if we continue, something like BROKEN/DANGEROUSLY_BROKEN.

 "Love the dolphins," she advised him. "Write by W.A.S.T.E.." 

No. It should only be "locally fatal", and it should clearly just do what BUG() does now - kill the process.

But that implies very much that you really cannot use FATAL() in general at all, since it would be illegal to use whenever some caller holds some non-local locks (which is almost always the case for most "peripheral code").

Linus

Well we still have panic()... It might be nice to have a PANIC() macro with a similar form to that of DEBUG/WARN/FATAL...

And that's the heart of the thing, if continuing is likely to trash filesystem or (unlikely) damage hardware, then the system should go down RIGHT NOW.

I've often wondered if it wouldn't be better to allow the user to provide a partition for oops use, where the kernel could write kmen and a few chosen other bit of information. Get all the oops output formatting code out of the kernel. Then the user could run tools like ksymoops against the oops after reboot, and a small utility could wrap and compress the oops, symbols table, config, etc, for future use by the user or developer.

I've sent a fair number of crash dumps of AIX to IBM, seems a good idea. And developers could have personal tools, archetecture dependent tools, ksysoops could be enhanced after the fact.

If it would help with a nasty bug I'd put all that and the kernel source I used, the module tree, etc, on a CD and send it. Whatever helps solve the problem. Too often the output is lost on the console and not written anywhere recoverable.

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

IOW, Linux Kernel Crash Dumps, something I've wanted for a long while.

http://lkcd.sourceforge.net/

From: Linus Torvalds <[email protected]>
Date: Tue, 10 Sep 2002 11:40:27 -0700 (PDT)

We'd end up with endless asserts in the networking layer, just because David would find it amusing.

:-) You know some of us do indeed miss the days of occaisionally waking up to find things like "inode.c: the problem is here" on our screens.

It drops you politely into the kernel debugger, you fix up the values and step over it. If you want to debug with zen mind power and printk feel free. For the rest of us BUG() is fine on SMP

Ok, a show of hands..

Of the millions (whatever) of Linux machines, how many have a kernel debugger attached? Count them.

In other words, if a user is faced with a dead machine with no other way to even know what BUG() triggered than to try to set up a cross debugger, just how useful is that BUG()? I claim it is pretty useless - simply because 99+% of all people won't even make a bug report in that case, they'll just push the reset button and consider Linux unreliable.

In other news, the approach that shows up in the kernel logs might just eventually be noticed and acted upon (especially if the machine acts strange and kills processes).

So I claim a BUG() that locks up the machine is useless. If the user can't just run ksymoops and email out the BUG message, that BUG() is _not_ fine on SMP.

It has nothing to do with zen mind power or printk's. It has everything to do with the fact that a working machine is about a million times easier to debug on than a dead one, _and_ is a lot more likely to get acted upon by most users.

Linus

Agreed. Along those same lines, it would be nice if the kernel could spit out symbolic names so the user can't screw up the backtrace and we've got a better chance of extracting a useful bug report from our users ;)

regards,

Rik

Spamtrap of the month: [email protected]

http://www.surriel.com/		http://distro.conectiva.com/

There is a patch for this. However its fairly useless since all the users are in X11 and while -ac will give you morse as well thats not terribly friendly.

There are some real mode patches that try and get you back into a sane video mode and dump you into a saner environment.

Why change video mode? As long as X told the kernel what it's doing, we know how to drive a linear framebuffer and in fact a few other types of framebuffers. In the 'oh shit' case we could at least _attempt_ to spew our dying splutter onto the screen.

--
dwmw2

It changes video mode to get the hardware into a known or roughly known state. Linear framebuffer is a bit of an assumption both at the low end (windowed) and high end (no framebuffer or tiled)

We shouldn't _assume_ it. XFree86 should tell us. And it can tell us about the other cases too; some of those we can deal with anyway.

--
dwmw2

You could make a usermode app that makes a DGA query and feeds the results to the kernel I guess

The way I see it, FATAL() is the only thing that really should escape from my local tree and end up ever being a permanent fixture of the code that is in the standard sources.

The other WARN/DEBUG variants seem, at least to me, to be a prime candidate for unmaintained clutter.

Also, another big win of OHSHIT(), or BUG_ON() or whatever you want to call it is that the thing is so cheap. Both in terms of space (no strings, no calls with varags arguments, 1 instruction) and time (a cycle or two).

With these DEBUG/WARN/FATAL things with the strings, I am much less likely to ever add them to code I am writing. Simply because I'll say "hmmm that's expensive, the check isn't that important"

On the other hand I do recognize your user side arguments.

But to counter the "BUG with locks freezes box" argument, 9 times out of 10 the thing that you are BUG()'ing on will freeze up the box anyways with the same locks held when some other thread of execution derefences some bad pointer created by, or as a side effect of, the erroneous state.

Eh, mine is getting one attached to it right now. It's getting more popular, and it would be more popular yet if it weren't considered some dirty little secret, or somehow unmanly.

Let's try a different show of hands: How many users would be happier if they knew that kernel developers are using modern techniques to improve the quality of the kernel?

Of course, I use the term "modern" here loosely, since kdb and kgdb are really only 80's technology. Without them, we're stuck in the 60's.

Daniel

Me too.

Reminds me of "Suns boot fast" (do a google search on it, and read the first thing that comes up).

Moving from the stone to the bronze age, one day at a time.

Yes. Using a really nice development environment is an "aha" experience. You can listen to people talk about it as much as you want, but if you have never actually used one you will never really understand what they're talking about, and why they get worked up about it.

Linux is WAY WAY far from being a really nice development environment. It's really nice in many other ways, but not in that way.

For example, there is no excuse for ever needing more than 1/4 second to get from the function you're looking at on the screen to its definition and source text, ready to read or edit.

There's no excuse for having to copy down an oops from a screen by hand, either. It's nice to know you can fall back to this if you have to, but having that be the default way of working is just broken.

There's no excuse for having to pepper low level code with printk's to bracket a simple segfault.

OK, I'll stop there. Actually, the only thing I'm really irritated about at the moment is the attitude of people who should know better, promoting the fiction that this veggie-zen-tools-made-out-of-wood thing is actually helping the kernel progress faster.

I suppose I'm going about this the wrong way, since directly pointing out wrong-headed thinking usually just causes a defensive reaction and puts people in a position where they're reluctant to back down for fear of losing karma. Well, then. Listen to Andrew. He knows whereof he speaks, and plus he's nice about it, unlike me.

Daniel

[Cc:'s mercilessly chopped down]
Daniel Phillips <[email protected]> said:

[...]

There are tools able to do so... nothing kernel-related.

Yes, I had to do that a (very) few times, when the machine went south and logging was dead before. But is is not the "default way" by any long shot.

It is no excuse to make _every_ kernel out there slow just for the hypothetical case it might crash sometime. Be optimistic, man! ;-)

Who said it makes the kernel progress faster? Kernel progress' bottleneck is the few people who know it in and out, and so are in position to decide on the patches presented to them. (It also depends on the scarcity of kernel architects, and no thanks, a comittee doing that work would get me to run away from Linux _very_ fast indeed; but that is irrelevant here).

Now, the productivity of the head developers/integrators is as high as the average quality of the patches they get. If Aunt Tillie and her sisters get a kernel debugger and start spewing off "Set to a random non-NULL value so it doesn't crash" patches kernel development will grind to a halt.

Yes, debuggers have their uses. But the ones I've used on userland programs were much, much less useful than you seem to think. Sure, it is nice to be able to check on the values in a data structure to find out how it got screwed up, but that's it (and that is a very occasional use for me, when other approaches have failed). And a kernel Oops usually gives you enough to work that out.

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

I do not see how using a debugger improves a quality of the kernel. Good thinking and coding does improve kernel quality. Debugger certainly does not help if someone cannot code.

A debugger can do some good things. Some people argue that it improves productivity, which I think may be true under some circomstances. If your build system sucks and/or slow, and if you work with a binary only software, debugger helps. If you work with something like Linux, and compile on something better than a 333MHz x86, it probably does not help your productivity. This is all wonderful, but has nothing to do with the code quality.

And to think that your users would be happier with a crap produced by a debugger touting Windows graduate than with a quality code debugged with observation simply defies any reason.

-- Pete

Uh, I feel obliged to respond to these statements just in case anyone thinks they contain anything which is correct.

I have spent twelve months doing kernel development without kgdb and eighteen months with. "With" is better.

uh-huh.

I have to agree. I don't ever recall using GDB to debug my own code, and I'm half-willing to accept the testosterone-fuelled position on that case, but in the cases where I'm supposed to be 'certifying' or otherwise debugging code which is provided by a customer, but which appears to have been written by a crack-smoking hobo they dragged in off the street, it's been _very_ useful.

--
dwmw2

It improves my quality of life, that would be enough by itself. And since I am doing this purely for my own satisfaction at the moment, I *will not* waste my time screwing around with 60's tools because somebody who is paid whether or not they work productively thinks they imbue the kernel with some kind of airy-fairy zen quality.

Look, we tried the zen state thing. It didn't work. Think about the madness in the period between 2.3 and 2.4, with one oops after another reported to the list, each taking days or weeks to track down. Sure, it allowed Linus and Al to show off their superior powers of cerebration, solving the things given only a few lines of oops as a result, but it sure fucked everything else up. As I recall, it wasn't fun at all in the end, not for Linus or Al either, and some folks got pretty close to burned out.

The answer to the question "is this sillyness slowing down development and reducing the quality of the kernel?" is "yes". I don't have to speculate about that any more, I've seen it enough with my own eyes. Now ask yourself who the most productive hackers are today, and ask yourself if they are using the good ol zen state blunt edged tools.

Sorry, but you are full of shit.

Daniel

OK, so you actually do not care about users getting happier. You should have added a smilie to the quote about the show of hands then.

The agrument about your quality of life does hold some water, at least I do not doubt that kdb makes you and Andrew happier. This is a wonderful thing. I do strongly suspect though, that any gains you get on the productivity front are NOT going to be used to improve code quality.

This has nothing to do with a debugger, this is a different topic. You actually want a crash dump analyzis tool, and so do I. So, let's discuss that. I happen to get e-mails with oops in USB callbacks pretty often, and they are always useless. It would be possible to track them if off-stack memory was saved, perhaps. However, to expect users to use debugger to collect this off-stack information is a delusion.

This is why Red Hat stopped shipping kdb and started to ship netdump (or so I think, anyway). It is a much more effective tool for the crash analysis, and it can be operated by a user. I think it beats a debugger fair and square. N.B. The data that netdump collects may be an image to be examined by a debugger (such as gdb), together with dedicated analysis tools. That's entirely different debugger, so no hypocrisicy here.

-- Pete

http://lkcd.sourceforge.net/

Linux crash dumps, and includes an analysis tool...

I used to work with the guy behind this, at SGI. He was one of three people in all of SGI that I wanted to hire after I left. I haven't played with this yet but I'll bet you 100:1 odds that you'll like it if Matt did it. He rocked at SGI.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

I believe we agree that you do not see it. :)

And reading glasses won't help someone who can't read. Is your above set of statements somehow meant to imply that a debugger cannot help someone who CAN code? (Logic. Logic is good here.)

A similar argument would be "Nobody should own an oven. If you can't cook you'll just make a mess."

It's a tool. Does anybody really disagree about its nature?

The main purpose of tools is to save time. No tool is a substitute for skill. it's possible to do everything a debugger can do via printks out to the printer and having a stack of source code printouts, and doing all your debugging in pencil. It's also possible to pound in a nail with a rock, if you think hammers are for sissys.

If you stop and think about it, it's sort of like amish kernel development, isn't it? (Hey, we've already got the community barn raising aspect. Now let's outlaw certain construction techniques! :)

I had the source code to the contractor-produce pile of cruft I worked with under OS/2. I could even read some of it, given enough time. Trust me, a source debugger REALLY helped find the differences between what it was supposed to be doing and what it was actually doing. (Or more accurately, the few places where what it was supposed to be doing and what it was doing matched up...)

I think we agree. So why recommend against debuggers if they save time and effort while producing patches of equal quality?

Sure, blame the tool. :)

So the state of the running kernel is more observablee without a kernel debugger than with one?

Debugging means, basically, "this sucker is doing something I didn't expect it to, and I have to find out what". A debugger is a tool with which to examine running code. It doesn't help you write the stuff, just examine it. The proper tool for the proper job...

The unix philosophy has always been to give people enough rope to shoot themselves in the foot with. I'm just wondering why the exception. (Yes I read the "bastard" speech. It seemed to boil down to a crude sturgeon's law filter because Linus was once again being overwhelmed by crap patches (an annual event around here). Making less than perfect coders code blind won't increase the quality of anything, but might decrease the quanity. I suppose that's one way of addressing the problem, but doesn't seem to me to be an optimal solution...)

Rob

(P.S. Don't mind me: I have a cold. If the above comes out as anything even resembling english, I'm ahead of the game...)

I can't speak for others, but my guess is that the people who don't like debuggers don't like them for pretty much the same reasons they don't like C++. The tool makes bad behaviour too seductive.

It's true that one can write supportable perl but noone but a naive person would base a multiple platform, multi-year lifespan product on perl.

It's true that one can write good systems code in C++ but experience has shown that noone but a naive person would argue for C++ for a kernel.

Debuggers are sort of in this camp. Yup, useful tool. The problem is that the real answer is that you should read and understand the code. It's a sign of a naive programmer when you hear "this code is all shit" and it's useful code. That means the programmer would rather rewrite working code than understand it enough to fix it. Extremely common. And extremely wrong in almost all cases. It's *hard* to understand code. Get over it. Read the code, think, read again, think some more, keep it up. Always always always assume the guy who came before you *did* know what they were doing. Otherwise all you do is replace mostly working code with brand new code that works for the *one* case in front of the new programmer and none of the 100's of cases that the old code handled.

I don't think anyone is against debuggers. I'm not. I'm against people not thinking. I'm for people who think, who are careful, who have some respect for code that works.

It's so much more fun to say "this code is shit, I can do better", but whenever I've said that I've been wrong about 90% of the time. And I'm a pretty good programmer, I know that I shouldn't think like that. All I'm saying is that thinking is greater than debuggers. Much greater.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

Second post in my "Understanding the Principles of Argumentation" series.

You have just provided us with a fine example of a common logical falacy called "bifurcation":

"Also referred to as the "black and white" fallacy. Bifurcation is the presentation of a situation or condition with only two alternatives, whereas in fact other alternatives exist or can exist."
(http://www.theology.edu/logic/logic23.htm)

Daniel

Once again. BS. 99% of the cases where I had to work on foreign code, it was a codebase where someone with a clue wrote something nice, and then lots of people without clue "improved" that code. Then I start to work on it and have to clean up the mess. First thing is that you want to everytime is, to _understand_ what the original author wanted to do with the code and what the clueless did to this idea. A debugger is a decent toy for this. commons-logging another.

Code which was written like you describe is never hard to understand. Clueful people know that they have to comment their "tricks".

Ask me about Bean-Setters with a return value. Now that's clever. =:-(

Regards
Henning

Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     [email protected]

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   [email protected]
D-91054 Buckenhof     Fax.: 09131 / 50654-20   

Hi,

Perl is pretty useful for code generation and/or parsing, e.g. in order to find mistakes you did or to build the thing you've created. It can also be used to convert code. Thus I wouldn't say it's useless in an multiple-platform multi-year environment.

That's true, but no reason to eat people for breakfast coz they do use debuggers to get beyond where code does what wrong.

Thunder

!assert(typeof((fool)->next) == typeof(fool));

You meant:

	assert(typeof((fool)->next) != typeof(fool));

Daniel

Hi,

No, I mean "Never assert that the one next to a fool must be a fool, either. You might be wrong."

Thunder

!assert(typeof((fool)->next) == typeof(fool));

A proper assert does not return a value, by definition.  It relies purely on side effects, that is, it causes a screeching halt if the logical expression was false.  It's always wrong to use an asssert in an expression, and since BUG_ON is just assert(!expression) this applies to BUG_ON as well.  The compiler should prevent you from making this mistake.

What I *thought* you meant was: "if the next fool that comes along is exactly the same as the last fool, run away screaming".  There's some wisdom in that.

Daniel

I think the point here is that debuggers are not for everyone. They are a tool, in much the same way as an adjustable wrench is. I prefer a set of spanners.

Is there anything wrong with NOT wanting to use a debugger? really?

I don't want to use reiserfs. I'm not lobbying to keep it out of the kernel for the significant number of people who do want it.

It's Linus's final call, and now we can fake 95% of the same functionality by attaching userspace debuggers to UML, so...

Rob

You're all talk and no action.

The last time I looked, the people who really _do_ improve the quality of the kernel don't tend to care too much about debuggers, or are at least capable to add a patch on their own.

In fact, of the people who opened their mouth about the BUG() issue, how many actually ended up personally _debugging_ the BUG() that we were talking about?

I did. Matt probably did. But I didn't see you fixing it with your debugger.

So next time you bring up the kernel debugger issue, show some code and real improvement first. Until then, don't spout off.

Linus

Hi,

Um, several kernel hackers here use UML also because of the gdb interface.

bye, Roman

[I haven't read the beginning of this thread. I don't know which debugger you're talking about, although I suspect from context it's KDB. I'm just going to pretend it's KGDB because that's the one that I consider most useful.]

Let me offer a different perspective, with example. Perhaps you remember the paste-o fix I sent you last week. A patch had changed:

list_entry(current->ptrace_children.next,struct task_struct,ptrace_list)
to:
list_entry(current->ptrace_children.next,struct task_struct,sibling)

I usually develop on 2.4. More, I usually develop on a tree that already has i386 kgdb merged, or on non-i386 architecture which has a kgdb stub as standard - MIPS or PowerPC. I find it a great deal more convenient, for a whole lot of reasons. For instance:

Tracking this bug down took me about six hours. Someone more familiar with that particular segment of code could, I assume, have done it more quickly. One advantage of a debugger is that it's easier to look at parts of the code you aren't intimately familiar with, and still find the problem. With access to a debugger, my process would have gone something like this:
- establish that exit_notify was crashing the first time that an
initial thread exited (half an hour. because my i386 test machine reboots very slowly.)
- step through it (five minutes)
- see it step into zap_thread, the incorrect line right there on my
screen, the corrupted argument immediately obvious

As it was, there is no 2.5 / i386 port of KGDB as far as I know; George's patches are only for 2.4. Porting KGDB to 2.5 would have taken me more than the hours I wasted with printks and oops traces.

Plus, in my experience the work model that BitKeeper encourages puts a significant penalty on including unrelated patches in the tree while you're debugging. It can be gotten around but it's exceptionally awkward. Adding in KGDB means time spent merging it into my tree and time spend merging it cleanly out when I'm done with it.

I even have ulterior motives in suggesting that KGDB be included - with a proper and complete KGDB in the kernel, the features that make KGDB more than just the minimal stubs that other platforms have could move to common code. MIPS, for instance, could have a thread-aware KGDB, for instance.

I've always thought it would be useful. Sure, everyone debugs differently; but a number of people seem to agree with me that KGDB is convenient.

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

I have kgdb for every kernel back to 2.4.0-something. Have a fish around in http://www.zip.com.au/~akpm/linux/patches/

I suspect the example which you give is not a very typical one. Generally people use kgdb for poking around looking at kernel state. I almost never single-step. I set breakpoints so that I can inspect state within a particular context, and for coverage testing ("is this path being executed").

Also as a replacement for printk/rebuild/reboot.

Also for inspecting ad-hoc instrumentation: just add `some_global_int++;' and then take a look at its value - much quicker than exposing it via /proc.

It's also very good to have kgdb on hand when you happen to hit a really rare bug - I hit a weirdo request queue corruption thing the other day, an hour into a `dbench 1024' run. Was able to get a decent amount of information. Heaven knows how long it would take to make that bug trigger a second time...

Right, I almost never single-step either. Probably, my favorite technique is to replace BUG() with asm("int3").

Yup. I still use a lot of printks, but they are mostly for tracing, not triangulation.

Yep, I noticed you doing that, very useful, I picked up that technique from you. Then I made a little all-purpose proc interface for myself to make it easy to export my stats structures, which is kind of nice too, because then you can put a patch out with the hooks still in it, making it easier for others to participate in the analysis.

Yes, this last one applies equally to kdb, but since I'm not relying on kdb at the moment, best to leave the lid on that can of flamable material.

Daniel

I'm not speaking for Linus, but I wouldn't be surprised we share the same view on this one. As someone who maintains a fairly large source base I get nervous when people tell me they need a debugger to work on the code. Why? Because if you really need that it is EXTREMELY likely that you don't understand the code. If you don't understand the code then YOU SHOULDN'T BE CHANGING IT. It is infuriating to have a section of tricky code that used to work, you turn your back, only to find that someone made a "simple change" which seems to work but actually makes things worse and invariably seems to break the code in a far more subtle way.

My position is that you either understand the code or you don't. Code that you don't understand is read only. Having a debugger show you some variables isn't going to make you understand the code at the level which is required in order to be making changes.

Does this mean I'm against debuggers? Not at all. But in 15 years of doing kernel work and 5 years of doing BK work the only thing I've ever used one for was to get a few variables printed out. And I've written a substantial chunk of a debugger years ago, it's not a question of lack of debugger knowledge. I just rarely find them useful.

Create a throwaway clone, merge in kdb, tag the tree with "baseline". Now hack away until you have a fix. If you never checked anything in after the baseline then "bk -r diffs -u" creates the patch for your bugfix. If you did, then diff against the baseline.

If BK is awkward by comparison to diff and patch, something is wrong, it definitely has the ability to make things far more pleasant than you seem to be experiencing.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

That's a circular argument. I use debuggers in order to understand code _better_. And to understand how code that I do know interacts with code that I don't know. I also rarely have the luxury of working in source bases that I've got long intimate association with - about the only code I can claim that for is GDB itself and it still surprises me.

Using a debugger I can pick up greatly improved understanding of what's going on - both what is and what should be. And what has changed in it recently, since I was last familiar with it, which was the problem here.

Good for you; as I said, everyone operates differently. The number of printing/thinking/rebooting cycles involved for me to work with a debugger is much shorter than without.

Perhaps I (yet again) need to spend a day learning more about the depths of BitKeeper. The last time I did it all I found were BitKeeper bugs, because the way I try to work with it appears to be contrary to the way other people want to work with it.

I guess I just need to get more used to doing all of my development in throwaway clones, and when it's absolutely perfect checking it into a longer-lived tree via exporting a GNU patch and importing that.

The above model gets much more complicated when the development lasts longer than a couple of hours, and you have to pull from another tree; already, to upgrade my working trees I need to cascade three pulls - with two sets of resolves if Ingo's been changing CLONE_ flags on me again :)

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

We clearly have different definitions of what is acceptable. If your attitude is that understanding the code is a luxury all that means is that I want you nowhere near any code I maintain. Non-trivial code requires non-trivial understanding and that understanding is not a "luxury" in my book.

If your company has such a poor business model that they can't afford to pay you enough to take the time to do a good job then find a different place to work. No amount of debugger "help" is going to make up for a lack of understanding.

Yeah, I'm up on my high horse, but this sort of stuff just infuriates me. If you aren't going to spend the time to do the code right, which means spending the time to *completely* understand what it is that the code is doing, why the problem is occurring, and why the fix is a real fix and not a bandaid, then all you are doing is creating more work for somebody else in the future. You are *guessing*. Maybe your company tolerates that but I don't have to and I sure as hell don't and I'd question anyone or any company that does. If you like working like that maybe you should consider Microsoft, they seem to really value that approach. Otherwise realize that you are straying and get back on the correct path.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

Well, since the code Daniel was referring to having debugged (ptrace and thread debacle earlier) was broken and he successfully fixed it -- he is welcome to come near code as far as I am concerned.

In short, he and Ingo fixed some messed up code and that is all that matters. I do not care if he uses a debugger or a magic ball, so long as he fixes it.

Personally, I do not use a debugger and it is partly because of the reasons you list. But if Daniel can fix a problem (which he most likely ran into firsthand with his work on gcc and gdb) then he is welcome in my eyes.

Please, give it up.

Robert Love

Maybe he works at a company with a good enough business model to realise that people who can't or won't use tools for pet high horse reasons are ineffeciencies that are best "downsized" either in ego or personnel count.

"If you aren't going to spend the time to do the code right"

Well DAC960 isn't his code. DAC960 hardware is also opaque. Its unfortunate you can't seperate the right and wrong way to use a debugger and what you learn from it. It's a tool nothing more. Its what you do with the data you get from it that matters. A debugger is a great way to make pigs apparently fly, but that isnt its only use

Alan

You are completely missing the point. I explictly stated that debuggers are just fine with me, I don't really care one way or the other. I also stated that I use debuggers and I'm OK with other people using them as well. Where's the "won't use tools for pet high horse reasons"? Nowhere.

My comments, which I stand behind now and will stand behind 10 years from now, are based on the fact that people who don't understand the code shouldn't be modifying the code. If a debugger helps you understand the code, go for it. However, my experience is that what he was saying resonates with "I'm going in to fix this problem so I can get back to work on my real project". And that is almost always wrong. If the problem was that bloody simple don't you think the original author of the code would have fixed it already? It's almost never as simple as a naive point of view thinks it is and that's exactly why you don't want people hacking about in that code. Either understand it and really fix it, own it, maintain it, live with it, or leave it alone.

You may have a different opinion, Alan, and that's fine. All that means is that you won't ever work here. One of the nice things about being the guy who runs the company is that you get to insist on a certain level of competence and professionalism or you're fired. It's one of the reasons I don't work for someone else, I like being able to say "do it right or don't do it, I pay the bills, that's what I want". I learned that at Sun, over my every objection because I was an idiot, but I learned it. After you learn the benefits of doing things right you have nothing but pity for people who do it wrong.

---
Larry McVoy            	 lm at bitmover.com           http://www.bitmover.com/lm 

BS. In 99% of the cases you fix the problem because the original author has abandoned the code or does no longer care about it.

Ask me about it. Once upon a time, I just wanted to use a Web Framework [1] for a project. Now I'm one of the main developers of it. Not because I wanted to be but out of pure necessity.

Regards
Henning

[1] Jakarta Turbine. http://jakarta.apache.org/turbine/

Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     [email protected]

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   [email protected]
D-91054 Buckenhof     Fax.: 09131 / 50654-20   

Sometimes people who maintain the code move on to other things.

So, maintenance is sometimes required by people who do not have "maintenance of this code" in their job description. When I'm paying their salary, they get to fix other peoples code, even if it's none of their business.

Now preferably such a fix would be passed through a maintainer who can say (with his intimate knowledge of the code): "Oops, you're right your patch is indeed an improvement".

If that's not possible, we'll have to make do with some testing.

Roger.

** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* The Worlds Ecosystem is a stable system. Stable systems may experience *
* excursions from the stable situation. We are currenly in such an       * 
* excursion: The stable situation does not include humans. ***************

On Sun, 15 Sep 2002, Larry McVoy wrote:

[...]

Larry, you tend to use words like 'company' and 'business' too much in your messages... B-)

Suppose that I (definitely not a guru - I deal with the Linux kernel just for fun, and I don't *need* any *professional* knowledge on it) find some obscure bug (maybe I'm using some uncommon HW). I understand *nothing* about the piece of kernel that actually BUG()s - probably it's the first time I see it. Nevertheless, I *like* kernel hacking. So I start chasing the bug (and a debugger helps me a lot). It takes hours/days but who cares? I'm having fun. Later I come up with a patch, which seems to fix it. It may be complete crap. But I send a bug report *and* my patch to Alan (or J. Kernel Mantainer). In a hour I get a reply back, saying:
"your patch is wrong, but I see the bug and I've fixed it in my tree" (that's because *he* knows the code).

Don't you think that even random and clueless hacking can be (at times) a valuable source of information for real hackers? That's very open source specific, of course you don't run a *company* like that, but who cares? Many of the lessons you learned in your business career apply here, but some simply don't. The Linux kernel development isn't run by someone that pays the bills...

.TM.

You are hereby invited to debug the DAC960 driver. Making the needed changes only took a few hours to get it to the point where it initializes, now it oopses in the interrupt handler, because I did the bio part wrong. Of course, you might not make any mistakes, but you probably will, so get ready with your printks. It's going to take you a while, I can assure you.

I flat out refuse to do this without kgdb.

Daniel

It isnt about understanding the code. You issue a perfectly valid command to your scsi driver. Your ethernet crashes. All your code is perfect.

Thats a real scenario. In fact thats a horribly common scenario, and the kind of thing I have to deal with every day of the week because I debug driver code.

Understanding your own code by using a debugger (or printk) to see what is does is not good practice. Understanding misdocumented or crap code often needs a debugger to see what is going on, and to be able to understand how to fix it. The moment you hit the hardware layer the fun really begins, and you need the debugger, not to understand your code flow but to snoop around the machine to see what the device did or didn't do.

Actually, in BK-current you don't even need to do that, as UML has been integrated into 2.5.x for a few days now.

I'll make a 2.5.35 later today, assuming no new threading bugs crop up. And then you can use a debugger on a bog-standard kernel, and maybe this issue won't come up ever again.

Yeah, right.

Linus

I use UML all the time. It's great, but it doesn't work for SMP debugging.

By the way, thanks much for integrating UML.

Daniel

That should not be something fundamental, though. It should be perfectly doable with threading. "SMOP".

Yeah, and gdb (not to mention all the grapical nice stuff) sucks in a threaded environment. At least it used to.

Linus

I run into problems fairly often that I can't reproduce in UML - timing sensitive, hardware sensitive, etc. Some of them KGDB perturbs too much to be useful, but most of the time I can get it to work. UML also doesn't use a lot of the code under arch/i386/ (or didn't at least) which makes debugging that code under UML a bit futile.

Well, yeah. It's getting a little bit better - a lot better for some cases - but no one's really sure where it needs to go to keep improving. I'm making a little progress.

Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

True, however, it's really amazing what a wide range of kernel problems UML does faithfully reproduce. It is, after all, just another port. You could make the same argument about MIPS distorting timings vis a vis i386.

Oh, and there is another big suckage in UML in the area of modules, you have to load the symbols by hand, which is just enough pain to make it not worth compiling things as modules in UML. Jeff has reasons why its hard to get gdb to load the module symbols automatically, but hopefully, that is another problem we can dig into now that UML is officially part of the tree.

Daniel

There is a nice expect script available with automates that for you. See
http://user-mode-linux.sf.net/debugging.html, towards the bottom.

That's from Chandan Kudige, who followed that up by making UML sort of work on Windows :-)

Jeff

In addition, there is a gdb hacker the thread who has been thinking a little about nonsucky ways of doing it. You might want to kick around some ideas with Daniel, some cute way of passing events.

Daniel

Timing works both ways. Maybe you run into a bug on native i386 that UML won't reproduce. Maybe UML will reproduce bugs you won't see on hardware. That doesn't help you track down a particular bug you're seeing on hardware, but as far as overall bug smashing goes, it looks like a wash to me.

That's potentially fixable. UML has SCSI support now, with a USB driver in the works. Other hardware access is possible, too.

Then we need to push code out from arch into the generic kernel. That's happening slowly, but there's a bunch more to go.

Jeff

Jeff and I occasionally kick this around and it always ends with "yeah, not hard at all, when I have more time on my hands than I know what to do with maybe I'll do that", or words to that effect. It's understandable why Jeff hasn't gotten around to doing it, given the workload he's had just maintaining UML in both the 2.4 and 2.5 kernel series. Plus Jeff does not get paid of any of this.

Now that it's integrated, nice things like that can happen, and maybe we won't have to impose on Jeff to do 100% of the work on UML in the future. (Well, actually I did about 0.00001% of the work on UML, to wit, a patch to change the bogus virtual contents of ptes to "physical" addresses, bringing UML in line with the other arches in that regard. <- part of my new policy of making sure everybody knows about every little contribution I make.)

It still does. That's yet another source of irritation.

Daniel

Not yet, but it's on my short list now.

When it happens, you can send Peter Braam a nice note for enticing me to get it done :-)

Jeff

Given that most of the codebase is in drivers (never mind non-UML supported arches), that's a little unlikely...

 "Love the dolphins," she advised him. "Write by W.A.S.T.E.." 

Oh sure. So last week's page release race counts as no action, and htree is not the fastest directory index in the known universe. I could go on. What I do not do is rub people's faces in every little thing I do. Please remember who found the last truncate race of the year, just before 2.4.0-prerelease.

I am personally working on a different problem.

I am setting up the debugger to work on the DAC960.

You're shooting way wide of the target.

Daniel

See, even though I'm not fundamentally against using kernel debuggers, I think this is very wrong. Where are you now? You are just learning about the bio interface and the changes needed to make it run. And this is definitely the most wrong point to start using a debugger, and can only result in a dac960 that works by trial and error.

Jens Axboe

Nevertheless, the existence of a case where it's not sensible to use a debugger does not prove the non-existence of cases where it _is_ sensible to use a debugger.

A case that happened to me recently -- tail-call optimisations screwed up the return value of a function somewhere deep in the routing code. Adding a printk made the problem go away. Staring at the C code was also of no use -- the C code was just fine.

Now, are you seriously suggesting that instead of using GDB to work out WTF was going on I should have spent three weeks starting at the output of 'objdump -d vmlinux' ?

While my balls are big enough and hairy enough that I don't need to use GDB to debug my own code, I feel no shame in admitting that I'm so much of a pussy I can't deal with the above case without GDB.

--
dwmw2

I'm not suggesting anything for your case, and I'm not arguing against kernel debuggers. Please re-read what I wrote: using a debugger for what Daniel is attempting right now is stupid. Are you seriously suggesting that you would trust your data to a driver that had been ported to 2.5, not by studying the interface changes but by 'code blow up, gdb inspection' method? I hope not.

I've used a kernel debugger a few times, for the things I tend to do it's not very helpful. Or let me rephrase that into saying that it's not more helpful than simply having the kernel compiled with -g and using gdb on the resulting vmlinux just for backtraces and code inspection. So I'm fine, and so I don't care very much about the typical pro/con integrated debugger debates.

Jens Axboe

I am not using the debugger to learn about bio, I use LXR for that. I am using the debugger because of the nature of the bugs I expect and already seen. Jens, I have 27 years of professional programming experience, you do not have to tell me when to use a debugger and when not to.

Daniel

Hi all,

just my .02 $

what I mostly admire in linux and it's development till now, is it's flexibility to adapt the dynamic complex changing requirements from server to desktop even embedded system and real-time system.

And still though no one feels disadvantaged,
since features not needed are "removable".

I'm just wondering "if it's worth the effort" to provide "removable customizable kernel debugging tools (entry/break points)", which one can replaces with tools of his/her choice.
I'm talking about some unified cleaver reasonable MACROS, which one can easy insert/use them where needed.

It's still wrong to force development tools to be persist in running system where they are not needed.

There are lots of "nice things" that can be implemented in kernel, but are they essential?
Tools should not become burdens!

Best regards,
S. Effendi

That's a little out of line. While Daniel seems to enjoy the sound of his own typing even more than most of us, he *does* do useful work.

Rusty.

   there are those who do and those who hang on and you don't see too
   many doers quoting their contemporaries.  -- Larry McVoy

Yeah, I'm sorry Daniel. I just don't appreciate people who demand me to support built-in kernel debuggers, despite me having made my position quite clear over the years...

Linus

Thanks. Well, it comes under the category of "managing Linus", and everybody does, or tries to, up to a point. Not that it often works.

Daniel

Well, how much work would you manage to do if you had 350 managers ? ;)

Rik

Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

Spamtraps of the month:  [email protected] [email protected]

I just had an idea: There are the periodic discussions about dumping the oops-report somewhere where you can log it on the next boot, right?

Disk drivers may be unreliable etc etc.

Main memory is probably cleared on boot, right? (would have been too obvious).

I'm pretty sure that my system has 16M of non-cleared memory: The video RAM. The state of my screen before the crash always shows up while X is starting.... (I was running a foolishly old 2.4.3-ac3 until a couple of weeks ago)

Is my system special in that it doesn't clear video RAM? Can we grab a pointer to video memory when we boot, check a magic value, if OK, print the oops report from last crash, put in a new magic value ("No oops report"), and when we crash write in the crash report with the special magic value?

Roger.

** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* The Worlds Ecosystem is a stable system. Stable systems may experience *
* excursions from the stable situation. We are currenly in such an       * 
* excursion: The stable situation does not include humans. ***************

There's an important case you're overlooking that takes us out of the "sometimes" zone, and that is where you want to load up some piece of heavily-context-dependent code with assertions, just to have confidence that the many assumptions actually hold. And once you have those hooks in the code, it often makes little sense to take them out, because they'll just have to go back in again the next time some remote part of the kernel violates one of the assumptions.

What you *want* to do, is just turn them off, not remove them. (Sure, there are lots that shouldn't survive the alpha version of any code, but still lots of good ones that should be kept, just stubbed out.)

So this is just a name problem: define MYSUBSYSTEM_BUG_ON which is nil in production, but equivalent to BUG_ON for alpha or beta code. In the former case it means the code is going to run a little faster, plus the system is going to be a little more resilient, as you say.

Otherwise, completely agreed.

Eventually we could try some fancy trick that involves keeping track of the locks in a systematic way, so that they can be analyzed automatically by a tool that generates lock-breaking code. Then a subsystem could use the generated code in its error path. Sure, it's a lot of work just to add another "9" to reliability, but when was anything ever easy?

Daniel

Hi,

Our gatekeeper has never even heard of X. And no, I wouldn't call it a server. The only thing it does is to control which doors and gates are open and which are closed, and whether or not the runaway is free...

Thunder

--./../...-/. -.--/---/..-/.-./..././.-../..-. .---/..-/.../- .-
--/../-./..-/-/./--..-- ../.----./.-../.-.. --./../...-/. -.--/---/..-
.- -/---/--/---/.-./.-./---/.--/.-.-.-
--./.-/-.../.-./.././.-../.-.-.-

So you wanted to say that you'd prefer the machine to crash on a BUG() than try to keep going in case of a recoverable error? I don't think you'd like to stay locked in. At least that was what this discussion was about.

Vojtech Pavlik
SuSE Labs