* Re: [PATCH 10/14] drivers/isdn: delete double assignment
From: walter harms @ 2010-10-26 11:55 UTC (permalink / raw)
To: Julia Lawall; +Cc: Karsten Keil, kernel-janitors, netdev, linux-kernel
In-Reply-To: <1288088743-3725-11-git-send-email-julia@diku.dk>
Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location. In the first case, the
> hscx array has two elements, so change the assignment to initialize the
> second one. In the second case, the two assignments are simply identical.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression i;
> @@
>
> *i = ...;
> i = ...;
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> In the first case, the patch changes the semantics and has not been tested.
>
> drivers/isdn/hardware/mISDN/mISDNinfineon.c | 2 +-
> drivers/isdn/hisax/l3_1tr6.c | 2 --
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> index af25e1f..e90db88 100644
> --- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> +++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> @@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
> mdelay(10);
> hw->ipac.isac.adf2 = 0x87;
> hw->ipac.hscx[0].slot = 0x1f;
> - hw->ipac.hscx[0].slot = 0x23;
> + hw->ipac.hscx[1].slot = 0x23;
> break;
> case INF_GAZEL_R753:
> val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
> diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
> index b0554f8..a5c76fc 100644
> --- a/drivers/isdn/hisax/l3_1tr6.c
> +++ b/drivers/isdn/hisax/l3_1tr6.c
> @@ -164,8 +164,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
> char tmp[80];
> struct sk_buff *skb = arg;
>
> - p = skb->data;
> -
> /* Channel Identification */
> p = skb->data;
> if ((p = findie(p, skb->len, WE0_chanID, 0))) {
>
perhaps you can move the next assignment out of if also ?
p = findie(skb->data, skb->len, WE0_chanID, 0);
if (p) { ....
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment
From: walter harms @ 2010-10-26 11:58 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <1288088743-3725-6-git-send-email-julia@diku.dk>
Julia Lawall schrieb:
> From: Julia Lawall <julia@diku.dk>
>
> The other code around these duplicated assignments initializes the 0 1 2
> and 3 elements of an array, so change the initialization of the
> rx_session_id array to do the same.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression i;
> @@
>
> *i = ...;
> i = ...;
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> This changes the semantics and has not been tested.
>
> drivers/net/sb1000.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
> index a9ae505..66c2f1a 100644
> --- a/drivers/net/sb1000.c
> +++ b/drivers/net/sb1000.c
> @@ -961,9 +961,9 @@ sb1000_open(struct net_device *dev)
> lp->rx_error_count = 0;
> lp->rx_error_dpc_count = 0;
> lp->rx_session_id[0] = 0x50;
> - lp->rx_session_id[0] = 0x48;
> - lp->rx_session_id[0] = 0x44;
> - lp->rx_session_id[0] = 0x42;
> + lp->rx_session_id[1] = 0x48;
> + lp->rx_session_id[2] = 0x44;
> + lp->rx_session_id[3] = 0x42;
> lp->rx_frame_id[0] = 0;
> lp->rx_frame_id[1] = 0;
> lp->rx_frame_id[2] = 0;
>
/me is surprised that this did not cause more problems.
Is it needed at all ?
re,
wh
^ permalink raw reply
* Re: [PATCH 10/14] drivers/isdn: delete double assignment
From: Julia Lawall @ 2010-10-26 12:01 UTC (permalink / raw)
To: walter harms; +Cc: Karsten Keil, kernel-janitors, netdev, linux-kernel
In-Reply-To: <4CC6C1C0.8020005@bfs.de>
On Tue, 26 Oct 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Delete successive assignments to the same location. In the first case, the
> > hscx array has two elements, so change the assignment to initialize the
> > second one. In the second case, the two assignments are simply identical.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression i;
> > @@
> >
> > *i = ...;
> > i = ...;
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> >
> > ---
> > In the first case, the patch changes the semantics and has not been tested.
> >
> > drivers/isdn/hardware/mISDN/mISDNinfineon.c | 2 +-
> > drivers/isdn/hisax/l3_1tr6.c | 2 --
> > 2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > index af25e1f..e90db88 100644
> > --- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > +++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
> > @@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
> > mdelay(10);
> > hw->ipac.isac.adf2 = 0x87;
> > hw->ipac.hscx[0].slot = 0x1f;
> > - hw->ipac.hscx[0].slot = 0x23;
> > + hw->ipac.hscx[1].slot = 0x23;
> > break;
> > case INF_GAZEL_R753:
> > val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
> > diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
> > index b0554f8..a5c76fc 100644
> > --- a/drivers/isdn/hisax/l3_1tr6.c
> > +++ b/drivers/isdn/hisax/l3_1tr6.c
> > @@ -164,8 +164,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
> > char tmp[80];
> > struct sk_buff *skb = arg;
> >
> > - p = skb->data;
> > -
> > /* Channel Identification */
> > p = skb->data;
> > if ((p = findie(p, skb->len, WE0_chanID, 0))) {
> >
>
>
> perhaps you can move the next assignment out of if also ?
>
> p = findie(skb->data, skb->len, WE0_chanID, 0);
> if (p) { ....
OK.
julia
^ permalink raw reply
* [PATCH net-next-2.6] tg3: use dma_alloc_coherent() instead of pci_alloc_consistent()
From: Eric Dumazet @ 2010-10-26 12:03 UTC (permalink / raw)
To: Amit Salecha, Matt Carlson
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ameen Rahman,
Anirban Chakraborty, Michael Chan
In-Reply-To: <1288089033.3169.73.camel@edumazet-laptop>
Le mardi 26 octobre 2010 à 12:30 +0200, Eric Dumazet a écrit :
> By the way, you should use dma_alloc_coherent() instead of
> pci_alloc_consistent() so that you can use GFP_KERNEL allocations
> instead of GFP_ATOMIC, it might help in low memory conditions (if you
> dont hold a spinlock at this point)
>
tg3 being often used as a reference network driver, I believe we should
change its allocations before too many people copy paste suboptimal
pci_alloc_consistent() stuff...
Matt, could you please queue this patch and submit it to David when
net-next-2.6 reopens ?
Thanks
[PATCH] tg3: use dma_alloc_coherent() instead of pci_alloc_consistent()
Using dma_alloc_coherent() permits to use GFP_KERNEL allocations instead
of GFP_ATOMIC ones. Its better when a machine is out of memory, because
this allows driver to sleep to get its memory and succeed its init,
especially when allocating high order pages.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Matt Carlson <mcarlson@broadcom.com>
CC: Michael Chan <mchan@broadcom.com>
---
drivers/net/tg3.c | 73 ++++++++++++++++++++++++--------------------
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 852e917..7dfa579 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -6339,13 +6339,13 @@ static void tg3_rx_prodring_fini(struct tg3 *tp,
kfree(tpr->rx_jmb_buffers);
tpr->rx_jmb_buffers = NULL;
if (tpr->rx_std) {
- pci_free_consistent(tp->pdev, TG3_RX_STD_RING_BYTES(tp),
- tpr->rx_std, tpr->rx_std_mapping);
+ dma_free_coherent(&tp->pdev->dev, TG3_RX_STD_RING_BYTES(tp),
+ tpr->rx_std, tpr->rx_std_mapping);
tpr->rx_std = NULL;
}
if (tpr->rx_jmb) {
- pci_free_consistent(tp->pdev, TG3_RX_JMB_RING_BYTES(tp),
- tpr->rx_jmb, tpr->rx_jmb_mapping);
+ dma_free_coherent(&tp->pdev->dev, TG3_RX_JMB_RING_BYTES(tp),
+ tpr->rx_jmb, tpr->rx_jmb_mapping);
tpr->rx_jmb = NULL;
}
}
@@ -6358,8 +6358,10 @@ static int tg3_rx_prodring_init(struct tg3 *tp,
if (!tpr->rx_std_buffers)
return -ENOMEM;
- tpr->rx_std = pci_alloc_consistent(tp->pdev, TG3_RX_STD_RING_BYTES(tp),
- &tpr->rx_std_mapping);
+ tpr->rx_std = dma_alloc_coherent(&tp->pdev->dev,
+ TG3_RX_STD_RING_BYTES(tp),
+ &tpr->rx_std_mapping,
+ GFP_KERNEL);
if (!tpr->rx_std)
goto err_out;
@@ -6370,9 +6372,10 @@ static int tg3_rx_prodring_init(struct tg3 *tp,
if (!tpr->rx_jmb_buffers)
goto err_out;
- tpr->rx_jmb = pci_alloc_consistent(tp->pdev,
- TG3_RX_JMB_RING_BYTES(tp),
- &tpr->rx_jmb_mapping);
+ tpr->rx_jmb = dma_alloc_coherent(&tp->pdev->dev,
+ TG3_RX_JMB_RING_BYTES(tp),
+ &tpr->rx_jmb_mapping,
+ GFP_KERNEL);
if (!tpr->rx_jmb)
goto err_out;
}
@@ -6491,7 +6494,7 @@ static void tg3_free_consistent(struct tg3 *tp)
struct tg3_napi *tnapi = &tp->napi[i];
if (tnapi->tx_ring) {
- pci_free_consistent(tp->pdev, TG3_TX_RING_BYTES,
+ dma_free_coherent(&tp->pdev->dev, TG3_TX_RING_BYTES,
tnapi->tx_ring, tnapi->tx_desc_mapping);
tnapi->tx_ring = NULL;
}
@@ -6500,25 +6503,26 @@ static void tg3_free_consistent(struct tg3 *tp)
tnapi->tx_buffers = NULL;
if (tnapi->rx_rcb) {
- pci_free_consistent(tp->pdev, TG3_RX_RCB_RING_BYTES(tp),
- tnapi->rx_rcb,
- tnapi->rx_rcb_mapping);
+ dma_free_coherent(&tp->pdev->dev,
+ TG3_RX_RCB_RING_BYTES(tp),
+ tnapi->rx_rcb,
+ tnapi->rx_rcb_mapping);
tnapi->rx_rcb = NULL;
}
tg3_rx_prodring_fini(tp, &tnapi->prodring);
if (tnapi->hw_status) {
- pci_free_consistent(tp->pdev, TG3_HW_STATUS_SIZE,
- tnapi->hw_status,
- tnapi->status_mapping);
+ dma_free_coherent(&tp->pdev->dev, TG3_HW_STATUS_SIZE,
+ tnapi->hw_status,
+ tnapi->status_mapping);
tnapi->hw_status = NULL;
}
}
if (tp->hw_stats) {
- pci_free_consistent(tp->pdev, sizeof(struct tg3_hw_stats),
- tp->hw_stats, tp->stats_mapping);
+ dma_free_coherent(&tp->pdev->dev, sizeof(struct tg3_hw_stats),
+ tp->hw_stats, tp->stats_mapping);
tp->hw_stats = NULL;
}
}
@@ -6531,9 +6535,10 @@ static int tg3_alloc_consistent(struct tg3 *tp)
{
int i;
- tp->hw_stats = pci_alloc_consistent(tp->pdev,
- sizeof(struct tg3_hw_stats),
- &tp->stats_mapping);
+ tp->hw_stats = dma_alloc_coherent(&tp->pdev->dev,
+ sizeof(struct tg3_hw_stats),
+ &tp->stats_mapping,
+ GFP_KERNEL);
if (!tp->hw_stats)
goto err_out;
@@ -6543,9 +6548,10 @@ static int tg3_alloc_consistent(struct tg3 *tp)
struct tg3_napi *tnapi = &tp->napi[i];
struct tg3_hw_status *sblk;
- tnapi->hw_status = pci_alloc_consistent(tp->pdev,
- TG3_HW_STATUS_SIZE,
- &tnapi->status_mapping);
+ tnapi->hw_status = dma_alloc_coherent(&tp->pdev->dev,
+ TG3_HW_STATUS_SIZE,
+ &tnapi->status_mapping,
+ GFP_KERNEL);
if (!tnapi->hw_status)
goto err_out;
@@ -6566,9 +6572,10 @@ static int tg3_alloc_consistent(struct tg3 *tp)
if (!tnapi->tx_buffers)
goto err_out;
- tnapi->tx_ring = pci_alloc_consistent(tp->pdev,
- TG3_TX_RING_BYTES,
- &tnapi->tx_desc_mapping);
+ tnapi->tx_ring = dma_alloc_coherent(&tp->pdev->dev,
+ TG3_TX_RING_BYTES,
+ &tnapi->tx_desc_mapping,
+ GFP_KERNEL);
if (!tnapi->tx_ring)
goto err_out;
}
@@ -6601,9 +6608,10 @@ static int tg3_alloc_consistent(struct tg3 *tp)
if (!i && (tp->tg3_flags3 & TG3_FLG3_ENABLE_RSS))
continue;
- tnapi->rx_rcb = pci_alloc_consistent(tp->pdev,
- TG3_RX_RCB_RING_BYTES(tp),
- &tnapi->rx_rcb_mapping);
+ tnapi->rx_rcb = dma_alloc_coherent(&tp->pdev->dev,
+ TG3_RX_RCB_RING_BYTES(tp),
+ &tnapi->rx_rcb_mapping,
+ GFP_KERNEL);
if (!tnapi->rx_rcb)
goto err_out;
@@ -14159,7 +14167,8 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
u32 *buf, saved_dma_rwctrl;
int ret = 0;
- buf = pci_alloc_consistent(tp->pdev, TEST_BUFFER_SIZE, &buf_dma);
+ buf = dma_alloc_coherent(&tp->pdev->dev, TEST_BUFFER_SIZE,
+ &buf_dma, GFP_KERNEL);
if (!buf) {
ret = -ENOMEM;
goto out_nofree;
@@ -14343,7 +14352,7 @@ static int __devinit tg3_test_dma(struct tg3 *tp)
}
out:
- pci_free_consistent(tp->pdev, TEST_BUFFER_SIZE, buf, buf_dma);
+ dma_free_coherent(&tp->pdev->dev, TEST_BUFFER_SIZE, buf, buf_dma);
out_nofree:
return ret;
}
^ permalink raw reply related
* Question on UNIX domain socket.
From: Tetsuo Handa @ 2010-10-26 12:15 UTC (permalink / raw)
To: mtk.manpages; +Cc: netdev
In-Reply-To: <201010171428.DDC17187.FFFJSLtOOHOMQV@I-love.SAKURA.ne.jp>
Hello.
Is the latest manpage for UNIX domain socket is at
http://www.kernel.org/doc/man-pages/online/pages/man7/unix.7.html ?
I got a question. Above page says
pathname: a UNIX domain socket can be bound to a null-terminated file
system pathname using bind(2). When the address of the socket is returned
by getsockname(2), getpeername(2), and accept(2), its length is
offsetof(struct sockaddr_un, sun_path) + strlen(sun_path) + 1, and sun_path
contains the null-terminated pathname.
However, running below program results in
"sun_path *not* containing the null-terminated pathname" (and therefore it is
not safe to use printf("%s", sun_path) even if sun_path[0] != '\0').
Should "sun_path contains the null-terminated pathname" be corrected?
Regards.
----- Test program start -----
#include <stdio.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
union {
struct sockaddr_un addr;
char buf[512];
} u;
socklen_t len = sizeof(u.addr);
int fd = socket(PF_UNIX, SOCK_STREAM, 0);
u.addr.sun_family = AF_UNIX;
snprintf(u.addr.sun_path, sizeof(u.buf) - 2, "/"
"1234567890123456789012345678901234567890"
"1234567890123456789012345678901234567890"
"1234567890123456789012345678901234567890"
"1234567890123456789012345678901234567890");
if (bind(fd, (struct sockaddr *) &u.addr, sizeof(u.addr)) == EOF)
return 1;
memset(&u, 0, sizeof(u)); /* You may comment out this line. */
printf("len=%d\n", len);
if (getsockname(fd, (struct sockaddr *) &u.addr, &len) == EOF)
return 1;
printf("strlen=%d len=%d\n", strlen(u.addr.sun_path), len);
return 0;
}
----- Test program end -----
----- Output of test program -----
len=110
strlen=108 len=111
^ permalink raw reply
* Re: [PATCH 10/14] drivers/isdn: delete double assignment
From: Julia Lawall @ 2010-10-26 12:20 UTC (permalink / raw)
To: walter harms; +Cc: Karsten Keil, kernel-janitors, netdev, linux-kernel
In-Reply-To: <4CC6C1C0.8020005@bfs.de>
From: Julia Lawall <julia@diku.dk>
Delete successive assignments to the same location. In the first case, the
hscx array has two elements, so change the assignment to initialize the
second one. In the second case, the two assignments are simply identical.
Furthermore, neither is necessary, because the effect of the assignment is
only visible in the next line, in the assignment in the if test. The patch
inlines the right hand side value in the latter assignment and pulls that
assignment out of the if test.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression i;
@@
*i = ...;
i = ...;
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
In the first case, the patch changes the semantics and has not been tested.
drivers/isdn/hardware/mISDN/mISDNinfineon.c | 2 +-
drivers/isdn/hisax/l3_1tr6.c | 6 ++----
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
index af25e1f..e90db88 100644
--- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
+++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
@@ -563,7 +563,7 @@ reset_inf(struct inf_hw *hw)
mdelay(10);
hw->ipac.isac.adf2 = 0x87;
hw->ipac.hscx[0].slot = 0x1f;
- hw->ipac.hscx[0].slot = 0x23;
+ hw->ipac.hscx[1].slot = 0x23;
break;
case INF_GAZEL_R753:
val = inl((u32)hw->cfg.start + GAZEL_CNTRL);
diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
index b0554f8..ee4dae1 100644
--- a/drivers/isdn/hisax/l3_1tr6.c
+++ b/drivers/isdn/hisax/l3_1tr6.c
@@ -164,11 +164,9 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
char tmp[80];
struct sk_buff *skb = arg;
- p = skb->data;
-
/* Channel Identification */
- p = skb->data;
- if ((p = findie(p, skb->len, WE0_chanID, 0))) {
+ p = findie(skb->data, skb->len, WE0_chanID, 0);
+ if (p) {
if (p[1] != 1) {
l3_1tr6_error(pc, "setup wrong chanID len", skb);
return;
^ permalink raw reply related
* Re: [PATCH 5/14] drivers/net/sb1000.c: delete double assignment
From: Julia Lawall @ 2010-10-26 12:25 UTC (permalink / raw)
To: walter harms; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <4CC6C252.7080905@bfs.de>
On Tue, 26 Oct 2010, walter harms wrote:
>
>
> Julia Lawall schrieb:
> > From: Julia Lawall <julia@diku.dk>
> >
> > The other code around these duplicated assignments initializes the 0 1 2
> > and 3 elements of an array, so change the initialization of the
> > rx_session_id array to do the same.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression i;
> > @@
> >
> > *i = ...;
> > i = ...;
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> >
> > ---
> > This changes the semantics and has not been tested.
> >
> > drivers/net/sb1000.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
> > index a9ae505..66c2f1a 100644
> > --- a/drivers/net/sb1000.c
> > +++ b/drivers/net/sb1000.c
> > @@ -961,9 +961,9 @@ sb1000_open(struct net_device *dev)
> > lp->rx_error_count = 0;
> > lp->rx_error_dpc_count = 0;
> > lp->rx_session_id[0] = 0x50;
> > - lp->rx_session_id[0] = 0x48;
> > - lp->rx_session_id[0] = 0x44;
> > - lp->rx_session_id[0] = 0x42;
> > + lp->rx_session_id[1] = 0x48;
> > + lp->rx_session_id[2] = 0x44;
> > + lp->rx_session_id[3] = 0x42;
> > lp->rx_frame_id[0] = 0;
> > lp->rx_frame_id[1] = 0;
> > lp->rx_frame_id[2] = 0;
> >
>
> /me is surprised that this did not cause more problems.
> Is it needed at all ?
The field appears to be useful. However, there is an ioctl that also
initializes the elements of this field with these values and that is
implemented correctly. Perhaps that was sufficient to hide the problem in
practice.
julia
^ permalink raw reply
* Re: ath9k crashing the kernel
From: Felix Fietkau @ 2010-10-26 12:28 UTC (permalink / raw)
To: Jaswinder Singh
Cc: Linux Kernel Mailing List, linux-wireless, netdev,
ath9k-devel-xDcbHBWguxHbcTqmT+pZeQ, Linus Torvalds,
shafi.wireless-Re5JQEeQqe8AvxtiuMwx3w, John W. Linville
In-Reply-To: <AANLkTikHTgoJyxm80MuXM9oTvCPTFAPM12zVih1RsTfy-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 2010-10-26 9:09 AM, Jaswinder Singh wrote:
>> ath9k is crashing the kernel :
>>
>> [ 21.276554] BUG: spinlock bad magic on CPU#1, NetworkManager/1056
>> [ 21.277015] lock: f5be80a8, .magic: 00000000, .owner: <none>/-1,
>> .owner_cpu: 0
>> [ 21.277015] Pid: 1056, comm: NetworkManager Not tainted 2.6.36-netbook+ #20
>> [ 21.277015] Call Trace:
>> [ 21.277015] [<c14767a7>] ? printk+0xf/0x11
>> [ 21.277015] [<c117b823>] spin_bug+0x7c/0x87
>> [ 21.301365] [<c117b8bd>] do_raw_spin_lock+0x1e/0x125
>> [ 21.301365] [<c1478d0a>] ? _raw_spin_unlock_bh+0x1a/0x1c
>> [ 21.301365] [<c1478dc3>] _raw_spin_lock_irqsave+0x17/0x1c
>> [ 21.318857] [<c1288a74>] ath9k_config+0x255/0x38b
>> [ 21.318857] [<c1447bdb>] ieee80211_hw_config+0x10a/0x114
>> [...]
>>
>> Linux 2.6.36 f6f94e2ab1 is good
>> and
>> 229aebb873e2972 is bad
>>
>
> After further investigation bad commit is :
>
> 3430098ae463e31ab16926ac3eb295368a3ca5d9 is the first bad commit
> commit 3430098ae463e31ab16926ac3eb295368a3ca5d9
> Author: Felix Fietkau <nbd-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> Date: Sun Oct 10 18:21:52 2010 +0200
>
> ath9k: implement channel utilization stats for survey
>
This should already be fixed in wireless-testing by the following commit
commit 20b25744d1366762c6878d3254f93973cafe1f8e
Author: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Date: Fri Oct 15 15:04:09 2010 -0700
ath9k: Properly initialize ath_common->cc_lock.
- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 5/9] ipvs network name space aware
From: Hans Schillstrom @ 2010-10-26 13:07 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, ja@ssi.bg, wensong@linux-vs.org,
daniel.lezcano@free.fr
In-Reply-To: <20101022190548.GA24978@verge.net.au>
On Friday 22 October 2010 21:05:48 Simon Horman wrote:
> On Fri, Oct 08, 2010 at 01:17:02PM +0200, Hans Schillstrom wrote:
> > This patch just contains ip_vs_ctl
> >
> > Signed-off-by:Hans Schillstrom <hans.schillstrom@ericsson.com>
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> > index ca8ec8c..7e99cbc 100644
> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
>
> [ snip ]
> Hi Hans,
>
> is there a reason that the order some of the entries in
> vs_vars has been switched around?
>
Yes there is, when vars will be copied to it's own NS it's a lot easier
when they are in sequence and without a potential insert in the middle
the #if 0 ...
have a look at __ip_vs_control_init(struct net *net)
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* [PATCH] fib_hash: fix rcu sparse and logical errors
From: Eric Dumazet @ 2010-10-26 13:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev
While fixing CONFIG_SPARSE_RCU_POINTER errors, I had to fix accesses to
fz->fz_hash for real.
- &fz->fz_hash[fn_hash(f->fn_key, fz)]
+ rcu_dereference(fz->fz_hash) + fn_hash(f->fn_key, fz)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/fib_hash.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/fib_hash.c b/net/ipv4/fib_hash.c
index 43e1c59..b232375 100644
--- a/net/ipv4/fib_hash.c
+++ b/net/ipv4/fib_hash.c
@@ -120,11 +120,12 @@ static inline void fn_rebuild_zone(struct fn_zone *fz,
struct fib_node *f;
hlist_for_each_entry_safe(f, node, n, &old_ht[i], fn_hash) {
- struct hlist_head __rcu *new_head;
+ struct hlist_head *new_head;
hlist_del_rcu(&f->fn_hash);
- new_head = &fz->fz_hash[fn_hash(f->fn_key, fz)];
+ new_head = rcu_dereference_protected(fz->fz_hash, 1) +
+ fn_hash(f->fn_key, fz);
hlist_add_head_rcu(&f->fn_hash, new_head);
}
}
@@ -179,8 +180,8 @@ static void fn_rehash_zone(struct fn_zone *fz)
memcpy(&nfz, fz, sizeof(nfz));
write_seqlock_bh(&fz->fz_lock);
- old_ht = fz->fz_hash;
- nfz.fz_hash = ht;
+ old_ht = rcu_dereference_protected(fz->fz_hash, 1);
+ RCU_INIT_POINTER(nfz.fz_hash, ht);
nfz.fz_hashmask = new_hashmask;
nfz.fz_divisor = new_divisor;
fn_rebuild_zone(&nfz, old_ht, old_divisor);
@@ -236,7 +237,7 @@ fn_new_zone(struct fn_hash *table, int z)
seqlock_init(&fz->fz_lock);
fz->fz_divisor = z ? EMBEDDED_HASH_SIZE : 1;
fz->fz_hashmask = fz->fz_divisor - 1;
- fz->fz_hash = fz->fz_embedded_hash;
+ RCU_INIT_POINTER(fz->fz_hash, fz->fz_embedded_hash);
fz->fz_order = z;
fz->fz_revorder = 32 - z;
fz->fz_mask = inet_make_mask(z);
@@ -272,7 +273,7 @@ int fib_table_lookup(struct fib_table *tb,
for (fz = rcu_dereference(t->fn_zone_list);
fz != NULL;
fz = rcu_dereference(fz->fz_next)) {
- struct hlist_head __rcu *head;
+ struct hlist_head *head;
struct hlist_node *node;
struct fib_node *f;
__be32 k;
@@ -282,7 +283,7 @@ int fib_table_lookup(struct fib_table *tb,
seq = read_seqbegin(&fz->fz_lock);
k = fz_key(flp->fl4_dst, fz);
- head = &fz->fz_hash[fn_hash(k, fz)];
+ head = rcu_dereference(fz->fz_hash) + fn_hash(k, fz);
hlist_for_each_entry_rcu(f, node, head, fn_hash) {
if (f->fn_key != k)
continue;
@@ -311,6 +312,7 @@ void fib_table_select_default(struct fib_table *tb,
struct fib_info *last_resort;
struct fn_hash *t = (struct fn_hash *)tb->tb_data;
struct fn_zone *fz = t->fn_zones[0];
+ struct hlist_head *head;
if (fz == NULL)
return;
@@ -320,7 +322,8 @@ void fib_table_select_default(struct fib_table *tb,
order = -1;
rcu_read_lock();
- hlist_for_each_entry_rcu(f, node, &fz->fz_hash[0], fn_hash) {
+ head = rcu_dereference(fz->fz_hash);
+ hlist_for_each_entry_rcu(f, node, head, fn_hash) {
struct fib_alias *fa;
list_for_each_entry_rcu(fa, &f->fn_alias, fa_list) {
@@ -374,7 +377,7 @@ out:
/* Insert node F to FZ. */
static inline void fib_insert_node(struct fn_zone *fz, struct fib_node *f)
{
- struct hlist_head *head = &fz->fz_hash[fn_hash(f->fn_key, fz)];
+ struct hlist_head *head = rtnl_dereference(fz->fz_hash) + fn_hash(f->fn_key, fz);
hlist_add_head_rcu(&f->fn_hash, head);
}
@@ -382,7 +385,7 @@ static inline void fib_insert_node(struct fn_zone *fz, struct fib_node *f)
/* Return the node in FZ matching KEY. */
static struct fib_node *fib_find_node(struct fn_zone *fz, __be32 key)
{
- struct hlist_head *head = &fz->fz_hash[fn_hash(key, fz)];
+ struct hlist_head *head = rtnl_dereference(fz->fz_hash) + fn_hash(key, fz);
struct hlist_node *node;
struct fib_node *f;
@@ -662,7 +665,7 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
static int fn_flush_list(struct fn_zone *fz, int idx)
{
- struct hlist_head *head = &fz->fz_hash[idx];
+ struct hlist_head *head = rtnl_dereference(fz->fz_hash) + idx;
struct hlist_node *node, *n;
struct fib_node *f;
int found = 0;
@@ -761,14 +764,15 @@ fn_hash_dump_zone(struct sk_buff *skb, struct netlink_callback *cb,
struct fn_zone *fz)
{
int h, s_h;
+ struct hlist_head *head = rcu_dereference(fz->fz_hash);
- if (fz->fz_hash == NULL)
+ if (head == NULL)
return skb->len;
s_h = cb->args[3];
for (h = s_h; h < fz->fz_divisor; h++) {
- if (hlist_empty(&fz->fz_hash[h]))
+ if (hlist_empty(head + h))
continue;
- if (fn_hash_dump_bucket(skb, cb, tb, fz, &fz->fz_hash[h]) < 0) {
+ if (fn_hash_dump_bucket(skb, cb, tb, fz, head + h) < 0) {
cb->args[3] = h;
return -1;
}
@@ -872,7 +876,7 @@ static struct fib_alias *fib_get_first(struct seq_file *seq)
if (!iter->zone->fz_nent)
continue;
- iter->hash_head = iter->zone->fz_hash;
+ iter->hash_head = rcu_dereference(iter->zone->fz_hash);
maxslot = iter->zone->fz_divisor;
for (iter->bucket = 0; iter->bucket < maxslot;
@@ -957,7 +961,7 @@ static struct fib_alias *fib_get_next(struct seq_file *seq)
goto out;
iter->bucket = 0;
- iter->hash_head = iter->zone->fz_hash;
+ iter->hash_head = rcu_dereference(iter->zone->fz_hash);
hlist_for_each_entry(fn, node, iter->hash_head, fn_hash) {
list_for_each_entry(fa, &fn->fn_alias, fa_list) {
^ permalink raw reply related
* Re: [PATCH net-next-2.6] net: Fix some corner cases in dev_can_checksum()
From: Ben Hutchings @ 2010-10-26 13:29 UTC (permalink / raw)
To: Jesse Gross; +Cc: David Miller, netdev
In-Reply-To: <AANLkTi=PiB_oCvaFzQpUNhgV8qsn9d-jy_ejGdbOTzQe@mail.gmail.com>
Jesse Gross wrote:
> On Fri, Oct 22, 2010 at 7:12 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> > dev_can_checksum() incorrectly returns true in these cases:
> >
> > 1. The skb has both out-of-band and in-band VLAN tags and the device
> > supports checksum offload for the encapsulated protocol but only with
> > one layer of encapsulation.
> > 2. The skb has a VLAN tag and the device supports generic checksumming
> > but not in conjunction with VLAN encapsulation.
> >
> > Rearrange the VLAN tag checks to avoid these.
> >
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
>
> If we assume that cards cannot handle offloading for double tagged
> packets, which is obviously the most conservative approach, we
> probably also need to change the checks for TSO/SG. There's no issue
> with extracting the protocol from the right header but we might assume
> that the card can handle double tag offloading when it can't. For
> both TSO/SG we check if there is either an in-band tag or out-of-band
> tag and use dev->vlan_features if that is the case. Maybe we need to
> handle it in software if it is double tagged.
That's something to check.
> On the other hand, I don't know whether it's true that cards can't
> handle offloading for packets tagged in both manners. I suppose that
> it depends on where the offloading and tagging are in the pipeline.
> For example, when it comes to SG I doubt that the cards care about
> vlan tags much at all.
I do know that current Solarflare controllers can parse two VLAN tags
and generate/validate TCP/IP-style checksums after them. We could add
vlan2_features which would be copied to a VLAN sub-device's
vlan_features, but then what happens when people want to handle triple
VLAN encapsulation?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Joe Buehler @ 2010-10-26 13:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1287851745.2658.364.camel@edumazet-laptop>
Eric Dumazet wrote:
> With a normal workload, on a dual cpu machine, a missing memory barrier
> can stay un-noticed for quite a long time. The race window is so small
> that probability for the bug might be 0.0000001 % or something like
> that :(
I'm looking at the LINUX source at the moment and not liking what I see
in include/asm-mips/barrier.h:
#define smp_mb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
#ifdef CONFIG_CPU_CAVIUM_OCTEON
#define smp_rmb() barrier()
#define smp_wmb() barrier()
#else
#define smp_rmb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
#define smp_wmb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
#endif
The Octeon documentation explicitly says that neither loads nor stores
need execute in program order, so the definitions for smp_rmb and
smp_wmb appear to be wrong wrong wrong.
It appears that smp_wmb should be making use of SYNCW and smp_rmb should
be making use of SYNC.
Should I pursue this question on the main LINUX kernel list?
Joe Buehler
^ permalink raw reply
* Re: [PATCH net-next-2.6 1/2] be2net: Adding an option to use INTx instead of MSI-X
From: Ben Hutchings @ 2010-10-26 13:32 UTC (permalink / raw)
To: Michael Ellerman; +Cc: David Miller, somnath.kotur, netdev, linux-pci
In-Reply-To: <1288075928.6578.185.camel@concordia>
Michael Ellerman wrote:
> On Mon, 2010-10-25 at 16:25 -0700, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Mon, 25 Oct 2010 23:38:53 +0100
> >
> > > David Miller wrote:
> > >> From: Somnath Kotur <somnath.kotur@emulex.com>
> > >> Date: Mon, 25 Oct 2010 16:42:35 +0530
> > >>
> > >> > By default, be2net uses MSIx wherever possible.
> > >> > Adding a module parameter to use INTx for users who do not want to use MSIx.
> > >> >
> > >> > Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
> > >>
> > >> Either add a new ethtool flag, or use the PCI subsystem facilities
> > >> for tweaking things to implement this.
> > >>
> > >> Do not use a module option, otherwise every other networking driver
> > >> author will get the same "cool" idea, give the module option
> > >> different names, and the resulting user experience is terrible.
> > >
> > > This has already happened, sadly. So far as I can see it's mostly done
> > > to allow users to work around systems with broken MSIs; I'm not aware of
> > > any other reason to prefer legacy interrupts. However, the PCI subsystem
> > > already implements a blacklist and a kernel parameter for disabling MSIs
> > > on these systems.
> >
> > The PCI subsystem bits I'm totally fine with.
> >
> > But in the drivers themselves, that's what I don't want.
>
> That horse has really really bolted, it's gawn.
>
> I count 26 drivers with "disable MSI/X" parameters. Some even have more
> than one.
>
> 11 of them are network drivers, 9 scsi, 3 ata.
>
> I agree it's a mess for users, but it's probably preferable to a
> non-working driver.
>
> Ethtool would be nice, but only for network drivers. Is there a generic
> solution, quirks are obviously not keeping people happy.
Since this is (normally) a property of the system, pci=nomsi is the
generic solution.
[...]
> Misc:
> sound/pci/hda/hda_intel.c:MODULE_PARM_DESC(enable_msi, "Enable Message Signaled Interrupt (MSI)");
> drivers/message/fusion/mptbase.c:MODULE_PARM_DESC(mpt_msi_enable_spi, " Enable MSI Support for SPI \
> drivers/message/fusion/mptbase.c:MODULE_PARM_DESC(mpt_msi_enable_fc, " Enable MSI Support for FC \
> drivers/message/fusion/mptbase.c:MODULE_PARM_DESC(mpt_msi_enable_sas, " Enable MSI Support for SAS \
> drivers/net/myri10ge/myri10ge.c:MODULE_PARM_DESC(myri10ge_msi, "Enable Message Signalled Interrupts");
[...]
drivers/net/sfc/efx.c:MODULE_PARM_DESC(interrupt_mode,
drivers/net/sfc/efx.c- "Interrupt mode (0=>MSIX 1=>MSI 2=>legacy)");
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Eric Dumazet @ 2010-10-26 13:36 UTC (permalink / raw)
To: Joe Buehler; +Cc: netdev
In-Reply-To: <4CC6D7CC.5040608@cox.net>
Le mardi 26 octobre 2010 à 09:29 -0400, Joe Buehler a écrit :
> Eric Dumazet wrote:
>
> > With a normal workload, on a dual cpu machine, a missing memory barrier
> > can stay un-noticed for quite a long time. The race window is so small
> > that probability for the bug might be 0.0000001 % or something like
> > that :(
>
> I'm looking at the LINUX source at the moment and not liking what I see
> in include/asm-mips/barrier.h:
>
> #define smp_mb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
> #ifdef CONFIG_CPU_CAVIUM_OCTEON
> #define smp_rmb() barrier()
> #define smp_wmb() barrier()
> #else
> #define smp_rmb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
> #define smp_wmb() __asm__ __volatile__(__WEAK_ORDERING_MB : : :"memory")
> #endif
>
> The Octeon documentation explicitly says that neither loads nor stores
> need execute in program order, so the definitions for smp_rmb and
> smp_wmb appear to be wrong wrong wrong.
>
> It appears that smp_wmb should be making use of SYNCW and smp_rmb should
> be making use of SYNC.
>
> Should I pursue this question on the main LINUX kernel list?
Well, it would be surprising this being wrong and crash only once in a
while in fib_rules_lookup
Did you tried my last patch ?
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Joe Buehler @ 2010-10-26 13:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1288100208.3169.112.camel@edumazet-laptop>
Eric Dumazet wrote:
> Well, it would be surprising this being wrong and crash only once in a
> while in fib_rules_lookup
> Did you tried my last patch ?
There was a patch to the kernel by David Daney back in January to
improve performance of Octeon memory barriers. The patch changes the
generic MIPS barrier code to introduce optimizations for Octeon. The
LINUX version I am using is from the Octeon SDK and appears to have an
early version of this patch. It's broken however -- the Jan patch has
proper SYNCW instructions in smp_wmb while the SDK version does not.
I have made your changes but will also fold in this change, then start
some load testing.
The real-time scheduler is broken in the LINUX I am using -- I get
kernel crashes -- and I would be most happy if the SYNCW fix fixed that
also.
Joe Buehler
^ permalink raw reply
* Re: [PATCH 1/2] r6040: fix multicast operations
From: Ben Hutchings @ 2010-10-26 14:02 UTC (permalink / raw)
To: Shawn Lin
Cc: Florian Fainelli, netdev, Marc Leclerc, Albert Chen, David Miller
In-Reply-To: <1287649654.1792.98.camel@shawn-desktop>
[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]
On Thu, Oct 21, 2010 at 04:27:34PM +0800, Shawn Lin wrote:
[...]
> > > + /* Use internal multicast address registers
> > > + * if the number of multicast addresses is not greater than MCAST_MAX.
> > > + */
> > > + else if (netdev_mc_empty(dev)) {
> > > + for (i = 0; i < MCAST_MAX ; i++) {
> > > + iowrite16(0, ioaddr + MID_1L + 8 * i);
> > > + iowrite16(0, ioaddr + MID_1M + 8 * i);
> > > + iowrite16(0, ioaddr + MID_1H + 8 * i);
> > > + }
> > > + } else if (netdev_mc_count(dev) <= MCAST_MAX) {
> > > + i = 0;
> > > + netdev_for_each_mc_addr(ha, dev) {
> > > + adrp = (u16 *) ha->addr;
> > > + iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
> > > + iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
> > > + iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
> > > + i++;
> > > + }
> >
> > What about the unused exact match entries? And why is the empty case
> > special?
>
> Unused exact match entries? I am not sure which entries are you
> mentioned.
If there are 1 or 2 addresses in the multicast list then some of the
exact match entries will be used and some will not. But the loop
above does not clear the unused entries.
[...]
> 2) if (netdev_mc_count(dev) <= 3)
> There are two hardware features could be used to filter multicast
> frames:
[...]
> 3) if (netdev_mc_empty(dev))
> because we masked the multicast hash table flag before examine all
> conditions, we only need to clear the addresses in the three
> MAC/Multicast registers.
[...]
But why is this so different from the case of 1-3 addresses? I would
write these two cases as:
else if (netdev_mc_count(dev) <= MCAST_MAX) {
i = 0;
netdev_for_each_mc_addr(ha, dev) {
adrp = (u16 *) ha->addr;
iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
i++;
}
while (i < MCAST_MAX) {
iowrite16(0, ioaddr + MID_1L + 8 * i);
iowrite16(0, ioaddr + MID_1M + 8 * i);
iowrite16(0, ioaddr + MID_1H + 8 * i);
i++;
}
}
Ben.
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 827 bytes --]
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Eric Dumazet @ 2010-10-26 14:33 UTC (permalink / raw)
To: Joe Buehler; +Cc: netdev
In-Reply-To: <4CC6DD69.4020502@cox.net>
Le mardi 26 octobre 2010 à 09:53 -0400, Joe Buehler a écrit :
> Eric Dumazet wrote:
>
> > Well, it would be surprising this being wrong and crash only once in a
> > while in fib_rules_lookup
>
> > Did you tried my last patch ?
>
> There was a patch to the kernel by David Daney back in January to
> improve performance of Octeon memory barriers. The patch changes the
> generic MIPS barrier code to introduce optimizations for Octeon. The
> LINUX version I am using is from the Octeon SDK and appears to have an
> early version of this patch. It's broken however -- the Jan patch has
> proper SYNCW instructions in smp_wmb while the SDK version does not.
>
> I have made your changes but will also fold in this change, then start
> some load testing.
>
> The real-time scheduler is broken in the LINUX I am using -- I get
> kernel crashes -- and I would be most happy if the SYNCW fix fixed that
> also.
>
>
Just to make sure, are you using a single syncw, or a double one ?
/*
* We actually use two syncw instructions in a row when we need a write
* memory barrier. This is because the CN3XXX series of Octeons have
* errata Core-401. This can cause a single syncw to not enforce
* ordering under very rare conditions. Even if it is rare, better safe
* than sorry.
*/
#define CVMX_SYNCW asm volatile ("syncw\n\tsyncw" : : : "memory")
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Joe Buehler @ 2010-10-26 14:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1288103614.2622.0.camel@edumazet-laptop>
Eric Dumazet wrote:
> Just to make sure, are you using a single syncw, or a double one ?
I'm using double. I know about the errata. It doesn't apply to all
Octeon versions but in their SDK they always use two without worrying
about the chip variant.
What I actually to patch this was look at the current LINUX prerelease
and copy what it does.
We'll see later today if all these changes fix my crash, it takes a
while to encounter it.
Joe Buehler
^ permalink raw reply
* Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched]
From: Eric Dumazet @ 2010-10-26 13:58 UTC (permalink / raw)
To: Joe Buehler; +Cc: netdev
In-Reply-To: <4CC6DD69.4020502@cox.net>
Le mardi 26 octobre 2010 à 09:53 -0400, Joe Buehler a écrit :
> Eric Dumazet wrote:
>
> > Well, it would be surprising this being wrong and crash only once in a
> > while in fib_rules_lookup
>
> > Did you tried my last patch ?
>
> There was a patch to the kernel by David Daney back in January to
> improve performance of Octeon memory barriers. The patch changes the
> generic MIPS barrier code to introduce optimizations for Octeon. The
> LINUX version I am using is from the Octeon SDK and appears to have an
> early version of this patch. It's broken however -- the Jan patch has
> proper SYNCW instructions in smp_wmb while the SDK version does not.
>
> I have made your changes but will also fold in this change, then start
> some load testing.
>
> The real-time scheduler is broken in the LINUX I am using -- I get
> kernel crashes -- and I would be most happy if the SYNCW fix fixed that
> also.
>
> Joe Buehler
>
Just to make sure, you do use two "syncw", not only one ?
/*
* We actually use two syncw instructions in a row when we need a write
* memory barrier. This is because the CN3XXX series of Octeons have
* errata Core-401. This can cause a single syncw to not enforce
* ordering under very rare conditions. Even if it is rare, better safe
* than sorry.
*/
#define CVMX_SYNCW asm volatile ("syncw\n\tsyncw" : : : "memory")
^ permalink raw reply
* Re: [PATCH] ipv6: addrconf: clear IPv6 addresses and routes when losing link
From: Stephen Hemminger @ 2010-10-26 15:28 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: netdev
In-Reply-To: <AANLkTinGpt=EQX3aXRaj5mEUMDNpyEapbs4VyOwgtN55@mail.gmail.com>
On Mon, 25 Oct 2010 22:44:03 -0700
Lorenzo Colitti <lorenzo@google.com> wrote:
> On Mon, Oct 25, 2010 at 9:38 PM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
> > This is incorrect. When link is lost, routes and address should not be
> > flushed. They should be marked as tentative and then go through DAD again
> > on the new network.
>
> That won't help the case I am trying to fix, which is the case where
> the new link has a global prefix different than the old link. Marking
> the addresses as tentative will simply make them pass DAD and come
> back as soon as link comes back. But since they don't match the prefix
> that is assigned to the new link, they are unusable, because packets
> can't be routed back to them.
For IPv4 this is already handled by network manager.
Why couldn't the same apply to IPv6?
> > If you do it this way, you break routing protocols when link is brought
> > down and back up.
>
> The only addresses and routes flushed in this way should be ones that
> aren't manually configured, i.e., the ones created by autoconf
> (addrconf.c:2720 onwards). These won't be used by routing protocols,
> except for link-local addresses. So I assume you're talking about
> link-local here.
Not sure, let me do test it.
> Link-local addresses are immediately recreated in a tentative state as
> soon as link comes back, because on NETDEV_UP addrconf_notify calls
> addrconf_dev_config. So, this patch only makes it so that they become
> tentative when link goes away and comes back. In that time, the router
> that temporarily loses link is unable to send packets for the brief
> period of time that the link is performing DAD, but if the router has
> lost link, it will also fail to send the packet while link is lost.
> What's the additional failure scenario? Will it help if I make it so
> that link-local addresses aren't touched at all?
Link-local works fine.
--
^ permalink raw reply
* Re: [PATCH 1/2 v3] xps: Improvements in TX queue selection
From: Tom Herbert @ 2010-10-26 15:32 UTC (permalink / raw)
To: Helmut Schaa; +Cc: davem, netdev, eric.dumazet
In-Reply-To: <201010260818.47523.helmut.schaa@googlemail.com>
> Wouldn't that break mac80211 QoS again for bridged AP mode interfaces (see
> commit deabc772f39405054a438d711f408d2d94d26d96, "net: fix tx queue selection
> for bridged devices implementing select_queue")?
>
Yes, looks like that would break. I'll fix that.
If a device only has one real TX queue should we still call
ndo_select_queue, or can we bypass it? (to save one conditional)
Thanks,
Tom
^ permalink raw reply
* Re: [PATCH 1/2 v3] xps: Improvements in TX queue selection
From: David Miller @ 2010-10-26 15:35 UTC (permalink / raw)
To: therbert; +Cc: helmut.schaa, netdev, eric.dumazet
In-Reply-To: <AANLkTimMrqhWbnLXk1xbYPY7WT7Gazn5G=S1QvVkZ86Y@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Tue, 26 Oct 2010 08:32:59 -0700
>> Wouldn't that break mac80211 QoS again for bridged AP mode interfaces (see
>> commit deabc772f39405054a438d711f408d2d94d26d96, "net: fix tx queue selection
>> for bridged devices implementing select_queue")?
>>
> Yes, looks like that would break. I'll fix that.
>
> If a device only has one real TX queue should we still call
> ndo_select_queue, or can we bypass it? (to save one conditional)
Probably bypass, for now.
^ permalink raw reply
* Re: [PATCH 0/4]qlcnic: bug fixes
From: David Miller @ 2010-10-26 15:37 UTC (permalink / raw)
To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1288085882-11988-1-git-send-email-amit.salecha@qlogic.com>
From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Tue, 26 Oct 2010 02:37:58 -0700
> Hi
> Series of 4 bug fixes. Apply them on net-2.6 branch.
Your patches weren't numbered, making the order I should
apply them ambiguous.
Please fix the subject lines and resubmit.
Thank you.
^ permalink raw reply
* Re: linux-next: Tree for October 25 (netfilter/nf_conntrack_reasm)
From: David Miller @ 2010-10-26 16:09 UTC (permalink / raw)
To: randy.dunlap; +Cc: sfr, netfilter-devel, netdev, linux-next, linux-kernel
In-Reply-To: <20101025.220316.39174493.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 25 Oct 2010 22:03:16 -0700 (PDT)
> From: Randy Dunlap <randy.dunlap@oracle.com>
> Date: Mon, 25 Oct 2010 16:55:29 -0700
>
>> On Mon, 25 Oct 2010 14:58:34 +1100 Stephen Rothwell wrote:
>>
>>> Hi all,
>>>
>>> Reminder: do not add 2.6.38 destined stuff to linux-next until after
>>> 2.6.37-rc1 is released.
>>
>> net/ipv6/netfilter/nf_conntrack_reasm.c:628: error: 'nf_ct_frag6_sysctl_header' undeclared (first use in this function)
>> net/ipv6/netfilter/nf_conntrack_reasm.c:628: error: 'nf_net_netfilter_sysctl_path' undeclared (first use in this function)
>> net/ipv6/netfilter/nf_conntrack_reasm.c:629: error: 'nf_ct_frag6_sysctl_table' undeclared (first use in this function)
>> net/ipv6/netfilter/nf_conntrack_reasm.c:640: error: 'nf_ct_frag6_sysctl_header' undeclared (first use in this function)
>>
>>
>> config file is attached.
>
> Should also be fixed by the commit I just pointed you to.
Actually it isn't :-) I'll commit the following to fix this,
thanks!
--------------------
netfilter: Add missing CONFIG_SYSCTL checks in ipv6's nf_conntrack_reasm.c
Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv6/netfilter/nf_conntrack_reasm.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 489d71b..3a3f129 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -625,21 +625,24 @@ int nf_ct_frag6_init(void)
inet_frags_init_net(&nf_init_frags);
inet_frags_init(&nf_frags);
+#ifdef CONFIG_SYSCTL
nf_ct_frag6_sysctl_header = register_sysctl_paths(nf_net_netfilter_sysctl_path,
nf_ct_frag6_sysctl_table);
if (!nf_ct_frag6_sysctl_header) {
inet_frags_fini(&nf_frags);
return -ENOMEM;
}
+#endif
return 0;
}
void nf_ct_frag6_cleanup(void)
{
+#ifdef CONFIG_SYSCTL
unregister_sysctl_table(nf_ct_frag6_sysctl_header);
nf_ct_frag6_sysctl_header = NULL;
-
+#endif
inet_frags_fini(&nf_frags);
nf_init_frags.low_thresh = 0;
--
1.7.3.2
^ permalink raw reply related
* [PATCH] netfilter: Add missing CONFIG_SYSCTL checks in ipv6's nf_conntrack_reasm.c
From: David Miller @ 2010-10-26 16:10 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, kaber
Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
I just committed this to net-2.6, just FYI.
net/ipv6/netfilter/nf_conntrack_reasm.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 489d71b..3a3f129 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -625,21 +625,24 @@ int nf_ct_frag6_init(void)
inet_frags_init_net(&nf_init_frags);
inet_frags_init(&nf_frags);
+#ifdef CONFIG_SYSCTL
nf_ct_frag6_sysctl_header = register_sysctl_paths(nf_net_netfilter_sysctl_path,
nf_ct_frag6_sysctl_table);
if (!nf_ct_frag6_sysctl_header) {
inet_frags_fini(&nf_frags);
return -ENOMEM;
}
+#endif
return 0;
}
void nf_ct_frag6_cleanup(void)
{
+#ifdef CONFIG_SYSCTL
unregister_sysctl_table(nf_ct_frag6_sysctl_header);
nf_ct_frag6_sysctl_header = NULL;
-
+#endif
inet_frags_fini(&nf_frags);
nf_init_frags.low_thresh = 0;
--
1.7.3.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox