Netdev List
 help / color / mirror / Atom feed
* 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

* Re: [PATCH RFC 55/77] ntb: Update MSI/MSI-X interrupts enablement code
From: Jon Mason @ 2013-10-03  1:02 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, 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.,
	linux-scsi
In-Reply-To: <49eb592e15aaec804f9c11ca132d2b85c516aefa.1380703263.git.agordeev@redhat.com>

On Wed, Oct 02, 2013 at 12:49:11PM +0200, Alexander Gordeev wrote:
> As result of recent re-design of the MSI/MSI-X interrupts enabling
> pattern this driver has to be updated to use the new technique to
> obtain a optimal number of MSI/MSI-X interrupts required.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  drivers/ntb/ntb_hw.c |   41 +++++++++++++----------------------------
>  drivers/ntb/ntb_hw.h |    2 --
>  2 files changed, 13 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index eccd5e5..7776429 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1032,23 +1032,26 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  	struct msix_entry *msix;
>  	int msix_entries;
>  	int rc, i;
> -	u16 val;
>  
> -	if (!pdev->msix_cap) {
> -		rc = -EIO;
> +	rc = pci_msix_table_size(pdev);
> +	if (rc < 0)
>  		goto err;
> -	}
>  
> -	rc = pci_read_config_word(pdev, pdev->msix_cap + PCI_MSIX_FLAGS, &val);
> -	if (rc)
> +	/*
> +	 * On SNB, the link interrupt is always tied to 4th vector.  If
> +	 * we can't get all 4, then we can't use MSI-X.
> +	 */
> +	if ((rc < SNB_MSIX_CNT) && (ndev->hw_type != BWD_HW)) {

Please check for the HW type first, and then compare to
ndev->limits.msix_cnt (which will be SNB_MSIX_CNT on SNB HW).  Also,
put the comment inside the if statement and remove the unecessary "()"
around the comparisons.  OCD on my part, but I like it that way.  

> +		rc = -ENOSPC;
>  		goto err;
> -
> -	msix_entries = msix_table_size(val);
> -	if (msix_entries > ndev->limits.msix_cnt) {
> +	}

else if...

> +	if (rc > ndev->limits.msix_cnt) {
>  		rc = -EINVAL;
>  		goto err;
>  	}
>  
> +	msix_entries = rc;
> +
>  	ndev->msix_entries = kmalloc(sizeof(struct msix_entry) * msix_entries,
>  				     GFP_KERNEL);
>  	if (!ndev->msix_entries) {
> @@ -1060,26 +1063,8 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  		ndev->msix_entries[i].entry = i;
>  
>  	rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -	if (rc < 0)
> +	if (rc)
>  		goto err1;
> -	if (rc > 0) {
> -		/* On SNB, the link interrupt is always tied to 4th vector.  If
> -		 * we can't get all 4, then we can't use MSI-X.
> -		 */
> -		if ((rc < SNB_MSIX_CNT) && (ndev->hw_type != BWD_HW)) {
> -			rc = -EIO;
> -			goto err1;
> -		}
> -
> -		dev_warn(&pdev->dev,
> -			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
> -			 rc);
> -		msix_entries = rc;
> -
> -		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> -		if (rc)
> -			goto err1;
> -	}
>  
>  	for (i = 0; i < msix_entries; i++) {
>  		msix = &ndev->msix_entries[i];
> diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
> index 0a31ced..50bd760 100644
> --- a/drivers/ntb/ntb_hw.h
> +++ b/drivers/ntb/ntb_hw.h
> @@ -60,8 +60,6 @@
>  #define PCI_DEVICE_ID_INTEL_NTB_SS_HSX		0x2F0F
>  #define PCI_DEVICE_ID_INTEL_NTB_B2B_BWD		0x0C4E
>  
> -#define msix_table_size(control)	((control & PCI_MSIX_FLAGS_QSIZE)+1)

Good riddance!  :-)

> -
>  #ifndef readq
>  static inline u64 readq(void __iomem *addr)
>  {
> -- 
> 1.7.7.6
> 

^ permalink raw reply

* Re: [PATCH RFC 53/77] ntb: Fix missed call to pci_enable_msix()
From: Jon Mason @ 2013-10-03  0:49 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, Matt Porter, stable,
	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.
In-Reply-To: <0590d63c3432229a3824bada71e07a08fb955498.1380703263.git.agordeev@redhat.com>

On Wed, Oct 02, 2013 at 12:49:09PM +0200, Alexander Gordeev wrote:
> Current MSI-X enablement code assumes MSI-Xs were successfully
> allocated in case less than requested vectors were available.
> That assumption is wrong, since MSI-Xs should be enabled with
> a repeated call to pci_enable_msix(). This update fixes this.

Good catch, I'll pull it in for my next NTB release.

Thanks,
Jon

> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  drivers/ntb/ntb_hw.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 1cb6e51..de2062c 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1075,6 +1075,10 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  			 "Only %d MSI-X vectors.  Limiting the number of queues to that number.\n",
>  			 rc);
>  		msix_entries = rc;
> +
> +		rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> +		if (rc)
> +			goto err1;
>  	}
>  
>  	for (i = 0; i < msix_entries; i++) {
> -- 
> 1.7.7.6
> 

^ permalink raw reply

* Re: [PATCH RFC 54/77] ntb: Ensure number of MSIs on SNB is enough for the link interrupt
From: Jon Mason @ 2013-10-03  0:48 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-mips, VMware, Inc., Benjamin Herrenschmidt, linux-nvme,
	linux-ide, stable, linux-s390, Andy King, linux-scsi, linux-rdma,
	x86, Ingo Molnar, linux-pci, Matt Porter, iss_storagedev,
	linux-driver, Michael Ellerman, Tejun Heo, Bjorn Helgaas,
	Dan Williams, Solarflare linux maintainers, netdev, linux-kernel,
	Ralf Baechle, e1000-devel, Martin Schwidefsky, linux390
In-Reply-To: <5d9c5b2d3bbc444ff32bddeece7a239d046bd79c.1380703263.git.agordeev@redhat.com>

On Wed, Oct 02, 2013 at 12:49:10PM +0200, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  drivers/ntb/ntb_hw.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index de2062c..eccd5e5 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1066,7 +1066,7 @@ static int ntb_setup_msix(struct ntb_device *ndev)
>  		/* On SNB, the link interrupt is always tied to 4th vector.  If
>  		 * we can't get all 4, then we can't use MSI-X.
>  		 */
> -		if (ndev->hw_type != BWD_HW) {
> +		if ((rc < SNB_MSIX_CNT) && (ndev->hw_type != BWD_HW)) {

Nack, this check is unnecessary.

Also, no comment in the commit on why it could be necessary.


>  			rc = -EIO;
>  			goto err1;
>  		}
> -- 
> 1.7.7.6
> 

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH RFC 01/77] PCI/MSI: Fix return value when populate_msi_sysfs() failed
From: Jon Mason @ 2013-10-03  0:39 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bjorn Helgaas, Ralf Baechle,
	Michael Ellerman, Benjamin Herrenschmidt, Martin Schwidefsky,
	Ingo Molnar, Tejun Heo, Dan Williams, Andy King, Matt Porter,
	stable-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux390-tA70FqPdS9bQT0dZR+AlfA,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, iss_storagedev-VXdhtT5mjnY,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-driver-h88ZbnxC6KDQT0dZR+AlfA, Solarflare linux maintainers,
	VMware, Inc.
In-Reply-To: <3ff5236944aae69f2cd934b5b6da7c1c269df7c1.1380703262.git.agordeev-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Wed, Oct 02, 2013 at 12:48:17PM +0200, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Since you are changing the behavior of the msix_capability_init
function on populate_msi_sysfs error, a comment describing why in this
commit would be nice.

> ---
>  drivers/pci/msi.c |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index d5f90d6..b43f391 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -719,7 +719,7 @@ static int msix_capability_init(struct pci_dev *dev,
>  
>  	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
>  	if (ret)
> -		goto error;
> +		goto out_avail;
>  
>  	/*
>  	 * Some devices require MSI-X to be enabled before we can touch the
> @@ -732,10 +732,8 @@ static int msix_capability_init(struct pci_dev *dev,
>  	msix_program_entries(dev, entries);
>  
>  	ret = populate_msi_sysfs(dev);
> -	if (ret) {
> -		ret = 0;
> -		goto error;
> -	}
> +	if (ret)
> +		goto out_free;
>  
>  	/* Set MSI-X enabled bits and unmask the function */
>  	pci_intx_for_msi(dev, 0);
> @@ -746,7 +744,7 @@ static int msix_capability_init(struct pci_dev *dev,
>  
>  	return 0;
>  
> -error:
> +out_avail:
>  	if (ret < 0) {
>  		/*
>  		 * If we had some success, report the number of irqs
> @@ -763,6 +761,7 @@ error:
>  			ret = avail;
>  	}
>  
> +out_free:
>  	free_msi_irqs(dev);
>  
>  	return ret;
> -- 
> 1.7.7.6
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 RFC 77/77] vxge: Update MSI/MSI-X interrupts enablement code
From: Jon Mason @ 2013-10-03  0:29 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, 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.,
	linux-scsi
In-Reply-To: <467ce10b1df795edf80ed222816ab739fee7b0ea.1380703263.git.agordeev@redhat.com>

On Wed, Oct 02, 2013 at 12:49:33PM +0200, Alexander Gordeev wrote:
> As result of recent re-design of the MSI/MSI-X interrupts enabling
> pattern this driver has to be updated to use the new technique to
> obtain a optimal number of MSI/MSI-X interrupts required.
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  drivers/net/ethernet/neterion/vxge/vxge-main.c |   36 ++++++++++-------------
>  1 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> index b81ff8b..b4d40dd 100644
> --- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
> +++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
> @@ -2297,7 +2297,21 @@ static int vxge_alloc_msix(struct vxgedev *vdev)
>  	int msix_intr_vect = 0, temp;
>  	vdev->intr_cnt = 0;
>  
> -start:
> +	ret = pci_msix_table_size(vdev->pdev);
> +	if (ret < 0)
> +		goto alloc_entries_failed;
> +
> +	if (ret < (vdev->no_of_vpath * 2 + 1)) {
> +		if ((max_config_vpath != VXGE_USE_DEFAULT) || (ret < 3)) {
> +			ret = -ENOSPC;
> +			goto alloc_entries_failed;
> +		}
> +		/* Try with less no of vector by reducing no of vpaths count */
> +		temp = (ret - 1)/2;
> +		vxge_close_vpaths(vdev, temp);
> +		vdev->no_of_vpath = temp;
> +	}

The original code was ugly (not my code, so I can say that).  I'd like
to see it a little stream lined.  Something like:


        vdev->intr_cnt = pci_msix_table_size(vdev->pdev);

        if (vdev->intr_cnt % 2 == 0)
                vdev->intr_cnt--;

        if (vdev->intr_cnt < 3 || max_config_vpath != VXGE_USE_DEFAULT)
                goto alloc_entries_failed;

        if (vdev->intr_cnt != vdev->no_of_vpath * 2 + 1) {
                vxge_close_vpaths(vdev, vdev->intr_cnt / 2);
                vdev->no_of_vpath = vdev->intr_cnt / 2;
        }


> +
>  	/* Tx/Rx MSIX Vectors count */
>  	vdev->intr_cnt = vdev->no_of_vpath * 2;
>  
> @@ -2347,25 +2361,7 @@ start:
>  	vdev->vxge_entries[j].in_use = 0;
>  
>  	ret = pci_enable_msix(vdev->pdev, vdev->entries, vdev->intr_cnt);
> -	if (ret > 0) {
> -		vxge_debug_init(VXGE_ERR,
> -			"%s: MSI-X enable failed for %d vectors, ret: %d",
> -			VXGE_DRIVER_NAME, vdev->intr_cnt, ret);
> -		if ((max_config_vpath != VXGE_USE_DEFAULT) || (ret < 3)) {
> -			ret = -ENOSPC;
> -			goto enable_msix_failed;
> -		}
> -
> -		kfree(vdev->entries);
> -		kfree(vdev->vxge_entries);
> -		vdev->entries = NULL;
> -		vdev->vxge_entries = NULL;
> -		/* Try with less no of vector by reducing no of vpaths count */
> -		temp = (ret - 1)/2;
> -		vxge_close_vpaths(vdev, temp);
> -		vdev->no_of_vpath = temp;
> -		goto start;
> -	} else if (ret < 0)
> +	if (ret)
>  		goto enable_msix_failed;

Nit, space here please.

>  	return 0;
>  
> -- 
> 1.7.7.6
> 

^ permalink raw reply

* Re: [PATCH net-next] net:drivers/net: Miscellaneous conversions to ETH_ALEN
From: Joe Perches @ 2013-10-03  0:28 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Luis R. Rodriguez, Kalle Valo, netdev, ath10k
In-Reply-To: <1380758954.2081.79.camel@joe-AO722>

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

^ permalink raw reply

* Re: [PATCH v2.41 5/5] datapath: Add basic MPLS support to kernel
From: Simon Horman @ 2013-10-03  0:20 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: dev@openvswitch.org, netdev, Jesse Gross, Ben Pfaff, Ravi K,
	Isaku Yamahata, Joe Stringer
In-Reply-To: <CALnjE+qOwkY0NJ=LbT0SgZFmnzvqRHcTS35xWVfMyPtjj4r0ZA@mail.gmail.com>

On Wed, Oct 02, 2013 at 11:03:57AM -0700, Pravin Shelar wrote:
> On Mon, Sep 30, 2013 at 11:47 PM, Simon Horman <horms@verge.net.au> wrote:
> > Allow datapath to recognize and extract MPLS labels into flow keys
> > and execute actions which push, pop, and set labels on packets.
> >
> > Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.
> >
> > Cc: Ravi K <rkerur@gmail.com>
> > Cc: Leo Alterman <lalterman@nicira.com>
> > Cc: Isaku Yamahata <yamahata@valinux.co.jp>
> > Cc: Joe Stringer <joe@wand.net.nz>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> >
> > +
> > +       /* this hack needed to get regular skb_gso_segment() */
> > +#ifdef HAVE___SKB_GSO_SEGMENT
> > +#undef __skb_gso_segment
> > +       skb_gso = __skb_gso_segment(skb, features, tx_path);
> > +#else
> > +#undef skb_gso_segment
> > +       skb_gso = skb_gso_segment(skb, features);
> > +#endif
> > +
> 
> We can get rid of #ifdefs by just using different name for
> rpl___skb_gso_segment(), something like mpls_vlan_skb_gso_segment().
> The way it is done for tnl-gso.

Thanks.

The reason that I had the code arranged this way was so that
calls to __skb_gso_segment() would go via rpl___skb_gso_segment()
on kernels older than v3.11. In particular calls outside of gso.c.

On closer examination the only such case is in ovs_dp_upcall().
Currently there should be no need to perform MPLS GSO segmentation in that
case because MPLS GSO segmentation can only be needed after actions are
applied.

However, I am concerned that it may be necessary later when
recirculation is introduced as in that case an upcall may occur
on a packet which has had actions applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox