Netdev List
 help / color / mirror / Atom feed
* [PATCH] nf_conntrack_reasm: add fast path for in-order fragments
From: Changli Gao @ 2010-07-01  3:58 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Eric Dumazet,
	netfilter-devel, netdev, Mitchell Erblich, Changli Gao

nf_conntrack_reasm: add fast path for in-order fragments

As the fragments are sent in order in most of OSes, such as Windows, Darwin and
FreeBSD, it is likely the new fragments are at the end of the inet_frag_queue.
In the fast path, we check if the skb at the end of the inet_frag_queue is the
prev we expect.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/ipv6/netfilter/nf_conntrack_reasm.c |    9 +++++++++
 1 file changed, 9 insertions(+)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 9254008..098a050 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -269,6 +269,11 @@ static int nf_ct_frag6_queue(struct nf_ct_frag6_queue *fq, struct sk_buff *skb,
 	 * in the chain of fragments so far.  We must know where to put
 	 * this fragment, right?
 	 */
+	prev = fq->q.fragments_tail;
+	if (!prev || NFCT_FRAG6_CB(prev)->offset < offset) {
+		next = NULL;
+		goto found;
+	}
 	prev = NULL;
 	for (next = fq->q.fragments; next != NULL; next = next->next) {
 		if (NFCT_FRAG6_CB(next)->offset >= offset)
@@ -276,6 +281,7 @@ static int nf_ct_frag6_queue(struct nf_ct_frag6_queue *fq, struct sk_buff *skb,
 		prev = next;
 	}
 
+found:
 	/* We found where to put this one.  Check for overlap with
 	 * preceding fragment, and, if needed, align things so that
 	 * any overlaps are eliminated.
@@ -341,6 +347,8 @@ static int nf_ct_frag6_queue(struct nf_ct_frag6_queue *fq, struct sk_buff *skb,
 
 	/* Insert this fragment in the chain of fragments. */
 	skb->next = next;
+	if (!next)
+		fq->q.fragments_tail = skb;
 	if (prev)
 		prev->next = skb;
 	else
@@ -464,6 +472,7 @@ nf_ct_frag6_reasm(struct nf_ct_frag6_queue *fq, struct net_device *dev)
 					  head->csum);
 
 	fq->q.fragments = NULL;
+	fq->q.fragments_tail = NULL;
 
 	/* all original skbs are linked into the NFCT_FRAG6_CB(head).orig */
 	fp = skb_shinfo(head)->frag_list;

^ permalink raw reply related

* Re: [net-next-2.6 PATCH] be2net: memory barrier fixes on IBM p7 platform
From: Sathya Perla @ 2010-07-01  6:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20100630.132749.218066997.davem@davemloft.net>

On Wed, 2010-06-30 at 13:27 -0700, David Miller wrote:
> > -
> > +	
> > +	rmb();
> 
> That first addition does nothing but add erroneous trailing
> whitespace.
> 
> You can physically see that something must be wrong here just by look
> at this patch chunk, please review things more thoroughly before
> submitting in the future.
> 
Will do, thanks.



___________________________________________________________________________________
This message, together with any attachment(s), contains confidential and proprietary information of
ServerEngines Corporation and is intended only for the designated recipient(s) named above. Any unauthorized
review, printing, retention, copying, disclosure or distribution is strictly prohibited.  If you are not the
intended recipient of this message, please immediately advise the sender by reply email message and
delete all copies of this message and any attachment(s). Thank you.


^ permalink raw reply

* Re: [PATCH] igbvf: avoid name clash between PF and VF
From: Stefan Assmann @ 2010-07-01  6:37 UTC (permalink / raw)
  To: Casey Leedom
  Cc: netdev, e1000-devel, Duyck, Alexander H, gregory.v.rose,
	jeffrey.t.kirsher, Andy Gospodarek
In-Reply-To: <201006300959.37145.leedom@chelsio.com>

On 30.06.2010 18:59, Casey Leedom wrote:
> | From: Stefan Assmann <sassmann@redhat.com>
> | Date: Wednesday, June 30, 2010 01:53 am
> | 
> | This is not a udev bug since udev doesn't create persistent rules for
> | VFs as their MAC address changes every reboot.
> | 
> | To avoid this problem we could change the kernel name for the VFs and
> | thus avoid confusion between VFs and PFs.
> | 
> | I've already discussed this with Alexander Duyck and Greg Rose, so far
> | they have no objection. However this problem appears for all drivers that
> | support PFs and VFs and thus the changes should be applied consistently
> | to all of these drivers.
> 
>   I'm not sure that this problem affects "all drivers which support PFs and VFs."  
> I think that you might mean "all drivers which support PFs and VFs with non-
> persistent MAC addresses for the VFs."  For instance, the MAC addresses 
> associated with the new cxgb4vf VFs are persistent so, from what I understand of 
> the scenario you outlined, I don't think that they would trigger the problem you 
> describe.  Please correct me if I've missed something.  Thanks.
> 
> Casey

You're correct, the problem shouldn't occur with cxgb4vf and therefore
this change shouldn't be necessary. However we might consider a
consistent naming scheme for VFs in all drivers. But I don't have a
strong opinion about this, either way would be fine by me.

  Stefan
--
Stefan Assmann         | Red Hat GmbH
Software Engineer      | Otto-Hahn-Strasse 20, 85609 Dornach
                       | HR: Amtsgericht Muenchen HRB 153243
                       | GF: Brendan Lane, Charlie Peters,
sassmann at redhat.com |     Michael Cunningham, Charles Cachera

^ permalink raw reply

* Re: RFC:  Allow 'ip' to run in daemon mode?
From: Simon Horman @ 2010-07-01  7:07 UTC (permalink / raw)
  To: Ben Greear; +Cc: NetDev, Stephen Hemminger
In-Reply-To: <4C2A1291.7000203@candelatech.com>

On Tue, Jun 29, 2010 at 08:34:41AM -0700, Ben Greear wrote:
> I'm considering modifying 'ip' to be able to run in daemon
> mode so that I can do lots of IP commands without having to
> pay the startup cost of iproute.
> 
> The -batch option almost works, but it's hard to programatically
> figure out failure codes.
> 
> I'm thinking about making these changes:
> 
> 1)  Move all of the error printing code into common methods (basically,
>    wrap printf).  In daemon mode this text can be sent back to the
>    calling process, and in normal mode, it will be printed to stdout/stderr
>    as it is currently.
> 
> 2)  Remove all or most calls to 'exit' and instead return error codes
>    to the calling logic.
> 
> 3)  Add ability to listen on a unix socket for commands, basically treat
>    them just like batch commands, one command per packet.
> 
> 4)  Return well formatted error code and text response to calling process
>    over the unix socket, maybe something like:
> 
> RV: [errno or equiv, zero for success]\n
> CMD: [ command string this relates to ]\n
> [ Optional free form text ]
> 
> 
> Does something like this have any chance of upstream inclusion?

Hi Ben,

can't you achieve as much by omitting 3) and using stdio (cleanly)?
Or in other words, fix batch mode rather than adding another mode.
Or are you worried about backwards-compatibility?


^ permalink raw reply

* [PATCH] ll_temac: add error checking to DMA init path
From: Denis Kirjanov @ 2010-07-01  9:39 UTC (permalink / raw)
  To: davem; +Cc: john.linn, brian.hill, netdev

Add error checking to DMA descriptor rings initialization code.

Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
drivers/net/ll_temac_main.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index 52dcc84..7b12d0e 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -202,14 +202,29 @@ static int temac_dma_bd_init(struct net_device *ndev)
 	int i;
 
 	lp->rx_skb = kzalloc(sizeof(*lp->rx_skb) * RX_BD_NUM, GFP_KERNEL);
+	if (!lp->rx_skb) {
+		dev_err(&ndev->dev,
+				"can't allocate memory for DMA RX buffer\n");
+		goto out;
+	}
 	/* allocate the tx and rx ring buffer descriptors. */
 	/* returns a virtual addres and a physical address. */
 	lp->tx_bd_v = dma_alloc_coherent(ndev->dev.parent,
 					 sizeof(*lp->tx_bd_v) * TX_BD_NUM,
 					 &lp->tx_bd_p, GFP_KERNEL);
+	if (!lp->tx_bd_v) {
+		dev_err(&ndev->dev,
+				"unable to allocate DMA TX buffer descriptors");
+		goto out;
+	}
 	lp->rx_bd_v = dma_alloc_coherent(ndev->dev.parent,
 					 sizeof(*lp->rx_bd_v) * RX_BD_NUM,
 					 &lp->rx_bd_p, GFP_KERNEL);
+	if (!lp->rx_bd_v) {
+		dev_err(&ndev->dev,
+				"unable to allocate DMA RX buffer descriptors");
+		goto out;
+	}
 
 	memset(lp->tx_bd_v, 0, sizeof(*lp->tx_bd_v) * TX_BD_NUM);
 	for (i = 0; i < TX_BD_NUM; i++) {
@@ -227,7 +242,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
 
 		if (skb == 0) {
 			dev_err(&ndev->dev, "alloc_skb error %d\n", i);
-			return -1;
+			goto out;
 		}
 		lp->rx_skb[i] = skb;
 		/* returns physical address of skb->data */
@@ -258,6 +273,9 @@ static int temac_dma_bd_init(struct net_device *ndev)
 	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
 
 	return 0;
+
+out:
+	return -ENOMEM;
 }
 
 /* ---------------------------------------------------------------------
@@ -505,7 +523,10 @@ static void temac_device_reset(struct net_device *ndev)
 	}
 	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
 
-	temac_dma_bd_init(ndev);
+	if (temac_dma_bd_init(ndev)) {
+		dev_err(&ndev->dev,
+				"temac_device_reset descriptor allocation failed\n");
+	}
 
 	temac_indirect_out32(lp, XTE_RXC0_OFFSET, 0);
 	temac_indirect_out32(lp, XTE_RXC1_OFFSET, 0);

^ permalink raw reply related

* [RESEND][PATCH] cxgb4: Use kfree_skb for skb pointers
From: Denis Kirjanov @ 2010-07-01  9:45 UTC (permalink / raw)
  To: davem; +Cc: netdev

Use kfree_skb for skb pointers

Acked-by: Dimitris Michailidis <dm@chelsio.com> 
Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---
drivers/net/cxgb4/l2t.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/cxgb4/l2t.c b/drivers/net/cxgb4/l2t.c
index 5b990d2..e8f0f55 100644
--- a/drivers/net/cxgb4/l2t.c
+++ b/drivers/net/cxgb4/l2t.c
@@ -314,7 +314,7 @@ static void t4_l2e_free(struct l2t_entry *e)
 			struct sk_buff *skb = e->arpq_head;
 
 			e->arpq_head = skb->next;
-			kfree(skb);
+			kfree_skb(skb);
 		}
 		e->arpq_tail = NULL;
 	}

^ permalink raw reply related

* [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 11:07 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra
  Cc: Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev, lkml,
	kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
	Thomas Gleixner, Andi Kleen
In-Reply-To: <20100625101022.GA16321@redhat.com>

Author: Sridhar Samudrala <sri@us.ibm.com>

sched: export sched_set/getaffinity to modules

vhost-net driver wants to copy the affinity from the
owner thread to thread it creates. Export
sched_set/get affinity to modules to make this possible
when vhost is built as a module.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

I'm not sure the previous time made it clear what exactly is the
proposed change, so reposting.  Info, Peter, could you ack merging the
following through the net-next tree please?

diff --git a/kernel/sched.c b/kernel/sched.c
index d484081..3759391 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4744,6 +4744,7 @@ out_put_task:
 	put_online_cpus();
 	return retval;
 }
+EXPORT_SYMBOL_GPL(sched_setaffinity);
 
 static int get_user_cpu_mask(unsigned long __user *user_mask_ptr, unsigned len,
 			     struct cpumask *new_mask)
@@ -4807,6 +4808,7 @@ out_unlock:
 
 	return retval;
 }
+EXPORT_SYMBOL_GPL(sched_getaffinity);
 
 /**
  * sys_sched_getaffinity - get the cpu affinity of a process

^ permalink raw reply related

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 11:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701110708.GA27368@redhat.com>

On Thu, 2010-07-01 at 14:07 +0300, Michael S. Tsirkin wrote:
> Author: Sridhar Samudrala <sri@us.ibm.com>
> 
> sched: export sched_set/getaffinity to modules
> 
> vhost-net driver wants to copy the affinity from the
> owner thread to thread it creates. Export
> sched_set/get affinity to modules to make this possible
> when vhost is built as a module.
> 
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> I'm not sure the previous time made it clear what exactly is the
> proposed change, so reposting.  Info, Peter, could you ack merging the
> following through the net-next tree please?
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index d484081..3759391 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -4744,6 +4744,7 @@ out_put_task:
>  	put_online_cpus();
>  	return retval;
>  }
> +EXPORT_SYMBOL_GPL(sched_setaffinity);
>  
>  static int get_user_cpu_mask(unsigned long __user *user_mask_ptr, unsigned len,
>  			     struct cpumask *new_mask)
> @@ -4807,6 +4808,7 @@ out_unlock:
>  
>  	return retval;
>  }
> +EXPORT_SYMBOL_GPL(sched_getaffinity);
>  
>  /**
>   * sys_sched_getaffinity - get the cpu affinity of a process

Urgh,.. so why again is that a good idea?

^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv4: sysctl to block responding on down interface
From: Andi Kleen @ 2010-07-01 11:23 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Stephen Hemminger, David Miller, netdev
In-Reply-To: <OF62B30BCC.F8523B86-ONC1257750.00687496-C1257750.0068AA27@transmode.se>

Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:

> Stephen Hemminger <shemminger@vyatta.com> wrote on 2010/06/11 17:48:54:
>>
>> When Linux is used as a router, it is undesirable for the kernel to process
>> incoming packets when the address assigned to the interface is down.
>> The initial problem report was for a management application that used ICMP
>> to check link availability.
>>
>> The default is disabled to maintain compatibility with previous behavior.
>> This is not recommended for server systems because it makes fail over more
>> difficult, and does not account for configurations where multiple interfaces
>> have the same IP address.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> Ping David et. all?
> I too want this.

Doesn't arpfilter enable this already? If you set in on the still up
interfaces those will not answer to other IP addresses.

This only works on the ARP level, so it has to wait until the arp
cache in the remote host times out.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* Re: [net-next-2.6 PATCH v2] x86: Align skb w/ start of cacheline on newer core 2/Xeon Arch
From: Andi Kleen @ 2010-07-01 11:28 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, netdev, gospo, bphilips, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Alexander Duyck
In-Reply-To: <20100630043728.9224.64191.stgit@localhost.localdomain>

Jeff Kirsher <jeffrey.t.kirsher@intel.com> writes:


Sorry for the late comment.

>  
> +#ifdef CONFIG_MCORE2
> +/*
> + * We handle most unaligned accesses in hardware.  On the other hand
> + * unaligned DMA can be quite expensive on some Nehalem processors.
> + *
> + * Based on this we disable the IP header alignment in network drivers.
> + */
> +#define NET_IP_ALIGN	0
> +#endif
>  #endif /* _ASM_X86_SYSTEM_H */

The ifdef should be imho dropped and the option be made unconditional
for all x86. I am not aware of any x86 core where unalignment is really
slow. This would increase the chance of it actually working on many
configurations which do not necessarily optimize for Core2.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 11:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277983179.1917.10.camel@laptop>

On Thu, 2010-07-01 at 13:19 +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 14:07 +0300, Michael S. Tsirkin wrote:
> > Author: Sridhar Samudrala <sri@us.ibm.com>
> > 
> > sched: export sched_set/getaffinity to modules
> > 
> > vhost-net driver wants to copy the affinity from the
> > owner thread to thread it creates. Export
> > sched_set/get affinity to modules to make this possible
> > when vhost is built as a module.

> Urgh,.. so why again is that a good idea?

In particular:
 - who sets the affinity of the task? 
 - why can't it set the kernel thread's affinity too?
 - what happens if someone changes the tasks' affinity?

So no, I don't think this is a sensible thing to do at all.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv4: sysctl to block responding on down interface
From: Joakim Tjernlund @ 2010-07-01 11:48 UTC (permalink / raw)
  To: Andi Kleen; +Cc: David Miller, netdev, Stephen Hemminger
In-Reply-To: <871vbn1m52.fsf@basil.nowhere.org>

Andi Kleen <andi@firstfloor.org> wrote on 2010/07/01 13:23:21:
>
> Joakim Tjernlund <joakim.tjernlund@transmode.se> writes:
>
> > Stephen Hemminger <shemminger@vyatta.com> wrote on 2010/06/11 17:48:54:
> >>
> >> When Linux is used as a router, it is undesirable for the kernel to process
> >> incoming packets when the address assigned to the interface is down.
> >> The initial problem report was for a management application that used ICMP
> >> to check link availability.
> >>
> >> The default is disabled to maintain compatibility with previous behavior.
> >> This is not recommended for server systems because it makes fail over more
> >> difficult, and does not account for configurations where multiple interfaces
> >> have the same IP address.
> >>
> >> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > Ping David et. all?
> > I too want this.
>
> Doesn't arpfilter enable this already? If you set in on the still up
> interfaces those will not answer to other IP addresses.
>
> This only works on the ARP level, so it has to wait until the arp
> cache in the remote host times out.

I tried that but it didn't work, but I didn't think of clearing
the ARP cache.
Anyhow, such methods seems worse than just doing ifconfig 0.0.0.0

  Jocke


^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 11:55 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277984603.1917.15.camel@laptop>

On Thu, Jul 01, 2010 at 01:43:23PM +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 13:19 +0200, Peter Zijlstra wrote:
> > On Thu, 2010-07-01 at 14:07 +0300, Michael S. Tsirkin wrote:
> > > Author: Sridhar Samudrala <sri@us.ibm.com>
> > > 
> > > sched: export sched_set/getaffinity to modules
> > > 
> > > vhost-net driver wants to copy the affinity from the
> > > owner thread to thread it creates. Export
> > > sched_set/get affinity to modules to make this possible
> > > when vhost is built as a module.
> 
> > Urgh,.. so why again is that a good idea?
> 
> In particular:
>  - who sets the affinity of the task? 

management tools do this when they start qemu.

>  - why can't it set the kernel thread's affinity too?

It can. However: the threads are started internally by the driver
when qemu does an ioctl.  What we want to do is give it a sensible
default affinity. management tool can later tweak it if it wants to.

>  - what happens if someone changes the tasks' affinity?

We would normally create a cgroup including all internal
tasks, making it easy to find and change affinity for
them all if necessary.

> So no, I don't think this is a sensible thing to do at all.

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 12:23 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701115507.GA31333@redhat.com>

On Thu, Jul 01, 2010 at 02:55:07PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 01, 2010 at 01:43:23PM +0200, Peter Zijlstra wrote:
> > On Thu, 2010-07-01 at 13:19 +0200, Peter Zijlstra wrote:
> > > On Thu, 2010-07-01 at 14:07 +0300, Michael S. Tsirkin wrote:
> > > > Author: Sridhar Samudrala <sri@us.ibm.com>
> > > > 
> > > > sched: export sched_set/getaffinity to modules
> > > > 
> > > > vhost-net driver wants to copy the affinity from the
> > > > owner thread to thread it creates. Export
> > > > sched_set/get affinity to modules to make this possible
> > > > when vhost is built as a module.
> > 
> > > Urgh,.. so why again is that a good idea?
> > 
> > In particular:
> >  - who sets the affinity of the task? 
> 
> management tools do this when they start qemu.
> 
> >  - why can't it set the kernel thread's affinity too?
> 
> It can. However: the threads are started internally by the driver
> when qemu does an ioctl.  What we want to do is give it a sensible
> default affinity. management tool can later tweak it if it wants to.
> 
> >  - what happens if someone changes the tasks' affinity?
> 
> We would normally create a cgroup including all internal
> tasks, making it easy to find and change affinity for
> them all if necessary.
> 
> > So no, I don't think this is a sensible thing to do at all.

The patch using this is here:
http://www.mail-archive.com/kvm@vger.kernel.org/msg35411.html

It simply copies the affinity from the parent when thread is created.

-- 
MST

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 12:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701115507.GA31333@redhat.com>

On Thu, 2010-07-01 at 14:55 +0300, Michael S. Tsirkin wrote:

> >  - why can't it set the kernel thread's affinity too?
> 
> It can. However: the threads are started internally by the driver
> when qemu does an ioctl.  What we want to do is give it a sensible
> default affinity. management tool can later tweak it if it wants to.

So have that ioctl return the tid of that new fancy thread and then set
its affinity, stuff it in cgroup, whatever you fancy.

> >  - what happens if someone changes the tasks' affinity?
> 
> We would normally create a cgroup including all internal
> tasks, making it easy to find and change affinity for
> them all if necessary. 

And to stuff them in a cgroup you also need the tid, at which point it
might as well set the affinity from userspace, right?

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 12:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701122340.GB31333@redhat.com>

On Thu, 2010-07-01 at 15:23 +0300, Michael S. Tsirkin wrote:
> 
> The patch using this is here:
> http://www.mail-archive.com/kvm@vger.kernel.org/msg35411.html
> 
> It simply copies the affinity from the parent when thread is created.

Sounds like policy, not something the kernel should do..

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 12:46 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277987657.1917.32.camel@laptop>

On Thu, 2010-07-01 at 14:34 +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 15:23 +0300, Michael S. Tsirkin wrote:
> > 
> > The patch using this is here:
> > http://www.mail-archive.com/kvm@vger.kernel.org/msg35411.html
> > 
> > It simply copies the affinity from the parent when thread is created.
> 
> Sounds like policy, not something the kernel should do..

The alternative would be using clone() instead of thread_create() and
inherit everything from the creating task. Inheriting from kthreadd and
then undoing some aspects just sounds like daft policy that really ought
to be in userspace.

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 12:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277987563.1917.28.camel@laptop>

On Thu, Jul 01, 2010 at 02:32:43PM +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 14:55 +0300, Michael S. Tsirkin wrote:
> 
> > >  - why can't it set the kernel thread's affinity too?
> > 
> > It can. However: the threads are started internally by the driver
> > when qemu does an ioctl.  What we want to do is give it a sensible
> > default affinity. management tool can later tweak it if it wants to.
> 
> So have that ioctl return the tid of that new fancy thread and then set
> its affinity, stuff it in cgroup, whatever you fancy.
> 
> > >  - what happens if someone changes the tasks' affinity?
> > 
> > We would normally create a cgroup including all internal
> > tasks, making it easy to find and change affinity for
> > them all if necessary. 
> 
> And to stuff them in a cgroup you also need the tid, at which point it
> might as well set the affinity from userspace, right?

We also put it in a cgroup transparently. I think that it's actually
important to do it on thread creation: if we don't, malicious userspace
can create large amount of work exceeding the cgroup limits.

And the same applies so the affinity, right? If the qemu process
is limited to a set of CPUs, isn't it important to make
the kernel thread that does work our behalf limited to the same
set of CPUs?

-- 
MST

^ permalink raw reply

* [PATCH 1/2] qlge: Replacing add_timer() to mod_timer()
From: leitao @ 2010-07-01 13:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, Breno Leitao, Ron Mercer

Currently qlge driver calls add_timer() instead of mod_timer().
This patch changes add_timer() to mod_timer(), which seems a better
solution.

Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index fa4b24c..509dadc 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4611,8 +4611,7 @@ static void ql_timer(unsigned long data)
 		return;
 	}
 
-	qdev->timer.expires = jiffies + (5*HZ);
-	add_timer(&qdev->timer);
+	mod_timer(&qdev->timer, jiffies + (5*HZ));
 }
 
 static int __devinit qlge_probe(struct pci_dev *pdev,
@@ -4808,8 +4807,7 @@ static void qlge_io_resume(struct pci_dev *pdev)
 		netif_err(qdev, ifup, qdev->ndev,
 			  "Device was not running prior to EEH.\n");
 	}
-	qdev->timer.expires = jiffies + (5*HZ);
-	add_timer(&qdev->timer);
+	mod_timer(&qdev->timer, jiffies + (5*HZ));
 	netif_device_attach(ndev);
 }
 
@@ -4871,8 +4869,7 @@ static int qlge_resume(struct pci_dev *pdev)
 			return err;
 	}
 
-	qdev->timer.expires = jiffies + (5*HZ);
-	add_timer(&qdev->timer);
+	mod_timer(&qdev->timer, jiffies + (5*HZ));
 	netif_device_attach(ndev);
 
 	return 0;
-- 
1.6.5.2


^ permalink raw reply related

* [PATCH 2/2] qlge: fix a eeh handler to not add a pending timer
From: leitao @ 2010-07-01 13:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, Breno Leitao, Ron Mercer
In-Reply-To: <e687077281d05d3a2da49431b7c0ff0b1076f3e6.1277936929.git.root@sanx1002.austin.ibm.com>

On some ocasions the function qlge_io_resume() tries to add a
pending timer, which causes the system to hit the BUG() on
add_timer() function.

