* Re: [PATCH RFC 50/77] mlx5: Update MSI/MSI-X interrupts enablement code
From: Eli Cohen @ 2013-10-03 7:14 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-kernel, Bjorn Helgaas, Ralf Baechle, Michael Ellerman,
Benjamin Herrenschmidt, Martin Schwidefsky, Ingo Molnar,
Tejun Heo, Dan Williams, Andy King, Jon Mason, Matt Porter,
linux-pci, linux-mips, linuxppc-dev, linux390, linux-s390, x86,
linux-ide, iss_storagedev, linux-nvme, linux-rdma, netdev,
e1000-devel, linux-driver, Solarflare linux maintainers,
"VMware, Inc." <pv-dr
In-Reply-To: <9650a7dfbcfd5f1da21f7b093665abf4b1041071.1380703263.git.agordeev@redhat.com>
On Wed, Oct 02, 2013 at 12:49:06PM +0200, Alexander Gordeev wrote:
>
> + err = pci_msix_table_size(dev->pdev);
> + if (err < 0)
> + return err;
> +
> nvec = dev->caps.num_ports * num_online_cpus() + MLX5_EQ_VEC_COMP_BASE;
> nvec = min_t(int, nvec, num_eqs);
> + nvec = min_t(int, nvec, err);
> if (nvec <= MLX5_EQ_VEC_COMP_BASE)
> return -ENOSPC;
Making sure we don't request more vectors then the device's is capable
of -- looks good.
>
> @@ -131,20 +136,15 @@ static int mlx5_enable_msix(struct mlx5_core_dev *dev)
> for (i = 0; i < nvec; i++)
> table->msix_arr[i].entry = i;
>
> -retry:
> - table->num_comp_vectors = nvec - MLX5_EQ_VEC_COMP_BASE;
> err = pci_enable_msix(dev->pdev, table->msix_arr, nvec);
> - if (err <= 0) {
> + if (err) {
> + kfree(table->msix_arr);
> return err;
> - } else if (err > MLX5_EQ_VEC_COMP_BASE) {
> - nvec = err;
> - goto retry;
> }
>
According to latest sources, pci_enable_msix() may still fail so why
do you want to remove this code?
> - mlx5_core_dbg(dev, "received %d MSI vectors out of %d requested\n", err, nvec);
> - kfree(table->msix_arr);
> + table->num_comp_vectors = nvec - MLX5_EQ_VEC_COMP_BASE;
>
> - return -ENOSPC;
> + return 0;
> }
>
> static void mlx5_disable_msix(struct mlx5_core_dev *dev)
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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 net] be2net: Warn users of possible broken functionality on BE2 cards with very old F/W versions with latest driver.
From: Sathya Perla @ 2013-10-03 7:11 UTC (permalink / raw)
To: Somnath Kotur, netdev@vger.kernel.org; +Cc: davem@davemloft.net, Somnath Kotur
In-Reply-To: <e84f68dc-6ab3-4e56-b2f5-70e51c46d1c3@CMEXHTCAS1.ad.emulex.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf
> Of Somnath Kotur
>
> On very old F/W versions < 4.0, the mailbox command to set interrupts on the
> card succeeds even though it is not supported and should have failed leading to
> interrupts not working.
> Hence warn users to upgrade to a suitable F/W version to avoid seeing broken
> functionality.
>
Som, we've already use the term "FW" (instead of "F/W") at various places in the
code. Can you pls stick to this.
This is important especially for log messages. "FW" (similar to "HW" ) is much more
commonly used and more readable.
> Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
> ---
> drivers/net/ethernet/emulex/benet/be_main.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c
> b/drivers/net/ethernet/emulex/benet/be_main.c
> index 2c38cc4..f4bbc92 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -3247,6 +3247,11 @@ static int be_setup(struct be_adapter *adapter)
>
> be_cmd_get_fw_ver(adapter, adapter->fw_ver, adapter->fw_on_flash);
>
> + if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
> + dev_err(dev, "F/W version is very old. IRQs may not work\n");
> + dev_err(dev, "Pls upgrade to F/W version >= 4.0\n");
> + }
Same as above.
> +
> if (adapter->vlans_added)
> be_vid_config(adapter);
>
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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
* [PATCH net] be2net: Warn users of possible broken functionality on BE2 cards with very old F/W versions with latest driver.
From: Somnath Kotur @ 2013-10-03 7:03 UTC (permalink / raw)
To: netdev; +Cc: davem, Somnath Kotur
On very old F/W versions < 4.0, the mailbox command to set interrupts on the
card succeeds even though it is not supported and should have failed leading to
interrupts not working.
Hence warn users to upgrade to a suitable F/W version to avoid seeing broken
functionality.
Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 2c38cc4..f4bbc92 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3247,6 +3247,11 @@ static int be_setup(struct be_adapter *adapter)
be_cmd_get_fw_ver(adapter, adapter->fw_ver, adapter->fw_on_flash);
+ if (BE2_chip(adapter) && memcmp(adapter->fw_ver, "4.", 2) < 0) {
+ dev_err(dev, "F/W version is very old. IRQs may not work\n");
+ dev_err(dev, "Pls upgrade to F/W version >= 4.0\n");
+ }
+
if (adapter->vlans_added)
be_vid_config(adapter);
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Julia Lawall @ 2013-10-03 6:32 UTC (permalink / raw)
To: Joe Perches
Cc: Eric Dumazet, Kalle Valo, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380778451.2081.103.camel@joe-AO722>
The following semantic patch fixes the type declarations. It should be
run with the argument --recursive-includes for best (but slowest) results.
julia
@@
identifier x;
expression e1;
type T;
@@
T x[
- 6
+ ETH_ALEN
];
... when any
(
memcpy(x,e1,ETH_ALEN)
|
memcpy(e1,x,ETH_ALEN)
|
memset(x,e1,ETH_ALEN)
)
@r@
type T,T1;
identifier x;
@@
T {
...
T1 x[6];
...
};
@s@
r.T *e;
identifier r.x;
expression e1;
@@
(
memcpy(e->x,e1,ETH_ALEN)
|
memcpy(e1,e->x,ETH_ALEN)
|
memset(e->x,e1,ETH_ALEN)
)
@depends on s@
type r.T,r.T1;
identifier r.x;
@@
T {
...
T1 x[
-6
+ ETH_ALEN
];
...
};
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Joe Perches @ 2013-10-03 6:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380780021.19002.164.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2013-10-02 at 23:00 -0700, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 22:34 -0700, Joe Perches wrote:
> > https://lkml.org/lkml/2013/10/1/517
> I had no problem with this one.
> Did I say something (funny or not) about it ?
Nope, you didn't say anything publicly or
even privately to me.
> Apparently you mix things and you do not like me commenting or giving a
> feedback on a _particular_ patch.
Feedback like expecting 500 individual patches
when you apparently saw one that modified 57 files
with multiple maintainers _is_ pretty funny.
cheers, Joe
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Eric Dumazet @ 2013-10-03 6:00 UTC (permalink / raw)
To: Joe Perches
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380778451.2081.103.camel@joe-AO722>
On Wed, 2013-10-02 at 22:34 -0700, Joe Perches wrote:
> https://lkml.org/lkml/2013/10/1/517
>
I had no problem with this one.
Did I say something (funny or not) about it ?
Apparently you mix things and you do not like me commenting or giving a
feedback on a _particular_ patch.
Everything is fine, really, let the fun continue !
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Julia Lawall @ 2013-10-03 5:47 UTC (permalink / raw)
To: Joe Perches
Cc: Eric Dumazet, Kalle Valo, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380778451.2081.103.camel@joe-AO722>
On Wed, 2 Oct 2013, Joe Perches wrote:
> On Wed, 2013-10-02 at 22:24 -0700, Eric Dumazet wrote:
> > On Wed, 2013-10-02 at 22:09 -0700, Joe Perches wrote:
> > > On Wed, 2013-10-02 at 21:44 -0700, Eric Dumazet wrote:
> > > > I mean the 6, of course, since Joe seems to actively track them, as if
> > > > ETH_ALEN could change eventually, you never know.
> > >
> > > You're funny Eric.
> > > You know it's just to ease grep pattern matching.
> >
> > You are not funny if you plan to send 500+ patches for every instance of
> > 6 changed to ETH_ALEN
>
> You're still funny.
>
> https://lkml.org/lkml/2013/10/1/517
>
> Lighten up though. It was just a straggler.
Actually, the few cases that I looked up by hand seemed to use 6
consistently for the declaration and the memcpy/memset. So it would be
nicer to fix them all at once.
At least in drivers/net there are around 40, not 500.
julia
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Julia Lawall @ 2013-10-03 5:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Perches, Kalle Valo, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380773924.19002.131.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2 Oct 2013, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 20:39 -0700, Joe Perches wrote:
> > Use the appropriate define instead of 6.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
> > Noticed-by: Julia Lawall <julia.lawall@lip6.fr> via spatch script
> >
> > ---
> >
> > drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> > index bee88e8..48d44e7 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -1758,7 +1758,7 @@ int ath10k_wmi_vdev_up(struct ath10k *ar, u32 vdev_id, u32 aid, const u8 *bssid)
> > cmd = (struct wmi_vdev_up_cmd *)skb->data;
> > cmd->vdev_id = __cpu_to_le32(vdev_id);
> > cmd->vdev_assoc_id = __cpu_to_le32(aid);
> > - memcpy(&cmd->vdev_bssid.addr, bssid, 6);
> > + memcpy(&cmd->vdev_bssid.addr, bssid, ETH_ALEN);
> >
> > ath10k_dbg(ATH10K_DBG_WMI,
> > "wmi mgmt vdev up id 0x%x assoc id %d bssid %pM\n",
>
> I don't get it.
>
> Why leaving this then ?
>
> struct wmi_mac_addr {
> union {
> u8 addr[6];
> struct {
> u32 word0;
> u32 word1;
> } __packed;
> } __packed;
> } __packed;
Yes, this was step 2...
julia
^ permalink raw reply
* Re: [PATCH net-next] net:drivers/net: Miscellaneous conversions to ETH_ALEN
From: Julia Lawall @ 2013-10-03 5:41 UTC (permalink / raw)
To: Joe Perches; +Cc: Luis R. Rodriguez, Kalle Valo, netdev, ath10k
In-Reply-To: <1380758954.2081.79.camel@joe-AO722>
On Wed, 2 Oct 2013, Joe Perches wrote:
> On Thu, 2013-10-03 at 00:38 +0200, Julia Lawall wrote:
> >
> >
> > On Wed, 2 Oct 2013, Joe Perches wrote:
> >
> > > On Wed, 2013-10-02 at 10:44 -0700, Luis R. Rodriguez wrote:
> > > > On Tue, Oct 1, 2013 at 11:40 PM, Joe Perches <joe@perches.com> wrote:
> > > > > Please include netdev. (cc'd)
> > > > >
> > > > >> Joe Perches <joe@perches.com> writes:
> > > > >>
> > > > >> > Convert the memset/memcpy uses of 6 to ETH_ALEN
> > > > >> > where appropriate.
> > > > >
> > > > >> > Signed-off-by: Joe Perches <joe@perches.com>
> > > >
> > > > I think these sorts of patches are good -- but once applied it'd be
> > > > good if we can get the SmPL grammar expressed for it and then have
> > > > developers / maintainers regularly doing:
> > > >
> > > > make coccicheck MODE=patch M=path > path-cocci.patch
> > > >
> > > > Unfortunately right now MODE=patch takes about 3 1/2 minutes for
> > > > ath9k, MODE=org takes ~10 minutes for ath9k (17 minutes for all of
> > > > ath/), and MODE=context takes ~8 minutes on ath9k -- I do believe its
> > > > a bit unreasonable to expect patch submitters to use this, but
> > > > certainly great practice. Some of the time differences on the reports
> > > > can be explained by the fact that some SmPL will only be used for some
> > > > modes.
> > > >
> > > > Even though it takes a while right now it'd be great practice to use
> > > > coccicheck to prevent these type of changes from going in again,
> > > > things that checkpatch.pl won't be able to catch.
> > >
> > > As far as I can tell, it's basically not possible for cocci to
> > > do this conversion.
> >
> > I tried looking for memcpys and memsets that do use ETH_ALEN and then
> > seeing what non-local functions the affected values flow to. I then
> > marked all of the calls to memcpy and memset that use 6 where an affected
> > value flows to one of the functions identified in the first pass. I get
> > 40 unique results on Linux 3.10.
> >
> > The semantic patch is below. It needs to be cleaned up to not return
> > duplicate results. It needs to be run with the argument --no-show-diff,
> > and the result is printed in emacs org mode.
>
> This has been running a _long_ time (broken?) on
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>
> $ spatch --version
> spatch version 1.0.0-rc14 without Python support and with PCRE support
Likewise. I can look at it, but generally I just use a timeout.
Note that you should run it on a whole directory, not on a file at a time,
so that it can collectthe most possible information on the first pass.
julia
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Joe Perches @ 2013-10-03 5:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380777884.19002.155.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2013-10-02 at 22:24 -0700, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 22:09 -0700, Joe Perches wrote:
> > On Wed, 2013-10-02 at 21:44 -0700, Eric Dumazet wrote:
> > > I mean the 6, of course, since Joe seems to actively track them, as if
> > > ETH_ALEN could change eventually, you never know.
> >
> > You're funny Eric.
> > You know it's just to ease grep pattern matching.
>
> You are not funny if you plan to send 500+ patches for every instance of
> 6 changed to ETH_ALEN
You're still funny.
https://lkml.org/lkml/2013/10/1/517
Lighten up though. It was just a straggler.
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Eric Dumazet @ 2013-10-03 5:24 UTC (permalink / raw)
To: Joe Perches
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380776994.2081.101.camel@joe-AO722>
On Wed, 2013-10-02 at 22:09 -0700, Joe Perches wrote:
> On Wed, 2013-10-02 at 21:44 -0700, Eric Dumazet wrote:
> > I mean the 6, of course, since Joe seems to actively track them, as if
> > ETH_ALEN could change eventually, you never know.
>
> You're funny Eric.
> You know it's just to ease grep pattern matching.
You are not funny if you plan to send 500+ patches for every instance of
6 changed to ETH_ALEN
I am not interested playing these grep games, honestly.
If you patch a driver, try to avoid sending one patch per line, that's
really not necessary, thank you.
If the structure definition uses [6], I see no reason to use ETH_ALEN in
the memset()/memcpy().
How is that funny, I don't know.
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Joe Perches @ 2013-10-03 5:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380775478.19002.139.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2013-10-02 at 21:44 -0700, Eric Dumazet wrote:
> I mean the 6, of course, since Joe seems to actively track them, as if
> ETH_ALEN could change eventually, you never know.
You're funny Eric.
You know it's just to ease grep pattern matching.
^ permalink raw reply
* Re: [PATCH net-next] fix unsafe set_memory_rw from softirq
From: Eric Dumazet @ 2013-10-03 4:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, netdev, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Daniel Borkmann, Paul E. McKenney, Xi Wang, x86,
Eric Dumazet, linux-kernel, Heiko Carstens
In-Reply-To: <1380775982.19002.145.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2013-10-02 at 21:53 -0700, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 21:44 -0700, Alexei Starovoitov wrote:
>
> > I think ifdef config_x86 is a bit ugly inside struct sk_filter, but
> > don't mind whichever way.
>
> Its not fair to make sk_filter bigger, because it means that simple (non
> JIT) filter might need an extra cache line.
>
> You could presumably use the following layout instead :
>
> struct sk_filter
> {
> atomic_t refcnt;
> struct rcu_head rcu;
> struct work_struct work;
>
> unsigned int len ____cacheline_aligned; /* Number of filter blocks */
> unsigned int (*bpf_func)(const struct sk_buff *skb,
> const struct sock_filter *filter);
> struct sock_filter insns[0];
> };
And since @len is not used by sk_run_filter() use :
struct sk_filter {
atomic_t refcnt;
int len; /* number of filter blocks */
struct rcu_head rcu;
struct work_struct work;
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct sock_filter *filter) ____cacheline_aligned;
struct sock_filter insns[0];
};
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Joe Perches @ 2013-10-03 4:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380773924.19002.131.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, 2013-10-02 at 21:18 -0700, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 20:39 -0700, Joe Perches wrote:
> > Use the appropriate define instead of 6.
[]
> I don't get it.
> Why leaving this then ?
>
> struct wmi_mac_addr {
> union {
> u8 addr[6];
> struct {
> u32 word0;
> u32 word1;
> } __packed;
> } __packed;
> } __packed;
'cause I just did the ones I noticed around the memcpy/memset
grep pattern I used.
$ grep -rP --include=*.[ch] -n "\b(memset|memcpy)\s*\([^,]+,[^,]+,\s*6\s*\)" drivers/net
You're welcome to look for all the [6] uses and convert the
appropriate ones too. Maybe use that eth_addr_t typedef.
$ git grep -E "\[\s*6\s*\]\s*;" drivers/net/
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Kalle Valo @ 2013-10-03 4:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Perches, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380775478.19002.139.camel@edumazet-glaptop.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Thu, 2013-10-03 at 07:34 +0300, Kalle Valo wrote:
>
>> Do you mean '6' or the union? The 6 can replaced with ETH_ALEN AFAICS.
>> But the union is needed for aligning the packets as firmware expects
>> them.
>
> I mean the 6, of course, since Joe seems to actively track them, as if
> ETH_ALEN could change eventually, you never know.
>
> I am worrying this will take another hundred patches ...
Ah, ok.
> Regarding this union, the __packed attributes seem overkill, no ???
Yeah, they do. This should be enough, right?
struct wmi_mac_addr {
union {
u8 addr[6];
struct {
u32 word0;
u32 word1;
};
};
} __packed;
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next] fix unsafe set_memory_rw from softirq
From: Eric Dumazet @ 2013-10-03 4:53 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, netdev, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Daniel Borkmann, Paul E. McKenney, Xi Wang, x86,
Eric Dumazet, linux-kernel, Heiko Carstens
In-Reply-To: <CAMEtUux5Fvffdg1G-aAe4XN8hJ3oUZyD2Gs3sqcwDdGf9VQ5fw@mail.gmail.com>
On Wed, 2013-10-02 at 21:44 -0700, Alexei Starovoitov wrote:
> I think ifdef config_x86 is a bit ugly inside struct sk_filter, but
> don't mind whichever way.
Its not fair to make sk_filter bigger, because it means that simple (non
JIT) filter might need an extra cache line.
You could presumably use the following layout instead :
struct sk_filter
{
atomic_t refcnt;
struct rcu_head rcu;
struct work_struct work;
unsigned int len ____cacheline_aligned; /* Number of filter blocks */
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct sock_filter *filter);
struct sock_filter insns[0];
};
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Eric Dumazet @ 2013-10-03 4:44 UTC (permalink / raw)
To: Kalle Valo
Cc: Joe Perches, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <87fvsjynuk.fsf@kamboji.qca.qualcomm.com>
On Thu, 2013-10-03 at 07:34 +0300, Kalle Valo wrote:
> Do you mean '6' or the union? The 6 can replaced with ETH_ALEN AFAICS.
> But the union is needed for aligning the packets as firmware expects
> them.
I mean the 6, of course, since Joe seems to actively track them, as if
ETH_ALEN could change eventually, you never know.
I am worrying this will take another hundred patches ...
Regarding this union, the __packed attributes seem overkill, no ???
^ permalink raw reply
* Re: [PATCH net-next] fix unsafe set_memory_rw from softirq
From: Alexei Starovoitov @ 2013-10-03 4:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, netdev, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Daniel Borkmann, Paul E. McKenney, Xi Wang, x86,
Eric Dumazet, linux-kernel, Heiko Carstens
In-Reply-To: <1380774230.19002.135.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, Oct 2, 2013 at 9:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2013-10-02 at 20:50 -0700, Alexei Starovoitov wrote:
>> on x86 system with net.core.bpf_jit_enable = 1
>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index a6ac848..378fa03 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -27,6 +27,7 @@ struct sk_filter
>> unsigned int len; /* Number of filter blocks */
>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>> const struct sock_filter *filter);
>> + struct work_struct work;
>> struct rcu_head rcu;
>> struct sock_filter insns[0];
>> };
>
> Nice catch !
>
> It seems only x86 and s390 needs this work_struct.
I think ifdef config_x86 is a bit ugly inside struct sk_filter, but
don't mind whichever way.
> (and you might CC Heiko Carstens <heiko.carstens@de.ibm.com> to ask him
> to make the s390 part, of Ack it if you plan to do it)
set_memory_rw() on s390 is a simple page table walker that doesn't do
any IPI unlike x86
Heiko, please confirm that it's not an issue there.
Thanks
Alexei
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Kalle Valo @ 2013-10-03 4:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Perches, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380773924.19002.131.camel@edumazet-glaptop.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Wed, 2013-10-02 at 20:39 -0700, Joe Perches wrote:
>> Use the appropriate define instead of 6.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> Noticed-by: Julia Lawall <julia.lawall@lip6.fr> via spatch script
>>
>> ---
>>
>> drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
>> index bee88e8..48d44e7 100644
>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>> @@ -1758,7 +1758,7 @@ int ath10k_wmi_vdev_up(struct ath10k *ar, u32 vdev_id, u32 aid, const u8 *bssid)
>> cmd = (struct wmi_vdev_up_cmd *)skb->data;
>> cmd->vdev_id = __cpu_to_le32(vdev_id);
>> cmd->vdev_assoc_id = __cpu_to_le32(aid);
>> - memcpy(&cmd->vdev_bssid.addr, bssid, 6);
>> + memcpy(&cmd->vdev_bssid.addr, bssid, ETH_ALEN);
>>
>> ath10k_dbg(ATH10K_DBG_WMI,
>> "wmi mgmt vdev up id 0x%x assoc id %d bssid %pM\n",
>
> I don't get it.
>
> Why leaving this then ?
>
> struct wmi_mac_addr {
> union {
> u8 addr[6];
> struct {
> u32 word0;
> u32 word1;
> } __packed;
> } __packed;
> } __packed;
Do you mean '6' or the union? The 6 can replaced with ETH_ALEN AFAICS.
But the union is needed for aligning the packets as firmware expects
them.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next] fix unsafe set_memory_rw from softirq
From: Eric Dumazet @ 2013-10-03 4:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, netdev, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Daniel Borkmann, Paul E. McKenney, Xi Wang, x86,
Eric Dumazet, linux-kernel
In-Reply-To: <1380772231-3566-1-git-send-email-ast@plumgrid.com>
On Wed, 2013-10-02 at 20:50 -0700, Alexei Starovoitov wrote:
> on x86 system with net.core.bpf_jit_enable = 1
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index a6ac848..378fa03 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -27,6 +27,7 @@ struct sk_filter
> unsigned int len; /* Number of filter blocks */
> unsigned int (*bpf_func)(const struct sk_buff *skb,
> const struct sock_filter *filter);
> + struct work_struct work;
> struct rcu_head rcu;
> struct sock_filter insns[0];
> };
Nice catch !
It seems only x86 and s390 needs this work_struct.
(and you might CC Heiko Carstens <heiko.carstens@de.ibm.com> to ask him
to make the s390 part, of Ack it if you plan to do it)
Thanks
^ permalink raw reply
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Eric Dumazet @ 2013-10-03 4:18 UTC (permalink / raw)
To: Joe Perches
Cc: Kalle Valo, Julia Lawall, Luis R. Rodriguez, netdev, ath10k,
John W. Linville
In-Reply-To: <1380771551.2081.93.camel@joe-AO722>
On Wed, 2013-10-02 at 20:39 -0700, Joe Perches wrote:
> Use the appropriate define instead of 6.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Noticed-by: Julia Lawall <julia.lawall@lip6.fr> via spatch script
>
> ---
>
> drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index bee88e8..48d44e7 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1758,7 +1758,7 @@ int ath10k_wmi_vdev_up(struct ath10k *ar, u32 vdev_id, u32 aid, const u8 *bssid)
> cmd = (struct wmi_vdev_up_cmd *)skb->data;
> cmd->vdev_id = __cpu_to_le32(vdev_id);
> cmd->vdev_assoc_id = __cpu_to_le32(aid);
> - memcpy(&cmd->vdev_bssid.addr, bssid, 6);
> + memcpy(&cmd->vdev_bssid.addr, bssid, ETH_ALEN);
>
> ath10k_dbg(ATH10K_DBG_WMI,
> "wmi mgmt vdev up id 0x%x assoc id %d bssid %pM\n",
I don't get it.
Why leaving this then ?
struct wmi_mac_addr {
union {
u8 addr[6];
struct {
u32 word0;
u32 word1;
} __packed;
} __packed;
} __packed;
^ permalink raw reply
* [PATCH net-next] fix unsafe set_memory_rw from softirq
From: Alexei Starovoitov @ 2013-10-03 3:50 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Daniel Borkmann, Paul E. McKenney, Xi Wang, x86, Eric Dumazet,
linux-kernel
on x86 system with net.core.bpf_jit_enable = 1
sudo tcpdump -i eth1 'tcp port 22'
causes the warning:
[ 56.766097] Possible unsafe locking scenario:
[ 56.766097]
[ 56.780146] CPU0
[ 56.786807] ----
[ 56.793188] lock(&(&vb->lock)->rlock);
[ 56.799593] <Interrupt>
[ 56.805889] lock(&(&vb->lock)->rlock);
[ 56.812266]
[ 56.812266] *** DEADLOCK ***
[ 56.812266]
[ 56.830670] 1 lock held by ksoftirqd/1/13:
[ 56.836838] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff8118f44c>] vm_unmap_aliases+0x8c/0x380
[ 56.849757]
[ 56.849757] stack backtrace:
[ 56.862194] CPU: 1 PID: 13 Comm: ksoftirqd/1 Not tainted 3.12.0-rc3+ #45
[ 56.868721] Hardware name: System manufacturer System Product Name/P8Z77 WS, BIOS 3007 07/26/2012
[ 56.882004] ffffffff821944c0 ffff88080bbdb8c8 ffffffff8175a145 0000000000000007
[ 56.895630] ffff88080bbd5f40 ffff88080bbdb928 ffffffff81755b14 0000000000000001
[ 56.909313] ffff880800000001 ffff880800000000 ffffffff8101178f 0000000000000001
[ 56.923006] Call Trace:
[ 56.929532] [<ffffffff8175a145>] dump_stack+0x55/0x76
[ 56.936067] [<ffffffff81755b14>] print_usage_bug+0x1f7/0x208
[ 56.942445] [<ffffffff8101178f>] ? save_stack_trace+0x2f/0x50
[ 56.948932] [<ffffffff810cc0a0>] ? check_usage_backwards+0x150/0x150
[ 56.955470] [<ffffffff810ccb52>] mark_lock+0x282/0x2c0
[ 56.961945] [<ffffffff810ccfed>] __lock_acquire+0x45d/0x1d50
[ 56.968474] [<ffffffff810cce6e>] ? __lock_acquire+0x2de/0x1d50
[ 56.975140] [<ffffffff81393bf5>] ? cpumask_next_and+0x55/0x90
[ 56.981942] [<ffffffff810cef72>] lock_acquire+0x92/0x1d0
[ 56.988745] [<ffffffff8118f52a>] ? vm_unmap_aliases+0x16a/0x380
[ 56.995619] [<ffffffff817628f1>] _raw_spin_lock+0x41/0x50
[ 57.002493] [<ffffffff8118f52a>] ? vm_unmap_aliases+0x16a/0x380
[ 57.009447] [<ffffffff8118f52a>] vm_unmap_aliases+0x16a/0x380
[ 57.016477] [<ffffffff8118f44c>] ? vm_unmap_aliases+0x8c/0x380
[ 57.023607] [<ffffffff810436b0>] change_page_attr_set_clr+0xc0/0x460
[ 57.030818] [<ffffffff810cfb8d>] ? trace_hardirqs_on+0xd/0x10
[ 57.037896] [<ffffffff811a8330>] ? kmem_cache_free+0xb0/0x2b0
[ 57.044789] [<ffffffff811b59c3>] ? free_object_rcu+0x93/0xa0
[ 57.051720] [<ffffffff81043d9f>] set_memory_rw+0x2f/0x40
[ 57.058727] [<ffffffff8104e17c>] bpf_jit_free+0x2c/0x40
[ 57.065577] [<ffffffff81642cba>] sk_filter_release_rcu+0x1a/0x30
[ 57.072338] [<ffffffff811108e2>] rcu_process_callbacks+0x202/0x7c0
[ 57.078962] [<ffffffff81057f17>] __do_softirq+0xf7/0x3f0
[ 57.085373] [<ffffffff81058245>] run_ksoftirqd+0x35/0x70
cannot reuse filter memory, since it's readonly, so have to
extend sk_filter with work_struct
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
arch/x86/net/bpf_jit_comp.c | 17 ++++++++++++-----
include/linux/filter.h | 1 +
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 79c216a..89a43df 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -772,13 +772,20 @@ out:
return;
}
+static void bpf_jit_free_deferred(struct work_struct *work)
+{
+ struct sk_filter *fp = container_of(work, struct sk_filter, work);
+ unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
+ struct bpf_binary_header *header = (void *)addr;
+
+ set_memory_rw(addr, header->pages);
+ module_free(NULL, header);
+}
+
void bpf_jit_free(struct sk_filter *fp)
{
if (fp->bpf_func != sk_run_filter) {
- unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
- struct bpf_binary_header *header = (void *)addr;
-
- set_memory_rw(addr, header->pages);
- module_free(NULL, header);
+ INIT_WORK(&fp->work, bpf_jit_free_deferred);
+ schedule_work(&fp->work);
}
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index a6ac848..378fa03 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -27,6 +27,7 @@ struct sk_filter
unsigned int len; /* Number of filter blocks */
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct sock_filter *filter);
+ struct work_struct work;
struct rcu_head rcu;
struct sock_filter insns[0];
};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: David Miller @ 2013-10-03 3:44 UTC (permalink / raw)
To: joe; +Cc: kvalo, julia.lawall, mcgrof, netdev, ath10k, linville
In-Reply-To: <1380771551.2081.93.camel@joe-AO722>
From: Joe Perches <joe@perches.com>
Date: Wed, 02 Oct 2013 20:39:11 -0700
> Use the appropriate define instead of 6.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Noticed-by: Julia Lawall <julia.lawall@lip6.fr> via spatch script
Applied, thanks Joe.
^ permalink raw reply
* [PATCH net-next] ath10k: wmi: Convert use of 6 to ETH_ALEN
From: Joe Perches @ 2013-10-03 3:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Julia Lawall, Luis R. Rodriguez, netdev, ath10k, John W. Linville
In-Reply-To: <1380771110.2081.89.camel@joe-AO722>
Use the appropriate define instead of 6.
Signed-off-by: Joe Perches <joe@perches.com>
Noticed-by: Julia Lawall <julia.lawall@lip6.fr> via spatch script
---
drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index bee88e8..48d44e7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1758,7 +1758,7 @@ int ath10k_wmi_vdev_up(struct ath10k *ar, u32 vdev_id, u32 aid, const u8 *bssid)
cmd = (struct wmi_vdev_up_cmd *)skb->data;
cmd->vdev_id = __cpu_to_le32(vdev_id);
cmd->vdev_assoc_id = __cpu_to_le32(aid);
- memcpy(&cmd->vdev_bssid.addr, bssid, 6);
+ memcpy(&cmd->vdev_bssid.addr, bssid, ETH_ALEN);
ath10k_dbg(ATH10K_DBG_WMI,
"wmi mgmt vdev up id 0x%x assoc id %d bssid %pM\n",
^ permalink raw reply related
* Re: [PATCH net-next] net:drivers/net: Miscellaneous conversions to ETH_ALEN
From: Joe Perches @ 2013-10-03 3:31 UTC (permalink / raw)
To: Julia Lawall; +Cc: Luis R. Rodriguez, Kalle Valo, netdev, ath10k
In-Reply-To: <1380760114.2081.81.camel@joe-AO722>
On Wed, 2013-10-02 at 17:28 -0700, Joe Perches wrote:
> On Wed, 2013-10-02 at 17:09 -0700, Joe Perches wrote:
> > This has been running a _long_ time (broken?) on
> > drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> >
> > $ spatch --version
> > spatch version 1.0.0-rc14 without Python support and with PCRE support
>
> Nope. Not hung/broken. Just a _long_ time to run.
>
> HANDLING: drivers/net/ethernet/mellanox/mlx4/en_rx.c
> HANDLING: drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> Note: processing took 2299.7s: drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> HANDLING: drivers/net/ethernet/mellanox/mlx4/port.c
Hi again Julia:
Oddly, just a few files took > 1000 seconds to run.
Most all the rest were < 10 seconds (no report).
A few were ~100 seconds.
HANDLING: drivers/net/ethernet/mellanox/mlx4/en_netdev.c
Note: processing took 2299.7s: drivers/net/ethernet/mellanox/mlx4/en_netdev.c
[]
HANDLING: drivers/net/wireless/ipw2x00/libipw_rx.c
Note: processing took 5308.0s: drivers/net/wireless/ipw2x00/libipw_rx.c
[]
HANDLING: drivers/net/wireless/hostap/hostap_80211_rx.c
Note: processing took 1336.6s: drivers/net/wireless/hostap/hostap_80211_rx.c
I don't know what causes the delays on these files.
The only output I get for net-next is:
* TODO
[[view:drivers/net/wireless/zd1201.c::face=ovl-face1::linb=331::colb=3::cole=9][ drivers/net/wireless/zd1201.c::331]]
* TODO
[[view:drivers/net/wireless/zd1201.c::face=ovl-face1::linb=332::colb=3::cole=9][ drivers/net/wireless/zd1201.c::332]]
* TODO
[[view:drivers/net/wireless/zd1201.c::face=ovl-face1::linb=332::colb=3::cole=9][ drivers/net/wireless/zd1201.c::332]]
* TODO
[[view:drivers/net/wireless/zd1201.c::face=ovl-face1::linb=332::colb=3::cole=9][ drivers/net/wireless/zd1201.c::332]]
* TODO
[[view:drivers/net/wireless/zd1201.c::face=ovl-face1::linb=333::colb=3::cole=9][ drivers/net/wireless/zd1201.c::333]]
* TODO
[[view:drivers/net/wireless/ath/ath10k/wmi.c::face=ovl-face1::linb=1761::colb=1::cole=7][ drivers/net/wireless/ath/ath10k/wmi.c::1761]]
and the zd1201 conversions would be kind of odd.
Anyway, the ath10k file is a good one to patch. Thanks.
I'll send a patch separately.
^ 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