Netdev List
 help / color / mirror / Atom feed
* [PATCH] net/hyperv: Fix the stop/wake queue mechanism
From: Haiyang Zhang @ 2011-12-02 19:56 UTC (permalink / raw)
  To: haiyangz, kys, davem, gregkh, linux-kernel, netdev, devel

The ring buffer is only used to pass meta data for outbound packets. The
actual payload is accessed by DMA from the host. So the stop/wake queue
mechanism based on counting and comparing number of pages sent v.s. number
of pages in the ring buffer is wrong. Also, there is a race condition in
the stop/wake queue calls, which can stop xmit queue forever.

The new stop/wake queue mechanism is based on the actual bytes used by
outbound packets in the ring buffer. The check for number of outstanding
sends after stop queue prevents the race condition that can cause wake
queue happening earlier than stop queue.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reported-by: Long Li <longli@microsoft.com>
---
 drivers/net/hyperv/netvsc.c     |   14 +++++++++++---
 drivers/net/hyperv/netvsc_drv.c |   24 +-----------------------
 2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 4a807e4..b6ac152 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -435,6 +435,9 @@ static void netvsc_send_completion(struct hv_device *device,
 			nvsc_packet->completion.send.send_completion_ctx);
 
 		atomic_dec(&net_device->num_outstanding_sends);
+
+		if (netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
 	} else {
 		netdev_err(ndev, "Unknown send completion packet type- "
 			   "%d received!!\n", nvsp_packet->hdr.msg_type);
@@ -485,11 +488,16 @@ int netvsc_send(struct hv_device *device,
 
 	}
 
-	if (ret != 0)
+	if (ret == 0) {
+		atomic_inc(&net_device->num_outstanding_sends);
+	} else if (ret == -EAGAIN) {
+		netif_stop_queue(ndev);
+		if (atomic_read(&net_device->num_outstanding_sends) < 1)
+			netif_wake_queue(ndev);
+	} else {
 		netdev_err(ndev, "Unable to send packet %p ret %d\n",
 			   packet, ret);
-	else
-		atomic_inc(&net_device->num_outstanding_sends);
+	}
 
 	return ret;
 }
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index b69c3a4..7da85eb 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -43,15 +43,10 @@
 struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
-	atomic_t avail;
 	struct delayed_work dwork;
 };
 
 
-#define PACKET_PAGES_LOWATER  8
-/* Need this many pages to handle worst case fragmented packet */
-#define PACKET_PAGES_HIWATER  (MAX_SKB_FRAGS + 2)
-
 static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
 MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
@@ -144,18 +139,8 @@ static void netvsc_xmit_completion(void *context)
 
 	kfree(packet);
 
-	if (skb) {
-		struct net_device *net = skb->dev;
-		struct net_device_context *net_device_ctx = netdev_priv(net);
-		unsigned int num_pages = skb_shinfo(skb)->nr_frags + 2;
-
+	if (skb)
 		dev_kfree_skb_any(skb);
-
-		atomic_add(num_pages, &net_device_ctx->avail);
-		if (atomic_read(&net_device_ctx->avail) >=
-				PACKET_PAGES_HIWATER)
-			netif_wake_queue(net);
-	}
 }
 
 static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
@@ -167,8 +152,6 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 
 	/* Add 1 for skb->data and additional one for RNDIS */
 	num_pages = skb_shinfo(skb)->nr_frags + 1 + 1;
-	if (num_pages > atomic_read(&net_device_ctx->avail))
-		return NETDEV_TX_BUSY;
 
 	/* Allocate a netvsc packet based on # of frags. */
 	packet = kzalloc(sizeof(struct hv_netvsc_packet) +
@@ -218,10 +201,6 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	if (ret == 0) {
 		net->stats.tx_bytes += skb->len;
 		net->stats.tx_packets++;
-
-		atomic_sub(num_pages, &net_device_ctx->avail);
-		if (atomic_read(&net_device_ctx->avail) < PACKET_PAGES_LOWATER)
-			netif_stop_queue(net);
 	} else {
 		/* we are shutting down or bus overloaded, just drop packet */
 		net->stats.tx_dropped++;
@@ -391,7 +370,6 @@ static int netvsc_probe(struct hv_device *dev,
 
 	net_device_ctx = netdev_priv(net);
 	net_device_ctx->device_ctx = dev;
-	atomic_set(&net_device_ctx->avail, ring_size);
 	hv_set_drvdata(dev, net);
 	INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_send_garp);
 
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH] infiniband: addr: Avoid unnecessary neigh lookup.
From: David Miller @ 2011-12-02 19:55 UTC (permalink / raw)
  To: roland; +Cc: netdev


The explicit neigh_lookup() in addr4_resolve() is unnecessary because this
is the exact same calculation the routing code already made to attach the
neigh to the route.

Therefore, just use dst_get_neighbour().

Signed-off-by: David S. Miller <davem@davemloft.net>
---

Roland, I'm auditing all of the uses of dst_get_neighbour() with the
end result being that a dst_put_neighbour() interface will be added
and all access will be reduced to quick:

	rcu_read_lock();
	n = dst_get_neighbour();
	...
	dst_put_neighbour();
	rcu_read_unlock();

sequences.  And then, at the next step, things will be further
whittled down into a refcount-less variant:

	rcu_read_lock();
	n = dst_get_neighbour_noref();
	...
	rcu_read_unlock();

Another key difference is that we are making the code able to handle
dst_get_neighbour() returning NULL.

So if it's OK with you I'd like to merge this stuff directly in the
net-next tree after getting your ACK on each patch.

In the case here, the neigh_lookup() call is spurious and is exactly
mimicking the neigh lookup that the ipv4 routing code does to attach
the neighbour to the route.  There is no need to duplicate that
operation, and we can thus use the route attached neigh directly.

 drivers/infiniband/core/addr.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index a20c3c8..83d88e1 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -214,20 +214,19 @@ static int addr4_resolve(struct sockaddr_in *src_in,
 		goto put;
 	}
 
-	neigh = neigh_lookup(&arp_tbl, &rt->rt_gateway, rt->dst.dev);
-	if (!neigh || !(neigh->nud_state & NUD_VALID)) {
-		rcu_read_lock();
-		neigh_event_send(dst_get_neighbour(&rt->dst), NULL);
-		rcu_read_unlock();
-		ret = -ENODATA;
-		if (neigh)
-			goto release;
-		goto put;
+	rcu_read_lock();
+	neigh = dst_get_neighbour(&rt->dst);
+	ret = -ENODATA;
+	if (!neigh)
+		goto unlock_put;
+	if (!(neigh->nud_state & NUD_VALID)) {
+		neigh_event_send(neigh, NULL);
+		goto unlock_put;
 	}
 
 	ret = rdma_copy_addr(addr, neigh->dev, neigh->ha);
-release:
-	neigh_release(neigh);
+unlock_put:
+	rcu_read_unlock();
 put:
 	ip_rt_put(rt);
 out:
-- 
1.7.7.3

^ permalink raw reply related

* Re: WARNING: at mm/slub.c:3357, kernel BUG at mm/slub.c:3413
From: Jerome Glisse @ 2011-12-02 19:43 UTC (permalink / raw)
  To: Markus Trippelsdorf
  Cc: Christoph Lameter, Alex, Shi, Dave Airlie, Eric Dumazet,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel,
	Pekka Enberg, linux-mm@kvack.org, Matt Mackall, tj, Alex Deucher
In-Reply-To: <20111201084437.GA1529@x4.trippels.de>

On Thu, Dec 01, 2011 at 09:44:37AM +0100, Markus Trippelsdorf wrote:
> On 2011.11.24 at 09:50 +0100, Markus Trippelsdorf wrote:
> > On 2011.11.23 at 10:06 -0600, Christoph Lameter wrote:
> > > On Wed, 23 Nov 2011, Markus Trippelsdorf wrote:
> > > 
> > > > > FIX idr_layer_cache: Marking all objects used
> > > >
> > > > Yesterday I couldn't reproduce the issue at all. But today I've hit
> > > > exactly the same spot again. (CCing the drm list)
> > > 
> > > Well this is looks like write after free.
> > > 
> > > > =============================================================================
> > > > BUG idr_layer_cache: Poison overwritten
> > > > -----------------------------------------------------------------------------
> > > > Object ffff8802156487c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
> > > > Object ffff8802156487d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
> > > > Object ffff8802156487e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
> > > > Object ffff8802156487f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
> > > > Object ffff880215648800: 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  ....kkkkkkkkkkkk
> > > > Object ffff880215648810: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
> > > 
> > > And its an integer sized write of 0. If you look at the struct definition
> > > and lookup the offset you should be able to locate the field that
> > > was modified.
> 
> It also happens with CONFIG_SLAB. 
> (If someone wants to reproduce the issue, just run a kexec boot loop and
> the bug will occur after a few (~10) iterations.)
> 

Can you provide the kexec command line you are using and full kernel
log (mostly interested in kernel option).

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH] atm: clip: Remove code commented out since eternity.
From: David Miller @ 2011-12-02 19:27 UTC (permalink / raw)
  To: netdev


Signed-off-by: David S. Miller <davem@davemloft.net>
---

Saw this while auditing dst_set_neighbour() call sites.

 net/atm/clip.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index f3b3615..c84ce7f 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -340,15 +340,6 @@ static netdev_tx_t clip_start_xmit(struct sk_buff *skb,
 	}
 	n = dst_get_neighbour(dst);
 	if (!n) {
-#if 0
-		n = clip_find_neighbour(skb_dst(skb), 1);
-		if (!n) {
-			dev_kfree_skb(skb);	/* lost that one */
-			dev->stats.tx_dropped++;
-			return 0;
-		}
-		dst_set_neighbour(dst, n);
-#endif
 		pr_err("NO NEIGHBOUR !\n");
 		dev_kfree_skb(skb);
 		dev->stats.tx_dropped++;
-- 
1.7.7.3

^ permalink raw reply related

* Re: BUG: unable to handle kernel NULL pointer dereference at 000000000000002c
From: Greg KH @ 2011-12-02 18:53 UTC (permalink / raw)
  To: s.priebe; +Cc: David Miller, eric.dumazet, jwboyer, hch, netdev, david, stable
In-Reply-To: <20111202.123435.740930342839308437.davem@davemloft.net>

On Fri, Dec 02, 2011 at 12:34:35PM -0500, David Miller wrote:
> From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
> Date: Fri, 02 Dec 2011 08:19:05 +0100
> 
> > Am 02.12.2011 07:17, schrieb David Miller:
> >> From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
> >> Date: Fri, 02 Dec 2011 07:08:57 +0100
> >> 
> >>> So can someone please send them to greg => stable@ so we can get them
> >>> into 3.0.X longterm stable.
> >> 
> >> I'll do it, these things just take care and time.
> > 
> > Thanks! Could you please CC me?
> 
> Sorry, I'm not going to do that, the I'd have to do it for anyone who asks
> me to and that's not reasonable.

Stefan, the stable@vger.kernel.org list is public, you are free to
subscribe it if you are curious as to when patches are both submitted to
it, and when they are applied to the trees.

greg k-h

^ permalink raw reply

* Re: Bug in computing data_len in tcp_sendmsg?
From: David Miller @ 2011-12-02 18:40 UTC (permalink / raw)
  To: eric.dumazet; +Cc: subramanian.vijay, therbert, netdev
In-Reply-To: <1322850989.2762.47.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 02 Dec 2011 19:36:29 +0100

> What I ask now is following problem (even prior to my frag allocation
> patch) :
> 
> 1) We allocate a linear skb (SG being off) to cook a tcp frame of length
> XXX bytes.
> 
> 2) We send it.
> 
> 3) We receive an ACK for first 31 bytes.
> 
> 4) We trim 31 bytes from the head of skb.  (skb_pull(skb, 31))
>     skb->data is now not anymore aligned to a 4 bytes boundary.
> 
> 5) Later, we need to retransmit skb (XXX minus 31 bytes already ACKed)
>    We push TCP header, and skb->data is not aligned.
>    TCP header is not aligned anymore. x86 doesnt care, but what about
> other arches with misalign traps ?

Yes, for non-SG this always was technically possible.

But now you've made it possible for SG too.

Frankly, I think we should:

1) Revert your patch, so it's not possible for SG once more.

2) Add code to reallocate the SKB linear data when this happens in
   the non-SG case.

^ permalink raw reply

* Re: Bug in computing data_len in tcp_sendmsg?
From: Eric Dumazet @ 2011-12-02 18:36 UTC (permalink / raw)
  To: David Miller; +Cc: subramanian.vijay, therbert, netdev
In-Reply-To: <20111202.131304.981034697414242002.davem@davemloft.net>

Le vendredi 02 décembre 2011 à 13:13 -0500, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 02 Dec 2011 17:05:35 +0100
> 
> > Le vendredi 02 décembre 2011 à 16:44 +0100, Eric Dumazet a écrit :
> > 
> >> [PATCH net-next] tcp: fix tcp_trim_head()
> >> 
> >> commit f07d960df3 (tcp: avoid frag allocation for small frames)
> >> breaked assumption in tcp stack that skb is either linear (data_len ==
> >> 0), either fully fragged (data_len == len)
> >> 
> >> Thanks to Vijay for providing a very detailed explanation.
> >> 
> >> Reported-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >> ---
> > 
> > Another problem is the possible misalignement of skb->data if/when we
> > receive an ACK of an odd (not a 4 multiple) tcp sequence.
> > 
> > So when/if packet is restransmited, tcp header (and IP header as well)
> > could be misaligned.
> > 
> > Hmm, I probably miss something obvious, since this problem could happen
> > for linear skbs before my patch ?
> 
> Unfortunately, even if netfilter or the packet scheduler pulled data, this
> misalignment wouldn't happen before your change.
> 
> Maybe we should revert your frag allocation avoidance change until we can
> sort this out.

The patch I posted should solve the problem Vijay spotted.
(It really does, I tested it, allowing mtu probing)

What I ask now is following problem (even prior to my frag allocation
patch) :

1) We allocate a linear skb (SG being off) to cook a tcp frame of length
XXX bytes.

2) We send it.

3) We receive an ACK for first 31 bytes.

4) We trim 31 bytes from the head of skb.  (skb_pull(skb, 31))
    skb->data is now not anymore aligned to a 4 bytes boundary.

5) Later, we need to retransmit skb (XXX minus 31 bytes already ACKed)
   We push TCP header, and skb->data is not aligned.
   TCP header is not aligned anymore. x86 doesnt care, but what about
other arches with misalign traps ?

^ permalink raw reply

* net.ipv*.conf.all.* sysctls
From: Mathieu Trudel-Lapierre @ 2011-12-02 18:32 UTC (permalink / raw)
  To: netdev

Hi,

Rehashing something that's I've found quite a few times in a quick
search in the archives, but still a little unclear to me.

As previously discussed here and as I could test on my system (Ubuntu
3.2.0-2.5-generic 3.2.0-rc3), seems like the net.ipv6.conf.all sysctls
aren't actually being propagated to all devices. Is this indeed an
issue and something that needs to be fixed, or are these settings
really expected to be used as "global" parameters?

I've done a simple test: after applying
net.ipv6.conf.all.use_tempaddr=2 and
net.ipv6.conf.default.use_tempaddr=2 to sysctl.conf and rebooting, I
still see the value for
net.ipv6.conf.eth0.use_tempaddr as 0, while other devices (wlan0) are
properly set to 2. This is most likely caused by eth0 being
initialized prior to the sysctl.conf file being read and applied.

Connecting to a wired network still shows the interface eth0 as not
using temporary addresses, which should be happening if "all" is
really meant as a global value to override what might be set
per-interface.

Besides fixing up sysctls to be applied early enough to affect eth0 as
well on my system; isn't there something else that needs to be done to
clarify and fix the /all/ settings? I believe either the documentation
and actual functionality needs to be fixed to be a global value and
override per-interface values, or the functionality needs to be
corrected to propagate values to the underlying interfaces.

Am I missing something here? I'm totally new to kernel development,
but would be happy to provide simple patches once the expected
behaviour is clarified.

Regards,

Mathieu Trudel-Lapierre <mathieu.tl@gmail.com>
Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
4096R/EE018C93 1967 8F7D 03A1 8F38 732E  FF82 C126 33E1 EE01 8C93

^ permalink raw reply

* PRIVATE LOAN
From: BARCLAY @ 2011-12-02 17:48 UTC (permalink / raw)




Loan Application at 3%(serious inquiries only). If interested should contact us at: charlesbarclaycompany@gmail.com

^ permalink raw reply

* Re: Bug in computing data_len in tcp_sendmsg?
From: David Miller @ 2011-12-02 18:13 UTC (permalink / raw)
  To: eric.dumazet; +Cc: subramanian.vijay, therbert, netdev
In-Reply-To: <1322841935.2762.33.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 02 Dec 2011 17:05:35 +0100

> Le vendredi 02 décembre 2011 à 16:44 +0100, Eric Dumazet a écrit :
> 
>> [PATCH net-next] tcp: fix tcp_trim_head()
>> 
>> commit f07d960df3 (tcp: avoid frag allocation for small frames)
>> breaked assumption in tcp stack that skb is either linear (data_len ==
>> 0), either fully fragged (data_len == len)
>> 
>> Thanks to Vijay for providing a very detailed explanation.
>> 
>> Reported-by: Vijay Subramanian <subramanian.vijay@gmail.com>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
> 
> Another problem is the possible misalignement of skb->data if/when we
> receive an ACK of an odd (not a 4 multiple) tcp sequence.
> 
> So when/if packet is restransmited, tcp header (and IP header as well)
> could be misaligned.
> 
> Hmm, I probably miss something obvious, since this problem could happen
> for linear skbs before my patch ?

