* 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: 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] 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
* [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: [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
* 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: [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: [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
* 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
* [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
* 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
* 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: 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 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
* 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: [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
* 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: [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] 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
* [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
* [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 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 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
* 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
* 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
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