* Re: [v3 RFC PATCH 0/4] Implement multiqueue virtio-net
From: Michael S. Tsirkin @ 2010-10-26 9:38 UTC (permalink / raw)
To: Krishna Kumar2
Cc: anthony, arnd, avi, davem, eric.dumazet, kvm, netdev, rusty
In-Reply-To: <OFBB51646E.01CC3630-ON652577C8.0030D6EF-652577C8.0031DA3C@in.ibm.com>
On Tue, Oct 26, 2010 at 02:38:53PM +0530, Krishna Kumar2 wrote:
> Results for UDP BW tests (unidirectional, sum across
> 3 iterations, each iteration of 45 seconds, default
> netperf, vhosts bound to cpus 0-3; no other tuning):
Is binding vhost threads to CPUs really required?
What happens if we let the scheduler do its job?
--
MST
^ permalink raw reply
* Re: [PATCH] qlcnic: dma address align check
From: Eric Dumazet @ 2010-10-26 9:54 UTC (permalink / raw)
To: Amit Kumar Salecha; +Cc: davem, netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1288085882-11988-5-git-send-email-amit.salecha@qlogic.com>
Le mardi 26 octobre 2010 à 02:38 -0700, Amit Kumar Salecha a écrit :
> Device requires tx_hw_cosnumer to be 64 byte aligned.
> Tx desc size is 64 byte, alloc tx_hw_consumer with tx desc.
>
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
> drivers/net/qlcnic/qlcnic_ctx.c | 35 +++++++++++++++--------------------
> 1 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
> index 1cdc05d..21c9c28 100644
> --- a/drivers/net/qlcnic/qlcnic_ctx.c
> +++ b/drivers/net/qlcnic/qlcnic_ctx.c
> @@ -418,18 +418,9 @@ int qlcnic_alloc_hw_resources(struct qlcnic_adapter *adapter)
> recv_ctx = &adapter->recv_ctx;
> tx_ring = adapter->tx_ring;
>
> - tx_ring->hw_consumer = (__le32 *)pci_alloc_consistent(pdev, sizeof(u32),
> - &tx_ring->hw_cons_phys_addr);
> - if (tx_ring->hw_consumer == NULL) {
> - dev_err(&pdev->dev, "failed to allocate tx consumer\n");
> - return -ENOMEM;
> - }
> - *(tx_ring->hw_consumer) = 0;
> -
> /* cmd desc ring */
> - addr = pci_alloc_consistent(pdev, TX_DESC_RINGSIZE(tx_ring),
> - &tx_ring->phys_addr);
> -
> + addr = pci_alloc_consistent(pdev, TX_DESC_RINGSIZE(tx_ring) +
> + sizeof(u32), &tx_ring->phys_addr);
Wont this use twice memory than before, due to power-of-two
allocations ?
Allocating 65536 + 4 bytes gives you 131072 bytes.
^ permalink raw reply
* [PATCH] inetpeer: __rcu annotations
From: Eric Dumazet @ 2010-10-26 9:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Adds __rcu annotations to inetpeer
(struct inet_peer)->avl_left
(struct inet_peer)->avl_right
This is a tedious cleanup, but removes one smp_wmb() from link_to_pool()
since we now use more self documenting rcu_assign_pointer().
Note the use of RCU_INIT_POINTER() instead of rcu_assign_pointer() in
all cases we dont need a memory barrier.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/inetpeer.h | 2
net/ipv4/inetpeer.c | 138 ++++++++++++++++++++++-----------------
2 files changed, 81 insertions(+), 59 deletions(-)
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 417d0c8..fe239bf 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -15,7 +15,7 @@
struct inet_peer {
/* group together avl_left,avl_right,v4daddr to speedup lookups */
- struct inet_peer *avl_left, *avl_right;
+ struct inet_peer __rcu *avl_left, *avl_right;
__be32 v4daddr; /* peer's address */
__u32 avl_height;
struct list_head unused;
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 9ffa24b..ccf01c9 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -72,18 +72,19 @@ static struct kmem_cache *peer_cachep __read_mostly;
#define node_height(x) x->avl_height
#define peer_avl_empty ((struct inet_peer *)&peer_fake_node)
+#define peer_avl_empty_rcu ((struct inet_peer __rcu __force *)&peer_fake_node)
static const struct inet_peer peer_fake_node = {
- .avl_left = peer_avl_empty,
- .avl_right = peer_avl_empty,
+ .avl_left = peer_avl_empty_rcu,
+ .avl_right = peer_avl_empty_rcu,
.avl_height = 0
};
static struct {
- struct inet_peer *root;
+ struct inet_peer __rcu *root;
spinlock_t lock;
int total;
} peers = {
- .root = peer_avl_empty,
+ .root = peer_avl_empty_rcu,
.lock = __SPIN_LOCK_UNLOCKED(peers.lock),
.total = 0,
};
@@ -156,11 +157,14 @@ static void unlink_from_unused(struct inet_peer *p)
*/
#define lookup(_daddr, _stack) \
({ \
- struct inet_peer *u, **v; \
+ struct inet_peer *u; \
+ struct inet_peer __rcu **v; \
\
stackptr = _stack; \
*stackptr++ = &peers.root; \
- for (u = peers.root; u != peer_avl_empty; ) { \
+ for (u = rcu_dereference_protected(peers.root, \
+ lockdep_is_held(&peers.lock)); \
+ u != peer_avl_empty; ) { \
if (_daddr == u->v4daddr) \
break; \
if ((__force __u32)_daddr < (__force __u32)u->v4daddr) \
@@ -168,7 +172,8 @@ static void unlink_from_unused(struct inet_peer *p)
else \
v = &u->avl_right; \
*stackptr++ = v; \
- u = *v; \
+ u = rcu_dereference_protected(*v, \
+ lockdep_is_held(&peers.lock)); \
} \
u; \
})
@@ -209,13 +214,17 @@ static struct inet_peer *lookup_rcu_bh(__be32 daddr)
/* Called with local BH disabled and the pool lock held. */
#define lookup_rightempty(start) \
({ \
- struct inet_peer *u, **v; \
+ struct inet_peer *u; \
+ struct inet_peer __rcu **v; \
*stackptr++ = &start->avl_left; \
v = &start->avl_left; \
- for (u = *v; u->avl_right != peer_avl_empty; ) { \
+ for (u = rcu_dereference_protected(*v, \
+ lockdep_is_held(&peers.lock)); \
+ u->avl_right != peer_avl_empty_rcu; ) { \
v = &u->avl_right; \
*stackptr++ = v; \
- u = *v; \
+ u = rcu_dereference_protected(*v, \
+ lockdep_is_held(&peers.lock)); \
} \
u; \
})
@@ -224,74 +233,86 @@ static struct inet_peer *lookup_rcu_bh(__be32 daddr)
* Variable names are the proof of operation correctness.
* Look into mm/map_avl.c for more detail description of the ideas.
*/
-static void peer_avl_rebalance(struct inet_peer **stack[],
- struct inet_peer ***stackend)
+static void peer_avl_rebalance(struct inet_peer __rcu **stack[],
+ struct inet_peer __rcu ***stackend)
{
- struct inet_peer **nodep, *node, *l, *r;
+ struct inet_peer __rcu **nodep;
+ struct inet_peer *node, *l, *r;
int lh, rh;
while (stackend > stack) {
nodep = *--stackend;
- node = *nodep;
- l = node->avl_left;
- r = node->avl_right;
+ node = rcu_dereference_protected(*nodep,
+ lockdep_is_held(&peers.lock));
+ l = rcu_dereference_protected(node->avl_left,
+ lockdep_is_held(&peers.lock));
+ r = rcu_dereference_protected(node->avl_right,
+ lockdep_is_held(&peers.lock));
lh = node_height(l);
rh = node_height(r);
if (lh > rh + 1) { /* l: RH+2 */
struct inet_peer *ll, *lr, *lrl, *lrr;
int lrh;
- ll = l->avl_left;
- lr = l->avl_right;
+ ll = rcu_dereference_protected(l->avl_left,
+ lockdep_is_held(&peers.lock));
+ lr = rcu_dereference_protected(l->avl_right,
+ lockdep_is_held(&peers.lock));
lrh = node_height(lr);
if (lrh <= node_height(ll)) { /* ll: RH+1 */
- node->avl_left = lr; /* lr: RH or RH+1 */
- node->avl_right = r; /* r: RH */
+ RCU_INIT_POINTER(node->avl_left, lr); /* lr: RH or RH+1 */
+ RCU_INIT_POINTER(node->avl_right, r); /* r: RH */
node->avl_height = lrh + 1; /* RH+1 or RH+2 */
- l->avl_left = ll; /* ll: RH+1 */
- l->avl_right = node; /* node: RH+1 or RH+2 */
+ RCU_INIT_POINTER(l->avl_left, ll); /* ll: RH+1 */
+ RCU_INIT_POINTER(l->avl_right, node); /* node: RH+1 or RH+2 */
l->avl_height = node->avl_height + 1;
- *nodep = l;
+ RCU_INIT_POINTER(*nodep, l);
} else { /* ll: RH, lr: RH+1 */
- lrl = lr->avl_left; /* lrl: RH or RH-1 */
- lrr = lr->avl_right; /* lrr: RH or RH-1 */
- node->avl_left = lrr; /* lrr: RH or RH-1 */
- node->avl_right = r; /* r: RH */
+ lrl = rcu_dereference_protected(lr->avl_left,
+ lockdep_is_held(&peers.lock)); /* lrl: RH or RH-1 */
+ lrr = rcu_dereference_protected(lr->avl_right,
+ lockdep_is_held(&peers.lock)); /* lrr: RH or RH-1 */
+ RCU_INIT_POINTER(node->avl_left, lrr); /* lrr: RH or RH-1 */
+ RCU_INIT_POINTER(node->avl_right, r); /* r: RH */
node->avl_height = rh + 1; /* node: RH+1 */
- l->avl_left = ll; /* ll: RH */
- l->avl_right = lrl; /* lrl: RH or RH-1 */
+ RCU_INIT_POINTER(l->avl_left, ll); /* ll: RH */
+ RCU_INIT_POINTER(l->avl_right, lrl); /* lrl: RH or RH-1 */
l->avl_height = rh + 1; /* l: RH+1 */
- lr->avl_left = l; /* l: RH+1 */
- lr->avl_right = node; /* node: RH+1 */
+ RCU_INIT_POINTER(lr->avl_left, l); /* l: RH+1 */
+ RCU_INIT_POINTER(lr->avl_right, node); /* node: RH+1 */
lr->avl_height = rh + 2;
- *nodep = lr;
+ RCU_INIT_POINTER(*nodep, lr);
}
} else if (rh > lh + 1) { /* r: LH+2 */
struct inet_peer *rr, *rl, *rlr, *rll;
int rlh;
- rr = r->avl_right;
- rl = r->avl_left;
+ rr = rcu_dereference_protected(r->avl_right,
+ lockdep_is_held(&peers.lock));
+ rl = rcu_dereference_protected(r->avl_left,
+ lockdep_is_held(&peers.lock));
rlh = node_height(rl);
if (rlh <= node_height(rr)) { /* rr: LH+1 */
- node->avl_right = rl; /* rl: LH or LH+1 */
- node->avl_left = l; /* l: LH */
+ RCU_INIT_POINTER(node->avl_right, rl); /* rl: LH or LH+1 */
+ RCU_INIT_POINTER(node->avl_left, l); /* l: LH */
node->avl_height = rlh + 1; /* LH+1 or LH+2 */
- r->avl_right = rr; /* rr: LH+1 */
- r->avl_left = node; /* node: LH+1 or LH+2 */
+ RCU_INIT_POINTER(r->avl_right, rr); /* rr: LH+1 */
+ RCU_INIT_POINTER(r->avl_left, node); /* node: LH+1 or LH+2 */
r->avl_height = node->avl_height + 1;
- *nodep = r;
+ RCU_INIT_POINTER(*nodep, r);
} else { /* rr: RH, rl: RH+1 */
- rlr = rl->avl_right; /* rlr: LH or LH-1 */
- rll = rl->avl_left; /* rll: LH or LH-1 */
- node->avl_right = rll; /* rll: LH or LH-1 */
- node->avl_left = l; /* l: LH */
+ rlr = rcu_dereference_protected(rl->avl_right,
+ lockdep_is_held(&peers.lock)); /* rlr: LH or LH-1 */
+ rll = rcu_dereference_protected(rl->avl_left,
+ lockdep_is_held(&peers.lock)); /* rll: LH or LH-1 */
+ RCU_INIT_POINTER(node->avl_right, rll); /* rll: LH or LH-1 */
+ RCU_INIT_POINTER(node->avl_left, l); /* l: LH */
node->avl_height = lh + 1; /* node: LH+1 */
- r->avl_right = rr; /* rr: LH */
- r->avl_left = rlr; /* rlr: LH or LH-1 */
+ RCU_INIT_POINTER(r->avl_right, rr); /* rr: LH */
+ RCU_INIT_POINTER(r->avl_left, rlr); /* rlr: LH or LH-1 */
r->avl_height = lh + 1; /* r: LH+1 */
- rl->avl_right = r; /* r: LH+1 */
- rl->avl_left = node; /* node: LH+1 */
+ RCU_INIT_POINTER(rl->avl_right, r); /* r: LH+1 */
+ RCU_INIT_POINTER(rl->avl_left, node); /* node: LH+1 */
rl->avl_height = lh + 2;
- *nodep = rl;
+ RCU_INIT_POINTER(*nodep, rl);
}
} else {
node->avl_height = (lh > rh ? lh : rh) + 1;
@@ -303,10 +324,10 @@ static void peer_avl_rebalance(struct inet_peer **stack[],
#define link_to_pool(n) \
do { \
n->avl_height = 1; \
- n->avl_left = peer_avl_empty; \
- n->avl_right = peer_avl_empty; \
- smp_wmb(); /* lockless readers can catch us now */ \
- **--stackptr = n; \
+ n->avl_left = peer_avl_empty_rcu; \
+ n->avl_right = peer_avl_empty_rcu; \
+ /* lockless readers can catch us now */ \
+ rcu_assign_pointer(**--stackptr, n); \
peer_avl_rebalance(stack, stackptr); \
} while (0)
@@ -330,24 +351,25 @@ static void unlink_from_pool(struct inet_peer *p)
* We use refcnt=-1 to alert lockless readers this entry is deleted.
*/
if (atomic_cmpxchg(&p->refcnt, 1, -1) == 1) {
- struct inet_peer **stack[PEER_MAXDEPTH];
- struct inet_peer ***stackptr, ***delp;
+ struct inet_peer __rcu **stack[PEER_MAXDEPTH];
+ struct inet_peer __rcu ***stackptr, ***delp;
if (lookup(p->v4daddr, stack) != p)
BUG();
delp = stackptr - 1; /* *delp[0] == p */
- if (p->avl_left == peer_avl_empty) {
+ if (p->avl_left == peer_avl_empty_rcu) {
*delp[0] = p->avl_right;
--stackptr;
} else {
/* look for a node to insert instead of p */
struct inet_peer *t;
t = lookup_rightempty(p);
- BUG_ON(*stackptr[-1] != t);
+ BUG_ON(rcu_dereference_protected(*stackptr[-1],
+ lockdep_is_held(&peers.lock)) != t);
**--stackptr = t->avl_left;
/* t is removed, t->v4daddr > x->v4daddr for any
* x in p->avl_left subtree.
* Put t in the old place of p. */
- *delp[0] = t;
+ RCU_INIT_POINTER(*delp[0], t);
t->avl_left = p->avl_left;
t->avl_right = p->avl_right;
t->avl_height = p->avl_height;
@@ -414,7 +436,7 @@ static int cleanup_once(unsigned long ttl)
struct inet_peer *inet_getpeer(__be32 daddr, int create)
{
struct inet_peer *p;
- struct inet_peer **stack[PEER_MAXDEPTH], ***stackptr;
+ struct inet_peer __rcu **stack[PEER_MAXDEPTH], ***stackptr;
/* Look up for the address quickly, lockless.
* Because of a concurrent writer, we might not find an existing entry.
^ permalink raw reply related
* Re: [v3 RFC PATCH 0/4] Implement multiqueue virtio-net
From: Krishna Kumar2 @ 2010-10-26 10:01 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: anthony, arnd, avi, davem, eric.dumazet, kvm, netdev,
netdev-owner, rusty
In-Reply-To: <20101026093846.GA6766@redhat.com>
> "Michael S. Tsirkin" <mst@redhat.com>
>
> On Tue, Oct 26, 2010 at 02:38:53PM +0530, Krishna Kumar2 wrote:
> > Results for UDP BW tests (unidirectional, sum across
> > 3 iterations, each iteration of 45 seconds, default
> > netperf, vhosts bound to cpus 0-3; no other tuning):
>
> Is binding vhost threads to CPUs really required?
> What happens if we let the scheduler do its job?
Nothing drastic, I remember BW% and SD% both improved a
bit as a result of binding. I started binding vhost thread
after Avi suggested it in response to my v1 patch (he
suggested some more that I haven't done), and have been
doing only this tuning ever since. This is part of his
mail for the tuning:
> vhost:
> thread #0: CPU0
> thread #1: CPU1
> thread #2: CPU2
> thread #3: CPU3
I simply bound each thread to CPU0-3 instead.
Thanks,
- KK
^ permalink raw reply
* RE: [PATCH] qlcnic: dma address align check
From: Amit Salecha @ 2010-10-26 10:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ameen Rahman,
Anirban Chakraborty
In-Reply-To: <1288086894.3169.53.camel@edumazet-laptop>
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Tuesday, October 26, 2010 3:25 PM
> To: Amit Salecha
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Ameen Rahman; Anirban
> Chakraborty
> Subject: Re: [PATCH] qlcnic: dma address align check
>
> Le mardi 26 octobre 2010 à 02:38 -0700, Amit Kumar Salecha a écrit :
> > Device requires tx_hw_cosnumer to be 64 byte aligned.
> > Tx desc size is 64 byte, alloc tx_hw_consumer with tx desc.
> >
> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> > ---
> > drivers/net/qlcnic/qlcnic_ctx.c | 35 +++++++++++++++--------------
> ------
> > 1 files changed, 15 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/net/qlcnic/qlcnic_ctx.c
> b/drivers/net/qlcnic/qlcnic_ctx.c
> > index 1cdc05d..21c9c28 100644
> > --- a/drivers/net/qlcnic/qlcnic_ctx.c
> > +++ b/drivers/net/qlcnic/qlcnic_ctx.c
> > @@ -418,18 +418,9 @@ int qlcnic_alloc_hw_resources(struct
> qlcnic_adapter *adapter)
> > recv_ctx = &adapter->recv_ctx;
> > tx_ring = adapter->tx_ring;
> >
> > - tx_ring->hw_consumer = (__le32 *)pci_alloc_consistent(pdev,
> sizeof(u32),
> > - &tx_ring->hw_cons_phys_addr);
> > - if (tx_ring->hw_consumer == NULL) {
> > - dev_err(&pdev->dev, "failed to allocate tx consumer\n");
> > - return -ENOMEM;
> > - }
> > - *(tx_ring->hw_consumer) = 0;
> > -
> > /* cmd desc ring */
> > - addr = pci_alloc_consistent(pdev, TX_DESC_RINGSIZE(tx_ring),
> > - &tx_ring->phys_addr);
> > -
> > + addr = pci_alloc_consistent(pdev, TX_DESC_RINGSIZE(tx_ring) +
> > + sizeof(u32), &tx_ring->phys_addr);
>
> Wont this use twice memory than before, due to power-of-two
> allocations ?
>
> Allocating 65536 + 4 bytes gives you 131072 bytes.
>
Is it ? I am not aware about such calculation.
Is pci_alloc_consistent guarantee to give PAGE align dma address ?
-Amit
^ permalink raw reply
* [PATCH 3/14] drivers/vhost/vhost.c: delete double assignment
From: Julia Lawall @ 2010-10-26 10:25 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtualization, kernel-janitors, linux-kernel, kvm, netdev
In-Reply-To: <1288088743-3725-1-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Delete successive assignments to the same location.
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>
---
drivers/vhost/vhost.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 94701ff..ed27727 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->avail_idx = 0;
vq->last_used_idx = 0;
vq->used_flags = 0;
- vq->used_flags = 0;
vq->log_used = false;
vq->log_addr = -1ull;
vq->vhost_hlen = 0;
^ permalink raw reply related
* [PATCH 5/14] drivers/net/sb1000.c: delete double assignment
From: Julia Lawall @ 2010-10-26 10:25 UTC (permalink / raw)
To: netdev; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <1288088743-3725-1-git-send-email-julia@diku.dk>
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;
^ permalink raw reply related
* [PATCH 7/14] drivers/net/typhoon.c: delete double assignment
From: Julia Lawall @ 2010-10-26 10:25 UTC (permalink / raw)
To: David Dillow; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1288088743-3725-1-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Delete successive assignments to the same location. The current definition
does not initialize the respRing structure, which has the same type as the
cmdRing structure, so initialize that one instead.
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/typhoon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index 1cc6713..fc014eb 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -1328,7 +1328,7 @@ typhoon_init_rings(struct typhoon *tp)
tp->rxHiRing.lastWrite = 0;
tp->rxBuffRing.lastWrite = 0;
tp->cmdRing.lastWrite = 0;
- tp->cmdRing.lastWrite = 0;
+ tp->respRing.lastWrite = 0;
tp->txLoRing.lastRead = 0;
tp->txHiRing.lastRead = 0;
^ permalink raw reply related
* [PATCH 10/14] drivers/isdn: delete double assignment
From: Julia Lawall @ 2010-10-26 10:25 UTC (permalink / raw)
To: Karsten Keil; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1288088743-3725-1-git-send-email-julia@diku.dk>
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))) {
^ permalink raw reply related
* RE: [PATCH] qlcnic: dma address align check
From: Eric Dumazet @ 2010-10-26 10:30 UTC (permalink / raw)
To: Amit Salecha
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ameen Rahman,
Anirban Chakraborty
In-Reply-To: <99737F4847ED0A48AECC9F4A1974A4B80F871A817A@MNEXMB2.qlogic.org>
Le mardi 26 octobre 2010 à 05:04 -0500, Amit Salecha a écrit :
> Is it ? I am not aware about such calculation.
Yes it is
> Is pci_alloc_consistent guarantee to give PAGE align dma address ?
I believe so
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)
If TX_DESC_RINGSIZE(tx_ring) is not a power of two, then yes you could
probably add 64 bytes and avoid allocating a full page only for the u32
field ;)
^ permalink raw reply
* RE: [PATCH] qlcnic: dma address align check
From: Amit Salecha @ 2010-10-26 10:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem@davemloft.net, netdev@vger.kernel.org, Ameen Rahman,
Anirban Chakraborty
In-Reply-To: <1288089033.3169.73.camel@edumazet-laptop>
> > Is pci_alloc_consistent guarantee to give PAGE align dma address ?
>
> I believe so
>
> 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)
>
>
> If TX_DESC_RINGSIZE(tx_ring) is not a power of two, then yes you could
> probably add 64 bytes and avoid allocating a full page only for the u32
> field ;)
>
>
TX_DESC_RINGSIZE is power of two.
If pci_alloc_consistent guarantee to give PAGE align dma address, then this patch is not required.
>
^ permalink raw reply
* Re: [PATCH 3/14] drivers/vhost/vhost.c: delete double assignment
From: Michael S. Tsirkin @ 2010-10-26 11:06 UTC (permalink / raw)
To: Julia Lawall; +Cc: virtualization, kernel-janitors, linux-kernel, kvm, netdev
In-Reply-To: <1288088743-3725-4-git-send-email-julia@diku.dk>
On Tue, Oct 26, 2010 at 12:25:32PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location.
>
> 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>
Thanks, applied.
> ---
> drivers/vhost/vhost.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 94701ff..ed27727 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->avail_idx = 0;
> vq->last_used_idx = 0;
> vq->used_flags = 0;
> - vq->used_flags = 0;
> vq->log_used = false;
> vq->log_addr = -1ull;
> vq->vhost_hlen = 0;
^ permalink raw reply
* Re: [v3 RFC PATCH 0/4] Implement multiqueue virtio-net
From: Michael S. Tsirkin @ 2010-10-26 11:09 UTC (permalink / raw)
To: Krishna Kumar2
Cc: anthony, arnd, avi, davem, eric.dumazet, kvm, netdev,
netdev-owner, rusty
In-Reply-To: <OFBE9E2C99.648EFB19-ON652577C8.0035FEDD-652577C8.0036AD51@in.ibm.com>
On Tue, Oct 26, 2010 at 03:31:39PM +0530, Krishna Kumar2 wrote:
> > "Michael S. Tsirkin" <mst@redhat.com>
> >
> > On Tue, Oct 26, 2010 at 02:38:53PM +0530, Krishna Kumar2 wrote:
> > > Results for UDP BW tests (unidirectional, sum across
> > > 3 iterations, each iteration of 45 seconds, default
> > > netperf, vhosts bound to cpus 0-3; no other tuning):
> >
> > Is binding vhost threads to CPUs really required?
> > What happens if we let the scheduler do its job?
>
> Nothing drastic, I remember BW% and SD% both improved a
> bit as a result of binding.
If there's a significant improvement this would mean that
we need to rethink the vhost-net interaction with the scheduler.
> I started binding vhost thread
> after Avi suggested it in response to my v1 patch (he
> suggested some more that I haven't done), and have been
> doing only this tuning ever since. This is part of his
> mail for the tuning:
>
> > vhost:
> > thread #0: CPU0
> > thread #1: CPU1
> > thread #2: CPU2
> > thread #3: CPU3
>
> I simply bound each thread to CPU0-3 instead.
>
> Thanks,
>
> - KK
^ permalink raw reply
* Reply Needed
From: Dr Carl Lee @ 2010-10-26 13:21 UTC (permalink / raw)
To: netdev
Hello,
I have a proposition for you, this however is not mandatory nor will I
in any manner compel you to honor against your will.Let me start by
introducing myself. I am Dr.Carl Lee, Director of Operations
of the Hang Seng Bank Ltd,Sai Wan Ho Branch. I have a mutual beneficial
business suggestion for you.
1. Can you handle this project?
2. Can I give you this trust ?
Absolute confidentiality is required from you.Besides,I will use my connection
to get some documents to back up the fund so that the fund can not be
question by any authority.
More information await you in my next response to your email message.
Treat as very urgent.
Yours Faithfully,
Dr. Carl Lee.
^ permalink raw reply
* Re: [PATCH 7/14] drivers/net/typhoon.c: delete double assignment
From: David Dillow @ 2010-10-26 11:52 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1288088743-3725-8-git-send-email-julia@diku.dk>
On Tue, 2010-10-26 at 12:25 +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location. The current definition
> does not initialize the respRing structure, which has the same type as the
> cmdRing structure, so initialize that one instead.
> Signed-off-by: Julia Lawall <julia@diku.dk>
Acked-by: David Dillow <dave@thedillows.org>
> ---
> This changes the semantics and has not been tested.
tp->respRing.lastWrite is only an artifact of using a common struct for
the rings and is not otherwise used, so no change to semantics.
^ permalink raw reply
* 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
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