Netdev List
 help / color / mirror / Atom feed
* [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tim Gardner @ 2013-03-10 15:39 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Tim Gardner, Trond Myklebust, J. Bruce Fields, David S. Miller,
	Tom Tucker, Haggai Eran, Or Gerlitz, Shani Michaeli,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA

rpcrdma_register_default_external() is several frames into
the call stack which goes deeper yet. You run the risk of stack
corruption by declaring such a large automatic variable,
so dynamically allocate the array of 'struct ib_phys_buf' objects in
order to silence the frame-larger-than warning.

net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]

gcc version 4.6.3

Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 net/sunrpc/xprtrdma/verbs.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 93726560..0916467 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -1736,9 +1736,13 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
 	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
 				  IB_ACCESS_REMOTE_READ);
 	struct rpcrdma_mr_seg *seg1 = seg;
-	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
+	struct ib_phys_buf *ipb;
 	int len, i, rc = 0;
 
+	ipb = kmalloc(sizeof(*ipb) * RPCRDMA_MAX_DATA_SEGS, GFP_KERNEL);
+	if (!ipb)
+		return -ENOMEM;
+
 	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
 		*nsegs = RPCRDMA_MAX_DATA_SEGS;
 	for (len = 0, i = 0; i < *nsegs;) {
@@ -1770,6 +1774,7 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
 		seg1->mr_len = len;
 	}
 	*nsegs = i;
+	kfree(ipb);
 	return rc;
 }
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 related

* Re: [PATCH net-next] Remove unused tw_cookie_values from tcp_timewait_sock
From: Eric Dumazet @ 2013-03-10 16:06 UTC (permalink / raw)
  To: Christoph Paasch; +Cc: David Miller, netdev, william.allen.simpson
In-Reply-To: <1362928719-13532-1-git-send-email-christoph.paasch@uclouvain.be>

On Sun, 2013-03-10 at 16:18 +0100, Christoph Paasch wrote:
> tw_cookie_values is never used in the TCP-stack.
> 
> It was added by 435cf559f (TCPCT part 1d: define TCP cookie option,
> extend existing struct's), but already at that time it was not used at
> all, nor mentioned in the commit-message.
> 
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> ---
>  include/linux/tcp.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index f28408c..515c374 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -361,10 +361,6 @@ struct tcp_timewait_sock {
>  #ifdef CONFIG_TCP_MD5SIG
>  	struct tcp_md5sig_key	  *tw_md5_key;
>  #endif
> -	/* Few sockets in timewait have cookies; in that case, then this
> -	 * object holds a reference to them (tw_cookie_values->kref).
> -	 */
> -	struct tcp_cookie_values  *tw_cookie_values;
>  };
>  
>  static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk)

Acked-by: Eric Dumazet <edumazet@google.com>

Actually, I am not sure TCPCT is really used...

^ permalink raw reply

* Re: [PATCH net-next] Remove unused tw_cookie_values from tcp_timewait_sock
From: Christoph Paasch @ 2013-03-10 16:33 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, william.allen.simpson
In-Reply-To: <1362931570.4051.34.camel@edumazet-glaptop>

On Sunday 10 March 2013 17:06:10 Eric Dumazet wrote:
> Acked-by: Eric Dumazet <edumazet@google.com>
> 
> Actually, I am not sure TCPCT is really used...

Yes, while looking through the code it seems to me that it is far from 
respecting RFC 6013.

It uses 253 as option-number and has comments saying "not yet implemented" 
(e.g., tcp_parse_options).

Should it be removed?


Christoph

-- 
IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
UCLouvain
--

^ permalink raw reply

* Re: hitting lockdep warning as of too early VF probe with 3.9-rc1
From: Greg Kroah-Hartman @ 2013-03-10 16:37 UTC (permalink / raw)
  To: Jack Morgenstein
  Cc: Ming Lei, Or Gerlitz, Or Gerlitz, David Miller, Roland Dreier,
	netdev, Yan Burman, Liran Liss
In-Reply-To: <201303101728.50883.jackm@dev.mellanox.co.il>

On Sun, Mar 10, 2013 at 05:28:50PM +0200, Jack Morgenstein wrote:
> Hello, Ming, Greg, Roland, Dave, all...
> 
> From a quick scan of ethernet drivers in Dave Miller's net-next git, I
> notice that the following drivers (apart from the Mellanox mlx4 driver)
> enable SRIOV during the PF probe:
>   cisco enic (function "enic_probe")
>   neterion vxge driver(function "vxge_probe")
>   Solarflare efx driver (function "efx_pci_probe", which invokes "efx_sriov_init")
>   emulex driver (function "be_probe" --> be_setup --> be_vf_setup)
> 
> It would seem that these drivers are susceptible to the nested probe/deadlock
> race condition as well.
> 
> I believe that it is healthiest for everyone if the probe code in the kernel itself
> would avoid such nested probe calls (rather than forcing vendors to deal
> with this issue).  The kernel code is certainly aware
> (or could easily track) that it is invoking the a driver's probe function
> while that same probe function has already been invoked and has not yet returned!