Unfortunately, even if netfilter or the packet scheduler pulled data, this
misalignment wouldn't happen before your change.

Maybe we should revert your frag allocation avoidance change until we can
sort this out.

^ permalink raw reply

* Re: [PATCH v7 10/10] Disable task moving when using kernel memory accounting
From: Glauber Costa @ 2011-12-02 18:11 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20111130112210.1d979512.kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

On 11/30/2011 12:22 AM, KAMEZAWA Hiroyuki wrote:
> On Tue, 29 Nov 2011 21:57:01 -0200
> Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
>
>> Since this code is still experimental, we are leaving the exact
>> details of how to move tasks between cgroups when kernel memory
>> accounting is used as future work.
>>
>> For now, we simply disallow movement if there are any pending
>> accounted memory.
>>
>> Signed-off-by: Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>> CC: Hiroyouki Kamezawa<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
>> ---
>>   mm/memcontrol.c |   23 ++++++++++++++++++++++-
>>   1 files changed, 22 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index a31a278..dd9a6d9 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -5453,10 +5453,19 @@ static int mem_cgroup_can_attach(struct cgroup_subsys *ss,
>>   {
>>   	int ret = 0;
>>   	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
>> +	struct mem_cgroup *from = mem_cgroup_from_task(p);
>> +
>> +#if defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM)&&  defined(CONFIG_INET)
>> +	if (from != memcg&&  !mem_cgroup_is_root(from)&&
>> +	    res_counter_read_u64(&from->tcp_mem.tcp_memory_allocated, RES_USAGE)) {
>> +		printk(KERN_WARNING "Can't move tasks between cgroups: "
>> +			"Kernel memory held.\n");
>> +		return 1;
>> +	}
>> +#endif
>
> I wonder....reading all codes again, this is incorrect check.
>
> Hm, let me cralify. IIUC, in old code, "prevent moving" is because you hold
> reference count of cgroup, which can cause trouble at rmdir() as leaking refcnt.
right.

> BTW, because socket is a shared resource between cgroup, changes in mm->owner
> may cause task cgroup moving implicitly. So, if you allow leak of resource
> here, I guess... you can take mem_cgroup_get() refcnt which is memcg-local and
> allow rmdir(). Then, this limitation may disappear.

Sorry, I didn't fully understand. Can you clarify further?
If the task is implicitly moved, it will end up calling can_attach as 
well, right?


>
> Then, users will be happy but admins will have unseen kernel resource usage in
> not populated(by rmdir) memcg. Hm, big trouble ?
>
> Thanks,
> -Kame
>

--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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 v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure
From: Glauber Costa @ 2011-12-02 18:04 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20111130111152.6b1c7366.kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
> On Tue, 29 Nov 2011 21:56:51 -0200
> Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
>
>> Hi,
>>
>> This patchset implements per-cgroup tcp memory pressure controls. It did not change
>> significantly since last submission: rather, it just merges the comments Kame had.
>> Most of them are style-related and/or Documentation, but there are two real bugs he
>> managed to spot (thanks)
>>
>> Please let me know if there is anything else I should address.
>>
>
> After reading all codes again, I feel some strange. Could you clarify ?
>
> Here.
> ==
> +void sock_update_memcg(struct sock *sk)
> +{
> +	/* right now a socket spends its whole life in the same cgroup */
> +	if (sk->sk_cgrp) {
> +		WARN_ON(1);
> +		return;
> +	}
> +	if (static_branch(&memcg_socket_limit_enabled)) {
> +		struct mem_cgroup *memcg;
> +
> +		BUG_ON(!sk->sk_prot->proto_cgroup);
> +
> +		rcu_read_lock();
> +		memcg = mem_cgroup_from_task(current);
> +		if (!mem_cgroup_is_root(memcg))
> +			sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg);
> +		rcu_read_unlock();
> ==
>
> sk->sk_cgrp is set to a memcg without any reference count.
>
> Then, no check for preventing rmdir() and freeing memcgroup.
>
> Is there some css_get() or mem_cgroup_get() somewhere ?
>

There were a css_get in the first version of this patchset. It was 
removed, however, because it was deemed anti-intuitive to prevent rmdir, 
since we can't know which sockets are blocking it, or do anything about 
it. Or did I misunderstand something ?
--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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 v2] net/macb: add DT support
From: David Miller @ 2011-12-02 17:58 UTC (permalink / raw)
  To: nicolas.ferre
  Cc: netdev, devicetree-discuss, linux-kernel, grant.likely, jamie,
	plagnioj, linux-arm-kernel
In-Reply-To: <1322847782-22650-1-git-send-email-nicolas.ferre@atmel.com>

From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Fri,  2 Dec 2011 18:43:02 +0100

> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Allow the device tree to provide the mac address and the phy mode.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> [nicolas.ferre@atmel.com: change "compatible" node property, doc and DT hwaddr]
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Jamie Iles <jamie@jamieiles.com>
> ---
> v2: modify macb_get_hwaddr_dt() parameter

You'll have to respin these two macb patches, as they don't apply properly
to the net-next tree which is where they should be targetted.

Thanks.

^ permalink raw reply

* Re: [PATCH v7 04/10] tcp memory pressure controls
From: Glauber Costa @ 2011-12-02 17:57 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA, KAMEZAWA Hiroyuki
In-Reply-To: <20111130104943.d9b210ee.kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>

On 11/29/2011 11:49 PM, KAMEZAWA Hiroyuki wrote:
>
>> -static struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>> +struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
>>   {
>>   	return container_of(cgroup_subsys_state(cont,
>>   				mem_cgroup_subsys_id), struct mem_cgroup,
>> @@ -4717,14 +4732,27 @@ static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
>>
>>   	ret = cgroup_add_files(cont, ss, kmem_cgroup_files,
>>   			       ARRAY_SIZE(kmem_cgroup_files));
>> +
>> +	if (!ret)
>> +		ret = mem_cgroup_sockets_init(cont, ss);
>>   	return ret;
>>   };
>
> You does initizalication here. The reason what I think is
> 1. 'proto_list' is not available at createion of root cgroup and
>      you need to delay set up until mounting.
>
> If so, please add comment or find another way.
> This seems not very clean to me.

Yes, we do can run into some ordering issues. A part of the 
initialization can be done earlier. But I preferred to move it all later
instead of creating two functions for it. But I can change that if you 
want, no big deal.

>
>
>
>
>> +static DEFINE_RWLOCK(proto_list_lock);
>> +static LIST_HEAD(proto_list);
>> +
>> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
>> +int mem_cgroup_sockets_init(struct cgroup *cgrp, struct cgroup_subsys *ss)
>> +{
>> +	struct proto *proto;
>> +	int ret = 0;
>> +
>> +	read_lock(&proto_list_lock);
>> +	list_for_each_entry(proto,&proto_list, node) {
>> +		if (proto->init_cgroup)
>> +			ret = proto->init_cgroup(cgrp, ss);
>> +			if (ret)
>> +				goto out;
>> +	}
>
> seems indent is bad or {} is missing.
>
Thanks. I'll rewrite it, since I did miss {} around the first if. But no 
test could possibly catch it, since what I wanted to write, and what I 
wrote by mistake end up being equivalent.

>> +EXPORT_SYMBOL(memcg_tcp_enter_memory_pressure);
>> +
>> +int tcp_init_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss)
>> +{
>> +	/*
>> +	 * The root cgroup does not use res_counters, but rather,
>> +	 * rely on the data already collected by the network
>> +	 * subsystem
>> +	 */
>> +	struct res_counter *res_parent = NULL;
>> +	struct cg_proto *cg_proto;
>> +	struct tcp_memcontrol *tcp;
>> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp);
>> +	struct mem_cgroup *parent = parent_mem_cgroup(memcg);
>> +
>> +	cg_proto = tcp_prot.proto_cgroup(memcg);
>> +	if (!cg_proto)
>> +		return 0;
>> +
>> +	tcp = tcp_from_cgproto(cg_proto);
>> +	cg_proto->parent = tcp_prot.proto_cgroup(parent);
>> +
>> +	tcp->tcp_prot_mem[0] = sysctl_tcp_mem[0];
>> +	tcp->tcp_prot_mem[1] = sysctl_tcp_mem[1];
>> +	tcp->tcp_prot_mem[2] = sysctl_tcp_mem[2];
>> +	tcp->tcp_memory_pressure = 0;
>
> Question:
>
> Is this value will be updated when an admin chages sysctl ?

yes.

> I guess, this value is set at system init script or some which may
> happen later than mounting cgroup.
> I don't like to write a guideline 'please set sysctl val before
> mounting cgroup'

Agreed.

This code is in patch 6 (together with the limiting):

+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+       rcu_read_lock();
+       memcg = mem_cgroup_from_task(current);
+
+       tcp_prot_mem(memcg, vec[0], 0);
+       tcp_prot_mem(memcg, vec[1], 1);
+       tcp_prot_mem(memcg, vec[2], 2);
+       rcu_read_unlock();
+#endif

tcp_prot_mem is just a wrapper around the assignment so we can access 
memcg's inner fields.

--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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: pull request: batman-adv 2011-11-26
From: David Miller @ 2011-12-02 17:57 UTC (permalink / raw)
  To: ordex-GaUfNO9RBHfsrOwW+9ziJQ
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	lindner_marek-LWAfsSFWpa4
In-Reply-To: <20111202171216.GA4561-E/2OGukznS5g9hUCZPvPmw@public.gmane.org>

From: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
Date: Fri, 2 Dec 2011 18:12:16 +0100

> First of all I think you are referring to patch 08/10, in which I moved a
> skb_linearise() operation.
> 
> To be sure it is really needed, I backtracked the code flow and noted down any
> eventual psbk_may_pull() (or any other linearisation operation). The result is:
> 
> => in batman_skb_recv()
> 	- pskb_may_pull(2)
>   => in recv_tt_query()
>   	  - pskb_may_pull(sizeof(header))
> 	  - skb_linearise()
> 
> Actually it seems we haven't any useless linearisation.
> Would you mind explain us where you actually found the problem, please?
> 
> It might also be that I misunderstood your advice.

You only need to call pskb_may_pull() on the parts of the packet you want to
access directly to parse headers etc.

If you use that interface properly, you never need to linearize, ever.

^ permalink raw reply

* Re: [PATCH] e1000e: Fix bug for e1000e interrupt default mode select.
From: Ben Hutchings @ 2011-12-02 17:55 UTC (permalink / raw)
  To: Kumar Gala
  Cc: jeffrey.t.kirsher, jesse.brandeburg, e1000-devel, netdev,
	Prabhakar
In-Reply-To: <1322809906-6428-1-git-send-email-galak@kernel.crashing.org>

On Fri, 2011-12-02 at 01:11 -0600, Kumar Gala wrote:
> From: Prabhakar <prabhakar@freescale.com>
> 
> If the kernel config does not have MSI enabled (CONFIG_PCI_MSI) the driver
> should not default to MSI interrupt mode but legacy interrupt mode.

It is supposed to automatically fall-back to legacy interrupt mode.
Does that not work?

Also, are there really systems with PCI Express and no MSI support?

Ben.

> Signed-off-by: Jin Qing <b24347@freescale.com>
> Signed-off-by: Prabhakar <prabhakar@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  drivers/net/ethernet/intel/e1000e/param.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
> index 20e93b0..18c35c6 100644
> --- a/drivers/net/ethernet/intel/e1000e/param.c
> +++ b/drivers/net/ethernet/intel/e1000e/param.c
> @@ -388,8 +388,13 @@ void __devinit e1000e_check_options(struct e1000_adapter *adapter)
>  		static struct e1000_option opt = {
>  			.type = range_option,
>  			.name = "Interrupt Mode",
> +#ifdef CONFIG_PCI_MSI
>  			.err  = "defaulting to 2 (MSI-X)",
>  			.def  = E1000E_INT_MODE_MSIX,
> +#else
> +			.err  = "defaulting to 0 (Legacy)",
> +			.def  = E1000E_INT_MODE_LEGACY,
> +#endif
>  			.arg  = { .r = { .min = MIN_INTMODE,
>  					 .max = MAX_INTMODE } }
>  		};

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] net/macb: add DT support
From: Nicolas Ferre @ 2011-12-02 17:53 UTC (permalink / raw)
  To: Jamie Iles
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20111202172854.GB4998@totoro>

On 12/02/2011 06:28 PM, Jamie Iles :
> Hi Nicolas,
>
> On Fri, Dec 02, 2011 at 06:14:10PM +0100, Nicolas Ferre wrote:
>> From: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>
>> Allow the device tree to provide the mac address and the phy mode.
>>
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>> [nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
>> Signed-off-by: Nicolas Ferre<nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> Cc: Jamie Iles<jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>
> Looks nice to me.  There's a patch below to add the GEM stuff to the
> binding too if you want to role that in.

Yes, I will push it in a "v3" (I was busy correcting a bug and did not 
see your answer before sending v2...).

> Acked-by: Jamie Iles<jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>
> 8<----
>
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> index 7f0b90a..e09e3fb 100644
> --- a/Documentation/devicetree/bindings/net/macb.txt
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -1,9 +1,11 @@
> -* Cadence MACB Ethernet controller
> +* Cadence MACB/GEM Ethernet controller
>
>   Required properties:
> -- compatible: Should be "cdns,[<chip>-]macb"
> +- compatible: Should be "cdns,[<chip>-]{macb,gem}"
>     Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
>     Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
> +  Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
> +  the Cadence GEM, or the generic form "cdns,gem".
>   - reg: Address and length of the register set for the device
>   - interrupts: Should contain macb interrupt
>   - phy-mode: String, operation mode of the PHY interface.
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 103c6e6..89060e6 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
>   	{ .compatible = "cdns,at32ap7000-macb" },
>   	{ .compatible = "cdns,at91sam9260-macb" },
>   	{ .compatible = "cdns,macb" },
> +	{ .compatible = "cdns,pc302-gem" },
> +	{ .compatible = "cdns,gem" },
>   	{ /* sentinel */ }
>   };

BTW, I think we may also modify the MII/RMII selection code for adding 
gigabit selection... but maybe you already have the patches?

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH net-next V0 19/21] mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
From: Roland Dreier @ 2011-12-02 17:51 UTC (permalink / raw)
  To: Yevgeny Petrilin
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, liranl-VPRAkNaXOzVS1MOuV/RT9w,
	jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
In-Reply-To: <4ED8A636.40901-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>

>  #ifdef CONFIG_PCI_IOV
> +
> +static int sr_iov;
> +module_param(sr_iov, int, 0444);
> +MODULE_PARM_DESC(sr_iov, "enable #sr_iov functions if sr_iov > 0");
> +
> +static int probe_vf;
> +module_param(probe_vf, int, 0444);
> +MODULE_PARM_DESC(probe_vf, "number of vfs to probe by pf driver (sr_iov > 0)");
> +
>  int mlx4_log_num_mgm_entry_size = 10;
>  module_param_named(log_num_mgm_entry_size,
>                        mlx4_log_num_mgm_entry_size, int, 0444);
> @@ -85,24 +95,30 @@ MODULE_PARM_DESC(log_num_mgm_entry_size, "log mgm size, that defines the num"
>                                         " log_num_mgm_entry_size <= 12");
>
>  #else /* CONFIG_PCI_IOV */
> +static int sr_iov;
> +#define probe_vf 0
>  int mlx4_log_num_mgm_entry_size = 9;
>  #endif /* CONFIG_PCI_IOV */

I don't think it's a good idea to put module parameters inside an
#ifdef.  Then depending on the
kernel config someone compiles with, their modprobe.conf may or may not break.

> +static int port_type_array[2] = {1, 1};
> +static int arr_argc = 2;
> +module_param_array(port_type_array, int, &arr_argc, 0444);
> +MODULE_PARM_DESC(port_type_array, "Array of port types: IB by default");

Is this really the best way to pass this in?  What if I have two
adapters, and I want to use
the first one for IB and the second for ethernet?  (Which doesn't
sound like a particularly
obscure config)

 - R.
--
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

* [PATCH] ARM: at91/net: add macb ethernet controller in 9g45 DT
From: Nicolas Ferre @ 2011-12-02 17:50 UTC (permalink / raw)
  To: robherring2-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, plagnioj-sclMFOaUSTBWk0Htik3J/w
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1322847782-22650-1-git-send-email-nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
enable it in at91sam9m10g45ek board device tree file.

Signed-off-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/at91sam9g45.dtsi     |    7 +++++++
 arch/arm/boot/dts/at91sam9m10g45ek.dts |    6 ++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index e89b1d7..67f94d3 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -101,6 +101,13 @@
 				atmel,use-dma-tx;
 				status = "disabled";
 			};
