* [PATCH] aoe: error printed 1 too early
@ 2009-02-25 13:38 Roel Kluin
2009-02-25 17:09 ` Ed Cashin
2009-03-04 8:08 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Roel Kluin @ 2009-02-25 13:38 UTC (permalink / raw)
To: ecashin; +Cc: lkml, Andrew Morton
with while (i-- > 0); i reaches -1 after the loop, so the test below is printed
one too early: 0 still means success.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index cc25057..eeea477 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -173,7 +173,7 @@ skbfree(struct sk_buff *skb)
return;
while (atomic_read(&skb_shinfo(skb)->dataref) != 1 && i-- > 0)
msleep(Sms);
- if (i <= 0) {
+ if (i < 0) {
printk(KERN_ERR
"aoe: %s holds ref: %s\n",
skb->dev ? skb->dev->name : "netif",
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] aoe: error printed 1 too early
2009-02-25 13:38 [PATCH] aoe: error printed 1 too early Roel Kluin
@ 2009-02-25 17:09 ` Ed Cashin
2009-02-25 22:01 ` David Miller
2009-03-04 8:08 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Ed Cashin @ 2009-02-25 17:09 UTC (permalink / raw)
To: Roel Kluin; +Cc: lkml, Andrew Morton
On Wed, Feb 25, 2009 at 02:38:54PM +0100, Roel Kluin wrote:
> with while (i-- > 0); i reaches -1 after the loop, so the test below is printed
> one too early: 0 still means success.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
> index cc25057..eeea477 100644
> --- a/drivers/block/aoe/aoedev.c
> +++ b/drivers/block/aoe/aoedev.c
> @@ -173,7 +173,7 @@ skbfree(struct sk_buff *skb)
> return;
> while (atomic_read(&skb_shinfo(skb)->dataref) != 1 && i-- > 0)
> msleep(Sms);
> - if (i <= 0) {
> + if (i < 0) {
> printk(KERN_ERR
> "aoe: %s holds ref: %s\n",
> skb->dev ? skb->dev->name : "netif",
>
You are talking about the case where, when entering the loop on the
"last chance" iteration, the states are,
atomic_read(&skb_shinfo(skb)->dataref == 1
i == 0
... in which case, the skb can and should be used but is not used
because after the value of i is tested it becomes -1.
Right?
Sounds like a good catch. Did you find it because you saw the "cannot
free skb" message?
--
Ed Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aoe: error printed 1 too early
2009-02-25 17:09 ` Ed Cashin
@ 2009-02-25 22:01 ` David Miller
2009-02-25 22:19 ` Ed Cashin
2009-02-25 22:29 ` roel kluin
0 siblings, 2 replies; 6+ messages in thread
From: David Miller @ 2009-02-25 22:01 UTC (permalink / raw)
To: ecashin; +Cc: roel.kluin, linux-kernel, akpm
From: Ed Cashin <ecashin@coraid.com>
Date: Wed, 25 Feb 2009 12:09:02 -0500
> Sounds like a good catch. Did you find it because you saw the "cannot
> free skb" message?
He's finding these by looking for them in the code, not by
actually triggering the condition.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aoe: error printed 1 too early
2009-02-25 22:01 ` David Miller
@ 2009-02-25 22:19 ` Ed Cashin
2009-02-25 22:29 ` roel kluin
1 sibling, 0 replies; 6+ messages in thread
From: Ed Cashin @ 2009-02-25 22:19 UTC (permalink / raw)
To: David Miller; +Cc: roel.kluin, linux-kernel, akpm
On Wed, Feb 25, 2009 at 02:01:36PM -0800, David Miller wrote:
> From: Ed Cashin <ecashin@coraid.com>
> Date: Wed, 25 Feb 2009 12:09:02 -0500
>
> > Sounds like a good catch. Did you find it because you saw the "cannot
> > free skb" message?
>
> He's finding these by looking for them in the code, not by
> actually triggering the condition.
Thanks. I was curious.
--
Ed Cashin <ecashin@coraid.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aoe: error printed 1 too early
2009-02-25 22:01 ` David Miller
2009-02-25 22:19 ` Ed Cashin
@ 2009-02-25 22:29 ` roel kluin
1 sibling, 0 replies; 6+ messages in thread
From: roel kluin @ 2009-02-25 22:29 UTC (permalink / raw)
To: David Miller; +Cc: ecashin, linux-kernel, akpm
2009/2/25 David Miller <davem@davemloft.net>:
> From: Ed Cashin <ecashin@coraid.com>
> Date: Wed, 25 Feb 2009 12:09:02 -0500
>
>> Sounds like a good catch. Did you find it because you saw the "cannot
>> free skb" message?
>
> He's finding these by looking for them in the code, not by
> actually triggering the condition.
>
Actually I made all functions a single line and then run some
bash scripts that parses pseudocode like:
echo "INFO: \7 reaches \9 - 1 after loop"
bli "
if \((${npz}[&|][&|])?
--($w) $em?[<=] ($K|$w|$d)
((&&|\|\|)$(nestedcode "()"))?\)
(\{
($(nestedcode "{}"))*
break ;
\}|
break ;
)
$ccode
(
$Q(\7 [=$em]= \9|\7 (>=|<) \9)
|
return \7 ;
)
"
This gives me a match like:
while(...) {
if (--i < 0)
break;
...
}
...
if (i != 0)
...
which is wrong since i reaches -1.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aoe: error printed 1 too early
2009-02-25 13:38 [PATCH] aoe: error printed 1 too early Roel Kluin
2009-02-25 17:09 ` Ed Cashin
@ 2009-03-04 8:08 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2009-03-04 8:08 UTC (permalink / raw)
To: roel.kluin; +Cc: ecashin, linux-kernel, akpm
From: Roel Kluin <roel.kluin@gmail.com>
Date: Wed, 25 Feb 2009 14:38:54 +0100
> with while (i-- > 0); i reaches -1 after the loop, so the test below is printed
> one too early: 0 still means success.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-04 8:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-25 13:38 [PATCH] aoe: error printed 1 too early Roel Kluin
2009-02-25 17:09 ` Ed Cashin
2009-02-25 22:01 ` David Miller
2009-02-25 22:19 ` Ed Cashin
2009-02-25 22:29 ` roel kluin
2009-03-04 8:08 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).