Patches to handle this are always gladly accepted.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] vhost_net: remove tx polling state
From: Michael S. Tsirkin @ 2013-03-10 16:50 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1362630716-57674-1-git-send-email-jasowang@redhat.com>

On Thu, Mar 07, 2013 at 12:31:56PM +0800, Jason Wang wrote:
> After commit 2b8b328b61c799957a456a5a8dab8cc7dea68575 (vhost_net: handle polling
> errors when setting backend), we in fact track the polling state through
> poll->wqh, so there's no need to duplicate the work with an extra
> vhost_net_polling_state. So this patch removes this and make the code simpler.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I'd prefer a more radical approach, since I think it can be even
simpler: tap and macvtap backends both only send events when tx queue
overruns which should almost never happen.

So let's just start polling when VQ is enabled
drop all poll_start/stop calls on data path.

The optimization was written for packet socket backend but I know of no
one caring about performance of that one that much.
Needs a bit of perf testing to make sure I didn't miss anything though
but it's not 3.9 material anyway so there's no rush.

> ---
>  drivers/vhost/net.c   |   60 ++++++++----------------------------------------
>  drivers/vhost/vhost.c |    3 ++
>  2 files changed, 13 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 959b1cd..d1a03dd 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -64,20 +64,10 @@ enum {
>  	VHOST_NET_VQ_MAX = 2,
>  };
>  
> -enum vhost_net_poll_state {
> -	VHOST_NET_POLL_DISABLED = 0,
> -	VHOST_NET_POLL_STARTED = 1,
> -	VHOST_NET_POLL_STOPPED = 2,
> -};
> -
>  struct vhost_net {
>  	struct vhost_dev dev;
>  	struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
>  	struct vhost_poll poll[VHOST_NET_VQ_MAX];
> -	/* Tells us whether we are polling a socket for TX.
> -	 * We only do this when socket buffer fills up.
> -	 * Protected by tx vq lock. */
> -	enum vhost_net_poll_state tx_poll_state;
>  	/* Number of TX recently submitted.
>  	 * Protected by tx vq lock. */
>  	unsigned tx_packets;
> @@ -155,28 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
>  	}
>  }
>  
> -/* Caller must have TX VQ lock */
> -static void tx_poll_stop(struct vhost_net *net)
> -{
> -	if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> -		return;
> -	vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> -	net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> -}
> -
> -/* Caller must have TX VQ lock */
> -static int tx_poll_start(struct vhost_net *net, struct socket *sock)
> -{
> -	int ret;
> -
> -	if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> -		return 0;
> -	ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> -	if (!ret)
> -		net->tx_poll_state = VHOST_NET_POLL_STARTED;
> -	return ret;
> -}
> -
>  /* In case of DMA done not in order in lower device driver for some reason.
>   * upend_idx is used to track end of used idx, done_idx is used to track head
>   * of used idx. Once lower device DMA done contiguously, we will signal KVM
> @@ -231,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>  static void handle_tx(struct vhost_net *net)
>  {
>  	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> +	struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
>  	unsigned out, in, s;
>  	int head;
>  	struct msghdr msg = {
> @@ -256,7 +225,7 @@ static void handle_tx(struct vhost_net *net)
>  	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>  	if (wmem >= sock->sk->sk_sndbuf) {
>  		mutex_lock(&vq->mutex);
> -		tx_poll_start(net, sock);
> +		vhost_poll_start(poll, sock->file);
>  		mutex_unlock(&vq->mutex);
>  		return;
>  	}
> @@ -265,7 +234,7 @@ static void handle_tx(struct vhost_net *net)
>  	vhost_disable_notify(&net->dev, vq);
>  
>  	if (wmem < sock->sk->sk_sndbuf / 2)
> -		tx_poll_stop(net);
> +		vhost_poll_stop(poll);
>  	hdr_size = vq->vhost_hlen;
>  	zcopy = vq->ubufs;
>  
> @@ -287,7 +256,7 @@ static void handle_tx(struct vhost_net *net)
>  
>  			wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>  			if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> -				tx_poll_start(net, sock);
> +				vhost_poll_start(poll, sock->file);
>  				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>  				break;
>  			}
> @@ -298,7 +267,7 @@ static void handle_tx(struct vhost_net *net)
>  				    (vq->upend_idx - vq->done_idx) :
>  				    (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
>  			if (unlikely(num_pends > VHOST_MAX_PEND)) {
> -				tx_poll_start(net, sock);
> +				vhost_poll_start(poll, sock->file);
>  				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>  				break;
>  			}
> @@ -364,7 +333,7 @@ static void handle_tx(struct vhost_net *net)
>  			}
>  			vhost_discard_vq_desc(vq, 1);
>  			if (err == -EAGAIN || err == -ENOBUFS)
> -				tx_poll_start(net, sock);
> +				vhost_poll_start(poll, sock->file);
>  			break;
>  		}
>  		if (err != len)
> @@ -627,7 +596,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>  
>  	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
>  	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> -	n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>  
>  	f->private_data = n;
>  
> @@ -637,32 +605,24 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>  static void vhost_net_disable_vq(struct vhost_net *n,
>  				 struct vhost_virtqueue *vq)
>  {
> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
>  	if (!vq->private_data)
>  		return;
> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> -		tx_poll_stop(n);
> -		n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> -	} else
> -		vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> +	vhost_poll_stop(poll);
>  }
>  
>  static int vhost_net_enable_vq(struct vhost_net *n,
>  				struct vhost_virtqueue *vq)
>  {
> +	struct vhost_poll *poll = n->poll + (vq - n->vqs);
>  	struct socket *sock;
> -	int ret;
>  
>  	sock = rcu_dereference_protected(vq->private_data,
>  					 lockdep_is_held(&vq->mutex));
>  	if (!sock)
>  		return 0;
> -	if (vq == n->vqs + VHOST_NET_VQ_TX) {
> -		n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> -		ret = tx_poll_start(n, sock);
> -	} else
> -		ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
>  
> -	return ret;
> +	return vhost_poll_start(poll, sock->file);
>  }
>  
>  static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 9759249..4eecdb8 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -89,6 +89,9 @@ int vhost_poll_start(struct vhost_poll *poll, struct file *file)
>  	unsigned long mask;
>  	int ret = 0;
>  
> +	if (poll->wqh)
> +		return 0;
> +
>  	mask = file->f_op->poll(file, &poll->table);
>  	if (mask)
>  		vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> -- 
> 1.7.1

^ permalink raw reply

* Re: [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tom Tucker @ 2013-03-10 17:16 UTC (permalink / raw)
  To: Tim Gardner
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	J. Bruce Fields, David S. Miller, Tom Tucker, Haggai Eran,
	Or Gerlitz, Shani Michaeli, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Steve Dickson
In-Reply-To: <1362929953-63785-1-git-send-email-tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


This is the result of 2773395b34883fe54418de188733a63bb38e0ad6. Steve 
might want to weigh in on this since it was done for performance reasons.

Tom

On 3/10/13 10:39 AM, Tim Gardner wrote:
> rpcrdma_register_default_external() is several frames into
> the call stack which goes deeper yet. You run the risk of stack
> corruption by declaring such a large automatic variable,
> so dynamically allocate the array of 'struct ib_phys_buf' objects in
> order to silence the frame-larger-than warning.
>
> net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
> net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>
> gcc version 4.6.3
>
> Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
> Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>   net/sunrpc/xprtrdma/verbs.c |    7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 93726560..0916467 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -1736,9 +1736,13 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
>   	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
>   				  IB_ACCESS_REMOTE_READ);
>   	struct rpcrdma_mr_seg *seg1 = seg;
> -	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
> +	struct ib_phys_buf *ipb;
>   	int len, i, rc = 0;
>   
> +	ipb = kmalloc(sizeof(*ipb) * RPCRDMA_MAX_DATA_SEGS, GFP_KERNEL);
> +	if (!ipb)
> +		return -ENOMEM;
> +
>   	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
>   		*nsegs = RPCRDMA_MAX_DATA_SEGS;
>   	for (len = 0, i = 0; i < *nsegs;) {
> @@ -1770,6 +1774,7 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
>   		seg1->mr_len = len;
>   	}
>   	*nsegs = i;
> +	kfree(ipb);
>   	return rc;
>   }
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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

* Greetings from George Daniels
From: Germaine Sandoval @ 2013-03-10 17:58 UTC (permalink / raw)




Greetings from George Daniels

I am George Daniels, a Banker and credit system programmer (HSBC bank). 
I saw your email address while browsing through  the bank D.T.C Screen in my office 
yesterday so I decided to use this very chance to know you. I believe 
we should use every opportunity to know each other better. However, I am contacting you 
for obvious reason which you will understand. 

I am sending this mail just to know if this email address is OK, 
reply me back so that I will send  more details to you. 
I have a very important thing to discuss with you, I look forward to receiving your response at 
georgedaniels@postino.net. Have a pleasant day.

George Daniels

^ permalink raw reply

* [RFC] net: cdc_ncm, cdc_mbim: allow user to prefer NCM for backwards compatibility
From: Bjørn Mork @ 2013-03-10 18:13 UTC (permalink / raw)
  To: Greg Suarez; +Cc: Alexey Orishko, linux-usb, netdev, Bjørn Mork
In-Reply-To: <87sj43cxg5.fsf@nemi.mork.no>

Devices implementing NCM backwards compatibility according to section
3.2 of the MBIM v1.0 specification allow either NCM or MBIM on a single
USB function, using different altsettings.  The cdc_ncm and cdc_mbim
drivers will both probe such functions, and must agree on a common
policy for selecting either MBIM or NCM.  Until now, this policy has
been set at build time based on CONFIG_USB_NET_CDC_MBIM.

Use a module parameter to set the system policy at runtime, allowing the
user to prefer NCM on systems with the cdc_mbim driver.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---

Maybe something like this?  I don't like the way this code looks, but I
am unable to make it look more readable...

Does this still work as expected with all your devices?  Does it allow you
to use cdc_ncm on devices with compatibility mode?  I still don't have any
such device, except for my fake emulated kvm one.

Thanks for testing.


Bjørn

 drivers/net/usb/cdc_mbim.c  |   12 +----------
 drivers/net/usb/cdc_ncm.c   |   49 ++++++++++++++++++++++++++++---------------
 include/linux/usb/cdc_ncm.h |    1 +
 3 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 248d2dc..70fd7c6 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -62,24 +62,14 @@ static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
 	return cdc_mbim_manage_power(dev, status);
 }
 
-
 static int cdc_mbim_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	struct cdc_ncm_ctx *ctx;
 	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
 	int ret = -ENODEV;
-	u8 data_altsetting = CDC_NCM_DATA_ALTSETTING_NCM;
+	u8 data_altsetting = cdc_ncm_select_altsetting(dev, intf);
 	struct cdc_mbim_state *info = (void *)&dev->data;
 
-	/* see if interface supports MBIM alternate setting */
-	if (intf->num_altsetting == 2) {
-		if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
-			usb_set_interface(dev->udev,
-					  intf->cur_altsetting->desc.bInterfaceNumber,
-					  CDC_NCM_COMM_ALTSETTING_MBIM);
-		data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
-	}
-
 	/* Probably NCM, defer for cdc_ncm_bind */
 	if (!cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
 		goto err;
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 61b74a2..8b6bd1b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -55,6 +55,14 @@
 
 #define	DRIVER_VERSION				"14-Mar-2012"
 
+#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
+static bool prefer_mbim = true;
+#else
+static bool prefer_mbim;
+#endif
+module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM on NCM/MBIM backwards compatible functions");
+
 static void cdc_ncm_txpath_bh(unsigned long param);
 static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
@@ -550,9 +558,12 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_unbind);
 
-static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+/* Select the MBIM altsetting iff it is preferred and available,
+ * returning the number of the corresponding data interface altsetting
+ */
+u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret;
+	struct usb_host_interface *alt;
 
 	/* The MBIM spec defines a NCM compatible default altsetting,
 	 * which we may have matched:
@@ -568,23 +579,27 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 	 *   endpoint descriptors, shall be constructed according to
 	 *   the rules given in section 6 (USB Device Model) of this
 	 *   specification."
-	 *
-	 * Do not bind to such interfaces, allowing cdc_mbim to handle
-	 * them
 	 */
-#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
-	if ((intf->num_altsetting == 2) &&
-	    !usb_set_interface(dev->udev,
-			       intf->cur_altsetting->desc.bInterfaceNumber,
-			       CDC_NCM_COMM_ALTSETTING_MBIM)) {
-		if (cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
-			return -ENODEV;
-		else
-			usb_set_interface(dev->udev,
-					  intf->cur_altsetting->desc.bInterfaceNumber,
-					  CDC_NCM_COMM_ALTSETTING_NCM);
+	if (prefer_mbim && intf->num_altsetting == 2) {
+		alt = usb_altnum_to_altsetting(intf, CDC_NCM_COMM_ALTSETTING_MBIM);
+		if (alt && cdc_ncm_comm_intf_is_mbim(alt) &&
+		    !usb_set_interface(dev->udev,
+				       intf->cur_altsetting->desc.bInterfaceNumber,
+				       CDC_NCM_COMM_ALTSETTING_MBIM))
+			return CDC_NCM_DATA_ALTSETTING_MBIM;
 	}
-#endif
+	return CDC_NCM_DATA_ALTSETTING_NCM;
+}
+EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting);
+
+static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	int ret;
+
+	/* MBIM backwards compatible function? */
+	cdc_ncm_select_altsetting(dev, intf);
+	if (cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
+		return -ENODEV;
 
 	/* NCM data altsetting is always 1 */
 	ret = cdc_ncm_bind_common(dev, intf, 1);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 3b8f9d4..cc25b70 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -127,6 +127,7 @@ struct cdc_ncm_ctx {
 	u16 connected;
 };
 
+extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
 extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
 extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tim Gardner @ 2013-03-10 18:20 UTC (permalink / raw)
  To: Tom Tucker
  Cc: linux-kernel, Trond Myklebust, J. Bruce Fields, David S. Miller,
	Tom Tucker, Haggai Eran, Or Gerlitz, Shani Michaeli, linux-nfs,
	netdev, Steve Dickson
In-Reply-To: <513CBFD6.1000504@opengridcomputing.com>

On 03/10/2013 11:16 AM, Tom Tucker wrote:
> 
> This is the result of 2773395b34883fe54418de188733a63bb38e0ad6. Steve
> might want to weigh in on this since it was done for performance reasons.
> 
> Tom
> 

It seems to me that the cost of a kmalloc()/kfree() is negligible
compared to network speeds.

rtg
-- 
Tim Gardner tim.gardner@canonical.com

^ permalink raw reply

* Re: [PATCH 1/3] bcm63xx_enet: use managed io memory allocations
From: Kevin Cernekee @ 2013-03-10 19:59 UTC (permalink / raw)
  To: Jonas Gorski; +Cc: netdev, David S. Miller, Maxime Bizon, Florian Fainelli
In-Reply-To: <1362923869-21346-1-git-send-email-jogo@openwrt.org>

On Sun, Mar 10, 2013 at 6:57 AM, Jonas Gorski <jogo@openwrt.org> wrote:
> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
> ---
>  drivers/net/ethernet/broadcom/bcm63xx_enet.c |   43 +++++---------------------
>  1 file changed, 7 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c

For the whole series:

Acked-by: Kevin Cernekee <cernekee@gmail.com>

^ permalink raw reply

* Re: [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: J. Bruce Fields @ 2013-03-10 20:28 UTC (permalink / raw)
  To: Tim Gardner
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
	David S. Miller, Tom Tucker, Haggai Eran, Or Gerlitz,
	Shani Michaeli, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1362929953-63785-1-git-send-email-tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

On Sun, Mar 10, 2013 at 09:39:13AM -0600, Tim Gardner wrote:
> rpcrdma_register_default_external() is several frames into
> the call stack which goes deeper yet. You run the risk of stack
> corruption by declaring such a large automatic variable,
> so dynamically allocate the array of 'struct ib_phys_buf' objects in
> order to silence the frame-larger-than warning.
> 
> net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
> net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 
> gcc version 4.6.3
> 
> Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
> Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>  net/sunrpc/xprtrdma/verbs.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 93726560..0916467 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -1736,9 +1736,13 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
>  	int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
>  				  IB_ACCESS_REMOTE_READ);
>  	struct rpcrdma_mr_seg *seg1 = seg;
> -	struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
> +	struct ib_phys_buf *ipb;
>  	int len, i, rc = 0;
>  
> +	ipb = kmalloc(sizeof(*ipb) * RPCRDMA_MAX_DATA_SEGS, GFP_KERNEL);

Have you checked that this occurs in a context where allocations are OK?
Checking very quickly through the callers I can't see any spinlocks or
anything, but I also don't see any other allocations.

Assuming this is just in rpciod context....  Trond's the authority, but
I think we generally try to avoid allocations here, or make them
GFP_NOFS if we must.

Would it be possible to allocate this array as part of the rpcrdma_req?

--b.

> +	if (!ipb)
> +		return -ENOMEM;
> +
>  	if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
>  		*nsegs = RPCRDMA_MAX_DATA_SEGS;
>  	for (len = 0, i = 0; i < *nsegs;) {
> @@ -1770,6 +1774,7 @@ rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
>  		seg1->mr_len = len;
>  	}
>  	*nsegs = i;
> +	kfree(ipb);
>  	return rc;
>  }
>  
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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] rrunner.c: fix possible memory leak in rr_init_one()
From: David Miller @ 2013-03-10 20:43 UTC (permalink / raw)
  To: daveo; +Cc: netdev, jes
In-Reply-To: <1362767295-16334-1-git-send-email-daveo@ll.mit.edu>

From: <daveo@ll.mit.edu>
Date: Fri, 8 Mar 2013 13:28:15 -0500

> From: David Oostdyk <daveo@ll.mit.edu>
> 
> In the event that register_netdev() failed, the rrpriv->evt_ring
> allocation would have not been freed.
> 
> Signed-off-by: David Oostdyk <daveo@ll.mit.edu>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH V1] ks8851_mll: basic ethernet statistics
From: David Miller @ 2013-03-10 20:44 UTC (permalink / raw)
  To: David.Choi; +Cc: netdev, Charles.Li
In-Reply-To: <FD9AD8C5375B924CABC56D982DB3A8020B4F7D67@EXMB1.micrel.com>

From: "Choi, David" <David.Choi@Micrel.Com>
Date: Fri, 8 Mar 2013 22:58:20 +0000

>  	while (ks->frame_cnt--) {
> +		if (unlikely(!(frame_hdr->sts & RXFSHR_RXFV) ||
> +		    frame_hdr->len >= RX_BUF_SIZE ||
> +		    frame_hdr->len <= 0)) {

You need to indent the last two lines so that the first character
lines up with the openning parenthesis of the unlikely() not
the if() statement.

^ permalink raw reply

* Re: [PATCH] ipv6: remove superfluous nla_data() NULL pointer checks
From: David Miller @ 2013-03-10 20:47 UTC (permalink / raw)
  To: minipli; +Cc: netdev
In-Reply-To: <1362844620-13035-1-git-send-email-minipli@googlemail.com>

From: Mathias Krause <minipli@googlemail.com>
Date: Sat,  9 Mar 2013 16:57:00 +0100

> nla_data() cannot return NULL, so these NULL pointer checks are
> superfluous.
> 
> Signed-off-by: Mathias Krause <minipli@googlemail.com>

Applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCH net] 6lowpan: Fix endianness issue in is_addr_link_local().
From: David Miller @ 2013-03-10 20:49 UTC (permalink / raw)
  To: yoshfuji; +Cc: linux-zigbee-devel, netdev
In-Reply-To: <513B897D.2000207@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Sun, 10 Mar 2013 04:11:57 +0900

> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next] don't compare skb->network_header with skb->tail
From: David Miller @ 2013-03-10 20:52 UTC (permalink / raw)
  To: honkiko; +Cc: netdev, stephen, zhiguo.hong