+
+			macb0: ethernet@fffbc000 {
+				compatible = "cdns,at32ap7000-macb", "cdns,macb";
+				reg = <0xfffbc000 0x100>;
+				interrupts = <25 4>;
+				status = "disabled";
+			};
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts b/arch/arm/boot/dts/at91sam9m10g45ek.dts
index 85b34f5..17377a2 100644
--- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
+++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
@@ -30,6 +30,12 @@
 			usart1: serial@fff90000 {
 				status = "okay";
 			};
+
+			macb0: ethernet@fffbc000 {
+				local-mac-address = [3a 0e 03 04 05 06];
+				phy-mode = "rmii";
+				status = "okay";
+			};
 		};
 	};
 };
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH v7 02/10] foundations of per-cgroup memory pressure controlling.
From: Glauber Costa @ 2011-12-02 17:46 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups
In-Reply-To: <20111130094305.9c69ecd8.kamezawa.hiroyu@jp.fujitsu.com>


>>   static void proto_seq_printf(struct seq_file *seq, struct proto *proto)
>>   {
>> +	struct mem_cgroup *memcg = mem_cgroup_from_task(current);
>> +
>>   	seq_printf(seq, "%-9s %4u %6d  %6ld   %-3s %6u   %-3s  %-10s "
>>   			"%2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c\n",
>>   		   proto->name,
>>   		   proto->obj_size,
>>   		   sock_prot_inuse_get(seq_file_net(seq), proto),
>> -		   proto->memory_allocated != NULL ? atomic_long_read(proto->memory_allocated) : -1L,
>> -		   proto->memory_pressure != NULL ? *proto->memory_pressure ? "yes" : "no" : "NI",
>> +		   sock_prot_memory_allocated(proto, memcg),
>> +		   sock_prot_memory_pressure(proto, memcg),
>
> I wonder I should say NO, here. (Networking guys are ok ??)
>
> IIUC, this means there is no way to see aggregated sockstat of all system.
> And the result depends on the cgroup which the caller is under control.
>
> I think you should show aggregated sockstat(global + per-memcg) here and
> show per-memcg ones via /cgroup interface or add private_sockstat to show
> per cgroup summary.
>

Hi Kame,

Yes, the statistics displayed depends on which cgroup you live.
Also, note that the parent cgroup here is always updated (even when 
use_hierarchy is set to 0). So it is always possible to grab global 
statistics, by being in the root cgroup.

For the others, I believe it to be a question of naturalization. Any 
tool that is fetching these values is likely interested in the amount of 
resources available/used. When you are on a cgroup, the amount of 
resources available/used changes, so that's what you should see.

Also brings the point of resource isolation: if you shouldn't interfere 
with other set of process' resources, there is no reason for you to see 
them in the first place.

So given all that, I believe that whenever we talk about resources in a 
cgroup, we should talk about cgroup-local ones.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH v2] net/macb: add DT support
From: Nicolas Ferre @ 2011-12-02 17:43 UTC (permalink / raw)
  To: robherring2-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, plagnioj-sclMFOaUSTBWk0Htik3J/w
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1322846050-4543-1-git-send-email-nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>

Allow the device tree to provide the mac address and the phy mode.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
[nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
Signed-off-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
---
v2: modify macb_get_hwaddr_dt() parameter

 Documentation/devicetree/bindings/net/macb.txt |   22 +++++++
 drivers/net/ethernet/cadence/macb.c            |   71 +++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h            |    2 +
 3 files changed, 87 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/macb.txt

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
new file mode 100644
index 0000000..7f0b90a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -0,0 +1,22 @@
+* Cadence MACB Ethernet controller
+
+Required properties:
+- compatible: Should be "cdns,[<chip>-]macb"
+  Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
+  Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain macb interrupt
+- phy-mode: String, operation mode of the PHY interface.
+  Supported values are: "mii", "rmii", "gmii", "rgmii".
+
+Optional properties:
+- local-mac-address: 6 bytes, mac address
+
+Examples:
+
+	macb0: ethernet@fffc4000 {
+		compatible = "cdns,at32ap7000-macb";
+		reg = <0xfffc4000 0x4000>;
+		interrupts = <21>;
+		phy-mode = "rmii";
+	};
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 64d6146..5e75855 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -23,6 +23,8 @@
 #include <linux/platform_data/macb.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
 
 #include "macb.h"
 
@@ -191,7 +193,6 @@ static int macb_mii_probe(struct net_device *dev)
 {
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev;
-	struct macb_platform_data *pdata;
 	int ret;
 
 	phydev = phy_find_first(bp->mii_bus);
@@ -200,14 +201,11 @@ static int macb_mii_probe(struct net_device *dev)
 		return -1;
 	}
 
-	pdata = bp->pdev->dev.platform_data;
 	/* TODO : add pin_irq */
 
 	/* attach the mac to the phy */
 	ret = phy_connect_direct(dev, phydev, &macb_handle_link_change, 0,
-				 pdata && pdata->is_rmii ?
-				 PHY_INTERFACE_MODE_RMII :
-				 PHY_INTERFACE_MODE_MII);
+				 bp->phy_interface);
 	if (ret) {
 		netdev_err(dev, "Could not attach to PHY\n");
 		return ret;
@@ -1244,6 +1242,50 @@ static const struct net_device_ops macb_netdev_ops = {
 #endif
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id macb_dt_ids[] = {
+	{ .compatible = "cdns,at32ap7000-macb" },
+	{ .compatible = "cdns,at91sam9260-macb" },
+	{ .compatible = "cdns,macb" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, macb_dt_ids);
+
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	if (np)
+		return of_get_phy_mode(np);
+
+	return -ENODEV;
+}
+
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	struct device_node *np = bp->pdev->dev.of_node;
+	if (np) {
+		const char *mac = of_get_mac_address(np);
+		if (mac) {
+			memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+#else
+static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+static int __devinit macb_get_hwaddr_dt(struct macb *bp)
+{
+	return -ENODEV;
+}
+#endif
+
 static int __init macb_probe(struct platform_device *pdev)
 {
 	struct macb_platform_data *pdata;
@@ -1318,10 +1360,22 @@ static int __init macb_probe(struct platform_device *pdev)
 	config |= macb_dbw(bp);
 	macb_writel(bp, NCFGR, config);
 
-	macb_get_hwaddr(bp);
-	pdata = pdev->dev.platform_data;
+	err = macb_get_hwaddr_dt(bp);
+	if (err < 0)
+		macb_get_hwaddr(bp);
+
+	err = macb_get_phy_mode_dt(pdev);
+	if (err < 0) {
+		pdata = pdev->dev.platform_data;
+		if (pdata && pdata->is_rmii)
+			bp->phy_interface = PHY_INTERFACE_MODE_RMII;
+		else
+			bp->phy_interface = PHY_INTERFACE_MODE_MII;
+	} else {
+		bp->phy_interface = err;
+	}
 
-	if (pdata && pdata->is_rmii)
+	if (bp->phy_interface == PHY_INTERFACE_MODE_RMII)
 #if defined(CONFIG_ARCH_AT91)
 		macb_or_gem_writel(bp, USRIO, (MACB_BIT(RMII) |
 					       MACB_BIT(CLKEN)));
@@ -1444,6 +1498,7 @@ static struct platform_driver macb_driver = {
 	.driver		= {
 		.name		= "macb",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(macb_dt_ids),
 	},
 };
 
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 1931078..335e288 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -532,6 +532,8 @@ struct macb {
 	unsigned int 		link;
 	unsigned int 		speed;
 	unsigned int 		duplex;
+
+	phy_interface_t		phy_interface;
 };
 
 static inline bool macb_is_gem(struct macb *bp)
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH net-next] forcedeath: Fix bql support for forcedeath
From: David Miller @ 2011-12-02 17:39 UTC (permalink / raw)
  To: eric.dumazet; +Cc: igorm, netdev
In-Reply-To: <1322822803.2607.2.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 02 Dec 2011 11:46:43 +0100

> Le vendredi 02 décembre 2011 à 10:48 +0100, igorm@etf.rs a écrit :
>> From: Igor Maravic <igorm@etf.rs>
>> 
>> Moved netdev_completed_queue() out of while loop in function nv_tx_done_optimized().
>> Because this function was in while loop,
>> BUG_ON(count > dql->num_queued - dql->num_completed)
>> was hit in dql_completed().
>> 
>> Signed-off-by: Igor Maravic <igorm@etf.rs>
>> ---
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: Small glitch in neptune ethernet driver
From: David Miller @ 2011-12-02 17:37 UTC (permalink / raw)
  To: thomas.jarosch; +Cc: netdev
In-Reply-To: <201112020949.32369.thomas.jarosch@intra2net.com>

From: Thomas Jarosch <thomas.jarosch@intra2net.com>
Date: Fri, 2 Dec 2011 09:49:32 +0100

> On Friday, 2. December 2011 04:00:03 David Miller wrote:
>> diff --git a/drivers/net/ethernet/sun/niu.c
>> b/drivers/net/ethernet/sun/niu.c index 680b107..56d106e 100644
>> --- a/drivers/net/ethernet/sun/niu.c
>> +++ b/drivers/net/ethernet/sun/niu.c
>> @@ -8579,9 +8579,11 @@ static int __devinit phy_record(struct niu_parent
>> *parent, if (dev_id_1 < 0 || dev_id_2 < 0)
>>  		return 0;
>>  	if (type == PHY_TYPE_PMA_PMD || type == PHY_TYPE_PCS) {
>> +		/* Becuase of the NIU_PHY_ID_MASK being applied, the 8704
> 
> The change looks good to me, just fix the typo in "Becuase".

Fixed, thanks.

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference at 000000000000002c
From: David Miller @ 2011-12-02 17:34 UTC (permalink / raw)
  To: s.priebe; +Cc: eric.dumazet, jwboyer, hch, netdev, david, stable
In-Reply-To: <4ED87BE9.20104@profihost.ag>

From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
Date: Fri, 02 Dec 2011 08:19:05 +0100

> Am 02.12.2011 07:17, schrieb David Miller:
>> From: Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
>> Date: Fri, 02 Dec 2011 07:08:57 +0100
>> 
>>> So can someone please send them to greg => stable@ so we can get them
>>> into 3.0.X longterm stable.
>> 
>> I'll do it, these things just take care and time.
> 
> Thanks! Could you please CC me?

Sorry, I'm not going to do that, the I'd have to do it for anyone who asks
me to and that's not reasonable.

^ permalink raw reply

* Re: [PATCH] net/macb: add DT support
From: Jamie Iles @ 2011-12-02 17:28 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1322846050-4543-1-git-send-email-nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Hi Nicolas,

On Fri, Dec 02, 2011 at 06:14:10PM +0100, Nicolas Ferre wrote:
> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> 
> Allow the device tree to provide the mac address and the phy mode.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> [nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
> Signed-off-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> Cc: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>

Looks nice to me.  There's a patch below to add the GEM stuff to the 
binding too if you want to role that in.

Acked-by: Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>

8<----

diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
index 7f0b90a..e09e3fb 100644
--- a/Documentation/devicetree/bindings/net/macb.txt
+++ b/Documentation/devicetree/bindings/net/macb.txt
@@ -1,9 +1,11 @@
-* Cadence MACB Ethernet controller
+* Cadence MACB/GEM Ethernet controller
 
 Required properties:
-- compatible: Should be "cdns,[<chip>-]macb"
+- compatible: Should be "cdns,[<chip>-]{macb,gem}"
   Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
   Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb"
+  Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
+  the Cadence GEM, or the generic form "cdns,gem".
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
 - phy-mode: String, operation mode of the PHY interface.
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 103c6e6..89060e6 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at32ap7000-macb" },
 	{ .compatible = "cdns,at91sam9260-macb" },
 	{ .compatible = "cdns,macb" },
+	{ .compatible = "cdns,pc302-gem" },
+	{ .compatible = "cdns,gem" },
 	{ /* sentinel */ }
 };

^ permalink raw reply related


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