Re: [PATCH] return hidden bug

Previous thread: [GIT PULL -mm] 0/9 Unionfs updates/cleanups/fixes by Erez Zadok on Sunday, October 21, 2007 - 7:51 pm. (14 messages)

Next thread: [PATCH] unlock before bug returns by Roel Kluin on Sunday, October 21, 2007 - 10:10 pm. (7 messages)
To: lkml <linux-kernel@...>
Date: Sunday, October 21, 2007 - 9:05 pm

return hidden bug

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>

diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..6a69425 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -365,8 +365,8 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size,
printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx "
" base %lx size %x\n", dma_addr, arena->dma_base,
arena->size);
- return;
BUG();
+ return;
}

npages = calc_npages((dma_addr & ~PAGE_MASK) + size);
-

To: Roel Kluin <12o3l@...>
Cc: lkml <linux-kernel@...>
Date: Sunday, October 21, 2007 - 9:42 pm

On Mon, 22 Oct 2007 03:05:05 +0200

BUG() will terminate the process that runs into it, so you can
just remove the return alltogether. If BUG() is hit, the return
will never be reached.

--
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan
-

To: Rik van Riel <riel@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 2:36 pm

Looking at the printk, I don't think this particular error ought to
forcefully kill things.

--
Mathematics is the supreme nostalgia of our time.
-

To: Rik van Riel <riel@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 12:42 pm

[Empty message]
To: Ray Lee <ray-lk@...>
Cc: Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 1:56 pm

True, but obviously not intended. I think the intention was to expose this bug.

-

To: Roel Kluin <12o3l@...>
Cc: Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 2:12 pm

Arguing intentions is very dangerous. I've written code like that
where the intention is to make it simple to turn a printk into a full
bug and back and forth during development. At the end of the day, the
fact remains that you're changing behavior.

Let me turn this around. Do you have an alpha and have you tried out
your patch? If not, then I'd suggest turning it into a WARN_ON(1)
instead, as in this specific case you're risking turning what was a
working system into one that doesn't.
-

To: Ray Lee <ray-lk@...>
Cc: Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 2:52 pm

No, I haven't and, I will change it, but it's included with my other
changes. see the reply that I'll write shortly for.
[PATCH retry] return hidden bug and unlock bugs.

Roel

-

To: Roel Kluin <12o3l@...>
Cc: Ray Lee <ray-lk@...>, Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 6:26 pm

Hugely trust inspiring isn't it -- the amount of eyes and comments you'll
get even on trivial patches like this? This development model is working!

Now if only we'd sometimes get some for non trivial patches as well...

Rene.
-

To: Rene Herman <rene.herman@...>
Cc: Roel Kluin <12o3l@...>, Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 7:00 pm

Go easy with the snarkiness, hmm? It's the trivial ones that seem to
be the most dangerous. The larger ones actually get *tested*

That's certainly true regardless, but for myself, I'd rather throw
some reviews out for the small ones since I have adequate time and
knowledge for them. The larger ones require domain specific knowledge
I lack, and time I don't have.

~r.
-

To: Ray Lee <ray-lk@...>
Cc: Roel Kluin <12o3l@...>, Rik van Riel <riel@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 8:15 pm

I was also commenting and don't generally on anything much more involved so

Exactly the problem. Comment was mostly triggered due to me looking at a
problem with a proprietary CD-ROM driver again tonight that I posted a few
months ago where the only comment has been from the fellow author. There the
problem was the block layer blowing up and given that it seems unlikely that
this wouldn't be a problem inside the newbie-driver itself, that the block
part of it was actually really small and people said they'd look at it, the
subsequent thundering silence still annoys me.

Ofcourse, now it seems the kernel itself has moved on enough that the driver
doesn't work at _all_ anymore and I at the moment lack the time to spend the
required hours googling around trying to find out what the heck changed out
from under me so that I might get it to at least do what it already did do.

Hey, I don't actually know and maybe I'm just wrong but I have the feeling
that over the last 1 or 2 years most new developers seem to be either people
that are payed to be so, perhaps in the form of graduation, or janitors. The
kernel is much, much more complex than even only a few years ago and at the
same time the number of knowledgeable developers who'll do something other
than their own thing and otherwise just wait around for something perfect to
merge seems to be approaching zero.

That is -- I do not feel that the current developer base is expending overly
many efforts to appear welcoming.

Please feel free to do the open-source thing and argue that's actually an
advantage (there we have that snarkiness again...) or otherwise ignore me.
I'll just sit here and be grumpy anyway. Might be better after a good
night's sleep...

Rene.

-

To: Rik van Riel <riel@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 5:30 am

Hi,

This isn't true when CONFIG_BUG is disabled (in embedded builds, for example).

Pekka
-

To: Pekka Enberg <penberg@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 11:19 am

On Mon, 22 Oct 2007 12:30:00 +0300

*blink*

I guess people who disable CONFIG_BUG really choose to shoot themselves
in the foot when something bad happens. The kernel is full of error
paths where the current thread really should not be continuing.

Oh well...

--
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan
-

To: Rik van Riel <riel@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 11:23 am

Hi Rik,

Sure, if you disable BUG_ON, it may or may not work. But making it
*worse* on purpose seems pointless.
-

To: Rik van Riel <riel@...>
Cc: Roel Kluin <12o3l@...>, lkml <linux-kernel@...>
Date: Monday, October 22, 2007 - 1:02 am

This is true in general. However, if someone builds the kernel
with CONFIG_BUG disabled then the return statement will make a
difference. Of course, if you're brave enough to have CONFIG_BUG
off then you can deal with the fall-out :)

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-

To: Rik van Riel <riel@...>
Cc: lkml <linux-kernel@...>
Date: Sunday, October 21, 2007 - 9:53 pm

---
hidden bug returns

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index e1c4707..ca55c33 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -365,7 +365,6 @@ pci_unmap_single(struct pci_dev *pdev, dma_addr_t dma_addr, size_t size,
printk(KERN_ERR "Bogus pci_unmap_single: dma_addr %lx "
" base %lx size %x\n", dma_addr, arena->dma_base,
arena->size);
- return;
BUG();
}

-

To: Roel Kluin <12o3l@...>
Cc: lkml <linux-kernel@...>
Date: Sunday, October 21, 2007 - 11:08 pm

On Mon, 22 Oct 2007 03:53:30 +0200

Acked-by: Rik van Riel <riel@redhat.com>

--
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan
-

Previous thread: [GIT PULL -mm] 0/9 Unionfs updates/cleanups/fixes by Erez Zadok on Sunday, October 21, 2007 - 7:51 pm. (14 messages)

Next thread: [PATCH] unlock before bug returns by Roel Kluin on Sunday, October 21, 2007 - 10:10 pm. (7 messages)