In-Reply-To: <1362904927-44673-1-git-send-email-honkiko@gmail.com>

From: Hong Zhiguo <honkiko@gmail.com>
Date: Sun, 10 Mar 2013 16:42:07 +0800

> in the case of NET_SKBUFF_DATA_USES_OFFSET(on 64-bit arch),
> skb->network_header is just offset over skb->head.
> 
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

So is skb->tail, so there is no problem.

^ permalink raw reply

* Re: [Patch net-next] ipv6: introduce ip6tunnel_xmit() helper
From: David Miller @ 2013-03-10 20:54 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, eric.dumazet, pshelar
In-Reply-To: <1362906039-8236-1-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Sun, 10 Mar 2013 17:00:39 +0800

> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> Similar to iptunnel_xmit(), group these operations into a
> helper function.
> 
> This by the way fixes the missing u64_stats_update_begin()
> and u64_stats_update_end() for 32 bit arch.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Pravin B Shelar <pshelar@nicira.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Looks good, applied, thanks.

^ permalink raw reply

* Re: [PATCH] eicon: Fixed checkpatch warning
From: David Miller @ 2013-03-10 20:56 UTC (permalink / raw)
  To: andreea.cristina.hodea; +Cc: mac, isdn, netdev, linux-kernel, hodea_andreea