This patch removes the timer during the EEH recovery.

Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 509dadc..d10bcef 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4712,6 +4712,8 @@ static void ql_eeh_close(struct net_device *ndev)
 		netif_stop_queue(ndev);
 	}
 
+	/* Disabling the timer */
+	del_timer_sync(&qdev->timer);
 	if (test_bit(QL_ADAPTER_UP, &qdev->flags))
 		cancel_delayed_work_sync(&qdev->asic_reset_work);
 	cancel_delayed_work_sync(&qdev->mpi_reset_work);
-- 
1.6.5.2


^ permalink raw reply related

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 13:07 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701125038.GA32223@redhat.com>

On Thu, 2010-07-01 at 15:50 +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 01, 2010 at 02:32:43PM +0200, Peter Zijlstra wrote:
> > On Thu, 2010-07-01 at 14:55 +0300, Michael S. Tsirkin wrote:
> > 
> > > >  - why can't it set the kernel thread's affinity too?
> > > 
> > > It can. However: the threads are started internally by the driver
> > > when qemu does an ioctl.  What we want to do is give it a sensible
> > > default affinity. management tool can later tweak it if it wants to.
> > 
> > So have that ioctl return the tid of that new fancy thread and then set
> > its affinity, stuff it in cgroup, whatever you fancy.
> > 
> > > >  - what happens if someone changes the tasks' affinity?
> > > 
> > > We would normally create a cgroup including all internal
> > > tasks, making it easy to find and change affinity for
> > > them all if necessary. 
> > 
> > And to stuff them in a cgroup you also need the tid, at which point it
> > might as well set the affinity from userspace, right?
> 
> We also put it in a cgroup transparently. I think that it's actually
> important to do it on thread creation: if we don't, malicious userspace
> can create large amount of work exceeding the cgroup limits.
> 
> And the same applies so the affinity, right? If the qemu process
> is limited to a set of CPUs, isn't it important to make
> the kernel thread that does work our behalf limited to the same
> set of CPUs?

I'm not sure we have anything like this, but I wouldn't think so, if a
driver creates a kthread and manages to inject tons of work its not
typically limited to whatever limits apply to the task that supplied the
work.

Take the encryption threads for example, those don't run in the context
of whoever provides the data to be encrypted (file,net whatever) and
thus the task responsible could consume heaps more resources than when
it would have to do the encryption itself.

That's how kthreads work.

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 13:08 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277988395.1917.47.camel@laptop>

On Thu, Jul 01, 2010 at 02:46:35PM +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 14:34 +0200, Peter Zijlstra wrote:
> > On Thu, 2010-07-01 at 15:23 +0300, Michael S. Tsirkin wrote:
> > > 
> > > The patch using this is here:
> > > http://www.mail-archive.com/kvm@vger.kernel.org/msg35411.html
> > > 
> > > It simply copies the affinity from the parent when thread is created.
> > 
> > Sounds like policy, not something the kernel should do..
> 
> The alternative would be using clone() instead of thread_create() and
> inherit everything from the creating task.
> Inheriting from kthreadd and then undoing some aspects just sounds
> like daft policy that really ought to be in userspace.

Yes, that's basically what this patchset is trying to do:
create a workqueue inheriting everything from the creating task.
Sridhar started with an API to do exactly this:
http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-05/msg07478.html

Then we switched to raw kthread to avoid stepping on cwq toes.
Maybe it makes sense to add kthread_clone (in addition to
kthread_create) that would do what you suggest?
If yes, any hints on an implementation?


-- 
MST

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Michael S. Tsirkin @ 2010-07-01 13:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <1277989646.1917.77.camel@laptop>