In-Reply-To: <1362918876-18694-1-git-send-email-hodea_andreea@yahoo.com>

From: Andreea Hodea <andreea.cristina.hodea@gmail.com>
Date: Sun, 10 Mar 2013 14:34:36 +0200

> drivers/isdn/hardware/eicon/diva_didd.c:32:6: warning: symbol
> 'DRIVERRELEASE_DIDD' was not declared. Should it be static?
> 
> Signed-off-by: Andreea Hodea <hodea_andreea@yahoo.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 1/3] bcm63xx_enet: use managed io memory allocations
From: David Miller @ 2013-03-10 20:57 UTC (permalink / raw)
  To: cernekee; +Cc: jogo, netdev, mbizon, florian
In-Reply-To: <CAJiQ=7Cz0e2q61X7gsnistP7BB2wv=U-QFoFWGZ+WZgnfzHz0A@mail.gmail.com>

From: Kevin Cernekee <cernekee@gmail.com>
Date: Sun, 10 Mar 2013 12:59:38 -0700

> On Sun, Mar 10, 2013 at 6:57 AM, Jonas Gorski <jogo@openwrt.org> wrote:
>> Signed-off-by: Jonas Gorski <jogo@openwrt.org>
>> ---
>>  drivers/net/ethernet/broadcom/bcm63xx_enet.c |   43 +++++---------------------
>>  1 file changed, 7 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
> 
> For the whole series:
> 
> Acked-by: Kevin Cernekee <cernekee@gmail.com>