On Thu, Jul 01, 2010 at 03:07:26PM +0200, Peter Zijlstra wrote:
> On Thu, 2010-07-01 at 15:50 +0300, Michael S. Tsirkin wrote:
> > On Thu, Jul 01, 2010 at 02:32:43PM +0200, Peter Zijlstra wrote:
> > > On Thu, 2010-07-01 at 14:55 +0300, Michael S. Tsirkin wrote:
> > > 
> > > > >  - why can't it set the kernel thread's affinity too?
> > > > 
> > > > It can. However: the threads are started internally by the driver
> > > > when qemu does an ioctl.  What we want to do is give it a sensible
> > > > default affinity. management tool can later tweak it if it wants to.
> > > 
> > > So have that ioctl return the tid of that new fancy thread and then set
> > > its affinity, stuff it in cgroup, whatever you fancy.
> > > 
> > > > >  - what happens if someone changes the tasks' affinity?
> > > > 
> > > > We would normally create a cgroup including all internal
> > > > tasks, making it easy to find and change affinity for
> > > > them all if necessary. 
> > > 
> > > And to stuff them in a cgroup you also need the tid, at which point it
> > > might as well set the affinity from userspace, right?
> > 
> > We also put it in a cgroup transparently. I think that it's actually
> > important to do it on thread creation: if we don't, malicious userspace
> > can create large amount of work exceeding the cgroup limits.
> > 
> > And the same applies so the affinity, right? If the qemu process
> > is limited to a set of CPUs, isn't it important to make
> > the kernel thread that does work our behalf limited to the same
> > set of CPUs?
> 
> I'm not sure we have anything like this, but I wouldn't think so, if a
> driver creates a kthread and manages to inject tons of work its not
> typically limited to whatever limits apply to the task that supplied the
> work.
> 
> Take the encryption threads for example, those don't run in the context
> of whoever provides the data to be encrypted (file,net whatever) and
> thus the task responsible could consume heaps more resources than when
> it would have to do the encryption itself.
> 
> That's how kthreads work.

Right. And IMHO ideally all such work would run on the appropriate
CPU and be accounted to. It's just that with virt people seem to
run untrusted applications and expect the damage to be contained.
So we came up with a simple approach that seems to do the
just just for us.

-- 
MST

^ permalink raw reply

* Re: [PATCH repost] sched: export sched_set/getaffinity to modules
From: Peter Zijlstra @ 2010-07-01 13:30 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Ingo Molnar, Sridhar Samudrala, Tejun Heo, Oleg Nesterov, netdev,
	lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
	Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100701130816.GB32223@redhat.com>

On Thu, 2010-07-01 at 16:08 +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 01, 2010 at 02:46:35PM +0200, Peter Zijlstra wrote:
> > On Thu, 2010-07-01 at 14:34 +0200, Peter Zijlstra wrote:
> > > On Thu, 2010-07-01 at 15:23 +0300, Michael S. Tsirkin wrote:
> > > > 
> > > > The patch using this is here:
> > > > http://www.mail-archive.com/kvm@vger.kernel.org/msg35411.html
> > > > 
> > > > It simply copies the affinity from the parent when thread is created.
> > > 
> > > Sounds like policy, not something the kernel should do..
> > 
> > The alternative would be using clone() instead of thread_create() and
> > inherit everything from the creating task.
> > Inheriting from kthreadd and then undoing some aspects just sounds
> > like daft policy that really ought to be in userspace.
> 
> Yes, that's basically what this patchset is trying to do:
> create a workqueue inheriting everything from the creating task.
> Sridhar started with an API to do exactly this:
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-05/msg07478.html
> 
> Then we switched to raw kthread to avoid stepping on cwq toes.
> Maybe it makes sense to add kthread_clone (in addition to
> kthread_create) that would do what you suggest?
> If yes, any hints on an implementation?

I think that's called kernel_thread() see
kernel/kthread.c:create_kthread().

Doing the whole kthreadd dance and then copying bits and pieces back
sounds very fragile, so yeah, something like that should work.

The other issue to consider is the thread group status of these things,
I think it would be best if these threads were still considered part of
the process that spawned them so that they would die nicely when the
process gets whacked.

At which point one could wonder if the kthread interface makes any
sense, why not let userspace fork tasks and let them call into the
kernel to perform work...

^ permalink raw reply

* ::::::APPLY FOR A LOAN:::::
From: Private Fund Loan Inc @ 2010-07-01 13:42 UTC (permalink / raw)
  To: inq

We offer loan at cheap rate, they say opprtunity does not knock twice.
But with our loan firm, you can get the cheapest loan at a low
percentage rate of 2% yearly from $5,000.00 Min. To $100 000 000.00 Max.

Contact us via email if you are interested with the following details
NAME: 
PHONE: 
LOAN DURATION:
ADDRESS: 
AMOUNT:

Private Fund Loan Inc
+60102950738


^ 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