Series applied to net-next, thanks.

^ permalink raw reply

* Re: [PATCHv2 net-next 01/15] net: add skb_dst_set_unref
From: David Miller @ 2013-03-10 21:00 UTC (permalink / raw)
  To: ja; +Cc: horms, lvs-devel, netdev
In-Reply-To: <alpine.LFD.2.00.1303101527530.1608@ja.ssi.bg>

From: Julian Anastasov <ja@ssi.bg>
Date: Sun, 10 Mar 2013 15:37:34 +0200 (EET)

> 	The idea looks good, here is the implementation.
> Can I use it in this form for next patchset versions?

Yes, but with one minor change:

> -	if (unlikely(dst->flags & DST_NOCACHE)) {
> +	if (unlikely(dst->flags & DST_NOCACHE && !force)) {

I keep forgetting the && vs. & operator precedence, and so
do many other people, so please put parenthesis around
the "dst->flags & DST_NOCACHE" expression, thanks.

^ permalink raw reply

* Re: [PATCH net-next] Remove unused tw_cookie_values from tcp_timewait_sock
From: David Miller @ 2013-03-10 21:09 UTC (permalink / raw)
  To: eric.dumazet; +Cc: christoph.paasch, netdev, william.allen.simpson
In-Reply-To: <1362931570.4051.34.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 10 Mar 2013 17:06:10 +0100

> On Sun, 2013-03-10 at 16:18 +0100, Christoph Paasch wrote:
>> tw_cookie_values is never used in the TCP-stack.
>> 
>> It was added by 435cf559f (TCPCT part 1d: define TCP cookie option,
>> extend existing struct's), but already at that time it was not used at
>> all, nor mentioned in the commit-message.
>> 
>> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
 ...
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: RFC: [PATCH 1/3] usb: cdc_ncm: patch for VMware
From: Loic Domaigne @ 2013-03-10 21:12 UTC (permalink / raw)
  To: Dan Williams
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1362781739.8581.5.camel-wKZy7rqYPVb5EHUCmHmTqw@public.gmane.org>

On Fri, Mar 08, 2013 at 04:28:59PM -0600, Dan Williams wrote:
> On Fri, 2013-03-08 at 22:03 +0100, Loic Domaigne wrote:
> >  
> > +/* maximum Rx URB size */
> > +/*
> > + * in the original Linux driver, the rx urb size can be up to
> > + * CDC_NCM_NTB_MAX_SIZE_RX.
> > + *
> > + * Under VMware (as of wks9), URB size greater than 16kB is a problem,
> > + * so simply adjust this define when the driver is compiled for a VMware
> > + * environment.
> > + *
> > + */
> > +#ifdef VMWARE_BUG
> > +#warning "Compiling for VMware"
> > +#define CDC_NCM_MAX_RX_URB_SIZE     16384
> > +#else
> > +#define CDC_NCM_MAX_RX_URB_SIZE     CDC_NCM_NTB_MAX_SIZE_RX
> > +#endif
> 
> I can't see how that is going to get past any sort of review.  Either
> there's some other way of detecting that the CPU is the VMWare emulated
> one or you're stuck with the bug until VMWare fixes it.

Yeah, I know.

The kludge consists to (re)compile the kernel module on the VMWare guest with
the VMWARE_BUG compiler flag set. 

We have a helper script for that task, but it's distros specific. We can 
detect automatically a VMWare emulated CPU in some cases, but not always.
As a result, we end up sometimes asking the user.

I am aware that it's not suitable as a generic solution. But waiting a fix
from VMWare might not be practical for you either.

Any better ideas?

Loic.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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

* [PATCHv2] net: can: af_can.c: Fix checkpatch warnings
From: Valentin Ilie @ 2013-03-10 21:15 UTC (permalink / raw)
  To: socketcan, davem; +Cc: linux-can, netdev, linux-kernel, Valentin Ilie
In-Reply-To: <1362918526-26730-1-git-send-email-valentin.ilie@gmail.com>

Replace printk(KERN_ERR with pr_err
Add space before {
Removed OOM messages

Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
---
 net/can/af_can.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index c48e522..8bacf28 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -525,7 +525,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 
 	d = find_dev_rcv_lists(dev);
 	if (!d) {
-		printk(KERN_ERR "BUG: receive list not found for "
+		pr_err("BUG: receive list not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		goto out;
@@ -552,7 +552,7 @@ void can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
 	 */
 
 	if (!r) {
-		printk(KERN_ERR "BUG: receive list entry not found for "
+		pr_err("BUG: receive list entry not found for "
 		       "dev %s, id %03X, mask %03X\n",
 		       DNAME(dev), can_id, mask);
 		r = NULL;
@@ -749,8 +749,7 @@ int can_proto_register(const struct can_proto *cp)
 	int err = 0;
 
 	if (proto < 0 || proto >= CAN_NPROTO) {
-		printk(KERN_ERR "can: protocol number %d out of range\n",
-		       proto);
+		pr_err("can: protocol number %d out of range\n", proto);
 		return -EINVAL;
 	}
 
@@ -761,8 +760,7 @@ int can_proto_register(const struct can_proto *cp)
 	mutex_lock(&proto_tab_lock);
 
 	if (proto_tab[proto]) {
-		printk(KERN_ERR "can: protocol %d already registered\n",
-		       proto);
+		pr_err("can: protocol %d already registered\n", proto);
 		err = -EBUSY;
 	} else
 		RCU_INIT_POINTER(proto_tab[proto], cp);
@@ -816,11 +814,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 
 		/* create new dev_rcv_lists for this device */
 		d = kzalloc(sizeof(*d), GFP_KERNEL);
-		if (!d) {
-			printk(KERN_ERR
-			       "can: allocation of receive list failed\n");
+		if (!d)
 			return NOTIFY_DONE;
-		}
 		BUG_ON(dev->ml_priv);
 		dev->ml_priv = d;
 
@@ -838,8 +833,8 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 				dev->ml_priv = NULL;
 			}
 		} else
-			printk(KERN_ERR "can: notifier: receive list not "
-			       "found for dev %s\n", dev->name);
+			pr_err("can: notifier: receive list not found for dev "
+			       "%s\n", dev->name);
 
 		spin_unlock(&can_rcvlists_lock);
 
@@ -927,7 +922,7 @@ static __exit void can_exit(void)
 	/* remove created dev_rcv_lists from still registered CAN devices */
 	rcu_read_lock();
 	for_each_netdev_rcu(&init_net, dev) {
-		if (dev->type == ARPHRD_CAN && dev->ml_priv){
+		if (dev->type == ARPHRD_CAN && dev->ml_priv) {
 
 			struct dev_rcv_lists *d = dev->ml_priv;
 
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCH] eicon: Fixed checkpatch warning
From: David Miller @ 2013-03-10 22:02 UTC (permalink / raw)
  To: andreea.cristina.hodea; +Cc: mac, isdn, netdev, linux-kernel, hodea_andreea
In-Reply-To: <20130310.165613.427736360718722534.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 10 Mar 2013 16:56:13 -0400 (EDT)

> From: Andreea Hodea <andreea.cristina.hodea@gmail.com>
> Date: Sun, 10 Mar 2013 14:34:36 +0200
> 
>> drivers/isdn/hardware/eicon/diva_didd.c:32:6: warning: symbol
>> 'DRIVERRELEASE_DIDD' was not declared. Should it be static?
>> 
>> Signed-off-by: Andreea Hodea <hodea_andreea@yahoo.com>
> 
> Applied, thanks.

I had to revert, did you even try to compile the driver after
this change?

The symbol is refenced by DbgRegister() calls in diddfunc.c, and
a very simple grep would have shown that to you.

Please do not just blindly try to fix checkpatch warnings.

^ permalink raw reply

* Re: [PATCHv2 net-next 01/15] net: add skb_dst_set_unref
From: Julian Anastasov @ 2013-03-10 22:05 UTC (permalink / raw)
  To: David Miller; +Cc: horms, lvs-devel, netdev
In-Reply-To: <20130310.170004.2239870728229481365.davem@davemloft.net>


	Hello,

On Sun, 10 Mar 2013, David Miller wrote:

> From: Julian Anastasov <ja@ssi.bg>
> Date: Sun, 10 Mar 2013 15:37:34 +0200 (EET)
> 
> > 	The idea looks good, here is the implementation.
> > Can I use it in this form for next patchset versions?
> 
> Yes, but with one minor change:
> 
> > -	if (unlikely(dst->flags & DST_NOCACHE)) {
> > +	if (unlikely(dst->flags & DST_NOCACHE && !force)) {
> 
> I keep forgetting the && vs. & operator precedence, and so
> do many other people, so please put parenthesis around
> the "dst->flags & DST_NOCACHE" expression, thanks.

	Done, I'll wait a week for more feedback...

Regards

--
Julian Anastasov <ja@ssi.bg>

^ 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