* [PATCH] rps: make distributing packets fairly among all the online CPUs default
@ 2010-03-23 6:24 Changli Gao
2010-03-23 7:20 ` Eric Dumazet
2010-03-23 7:27 ` Eric Dumazet
0 siblings, 2 replies; 12+ messages in thread
From: Changli Gao @ 2010-03-23 6:24 UTC (permalink / raw)
To: David S. Miller; +Cc: Tom Herbert, xiaosuo, netdev
make distributing packets fairly among all the online CPUs default.
Make distributing packets fairly among all the online CPUs default, then
users don't need any explicit configuration to get the benefit of RPS.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index c0e2608..a4246f1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5234,6 +5234,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
}
EXPORT_SYMBOL(netif_stacked_transfer_operstate);
+static struct rps_map* alloc_rps_map(void)
+{
+ struct rps_map *map;
+ int i, cpu;
+
+ map = kzalloc(max_t(unsigned,
+ RPS_MAP_SIZE(cpumask_weight(cpu_online_mask)),
+ L1_CACHE_BYTES), GFP_KERNEL);
+ if (map == NULL)
+ return NULL;
+ i = 0;
+ for_each_online_cpu(cpu)
+ map->cpus[i++] = cpu;
+ map->len = i;
+
+ return map;
+}
+
/**
* register_netdevice - register a network device
* @dev: device to register
@@ -5282,7 +5300,13 @@ int register_netdevice(struct net_device *dev)
ret = -ENOMEM;
goto out;
}
-
+ dev->_rx->rps_map = alloc_rps_map();
+ if (dev->_rx->rps_map == NULL) {
+ kfree(dev->_rx);
+ dev->_rx = NULL;
+ ret = -ENOMEM;
+ goto out;
+ }
dev->_rx->first = dev->_rx;
atomic_set(&dev->_rx->count, 1);
dev->num_rx_queues = 1;
@@ -5688,8 +5712,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
* Set a pointer to first element in the array which holds the
* reference count.
*/
- for (i = 0; i < queue_count; i++)
+ for (i = 0; i < queue_count; i++) {
rx[i].first = rx;
+ rx[i].rps_map = alloc_rps_map();
+ if (rx[i].rps_map == NULL)
+ goto free_rx;
+ }
dev = PTR_ALIGN(p, NETDEV_ALIGN);
dev->padded = (char *)dev - (char *)p;
@@ -5723,6 +5751,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
return dev;
free_rx:
+ for (i = 0; i < queue_count; i++)
+ kfree(rx[i].rps_map);
kfree(rx);
free_tx:
kfree(tx);
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 6:24 [PATCH] rps: make distributing packets fairly among all the online CPUs default Changli Gao
@ 2010-03-23 7:20 ` Eric Dumazet
2010-03-23 9:56 ` Changli Gao
2010-03-23 7:27 ` Eric Dumazet
1 sibling, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2010-03-23 7:20 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, Tom Herbert, netdev
Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> make distributing packets fairly among all the online CPUs default.
>
> Make distributing packets fairly among all the online CPUs default, then
> users don't need any explicit configuration to get the benefit of RPS.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c0e2608..a4246f1 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5234,6 +5234,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
> }
> EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>
> +static struct rps_map* alloc_rps_map(void)
> +{
> + struct rps_map *map;
> + int i, cpu;
> +
> + map = kzalloc(max_t(unsigned,
> + RPS_MAP_SIZE(cpumask_weight(cpu_online_mask)),
> + L1_CACHE_BYTES), GFP_KERNEL);
> + if (map == NULL)
> + return NULL;
> + i = 0;
> + for_each_online_cpu(cpu)
> + map->cpus[i++] = cpu;
> + map->len = i;
> +
> + return map;
> +}
> +
> /**
> * register_netdevice - register a network device
> * @dev: device to register
> @@ -5282,7 +5300,13 @@ int register_netdevice(struct net_device *dev)
> ret = -ENOMEM;
> goto out;
> }
> -
> + dev->_rx->rps_map = alloc_rps_map();
> + if (dev->_rx->rps_map == NULL) {
> + kfree(dev->_rx);
> + dev->_rx = NULL;
> + ret = -ENOMEM;
> + goto out;
> + }
> dev->_rx->first = dev->_rx;
> atomic_set(&dev->_rx->count, 1);
> dev->num_rx_queues = 1;
> @@ -5688,8 +5712,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
> * Set a pointer to first element in the array which holds the
> * reference count.
> */
> - for (i = 0; i < queue_count; i++)
> + for (i = 0; i < queue_count; i++) {
> rx[i].first = rx;
> + rx[i].rps_map = alloc_rps_map();
> + if (rx[i].rps_map == NULL)
> + goto free_rx;
> + }
>
> dev = PTR_ALIGN(p, NETDEV_ALIGN);
> dev->padded = (char *)dev - (char *)p;
> @@ -5723,6 +5751,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
> return dev;
>
> free_rx:
> + for (i = 0; i < queue_count; i++)
> + kfree(rx[i].rps_map);
> kfree(rx);
> free_tx:
> kfree(tx);
>
>
> --
You cannot do this like this, these allocations wont be freed.
Better would be to take a look at net/core/net-sysfs.c
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 7:20 ` Eric Dumazet
@ 2010-03-23 9:56 ` Changli Gao
2010-03-23 11:23 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Changli Gao @ 2010-03-23 9:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev
On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>
> You cannot do this like this, these allocations wont be freed.
>
> Better would be to take a look at net/core/net-sysfs.c
>
Sorry, but I don't know why these allocations won't be freed.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 9:56 ` Changli Gao
@ 2010-03-23 11:23 ` Eric Dumazet
2010-03-23 11:27 ` Changli Gao
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2010-03-23 11:23 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, netdev
Le mardi 23 mars 2010 à 17:56 +0800, Changli Gao a écrit :
> On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> >
> > You cannot do this like this, these allocations wont be freed.
> >
> > Better would be to take a look at net/core/net-sysfs.c
> >
>
> Sorry, but I don't know why these allocations won't be freed.
>
>
Hint : This will be freed only if CONFIG_SYSFS is set
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 11:23 ` Eric Dumazet
@ 2010-03-23 11:27 ` Changli Gao
0 siblings, 0 replies; 12+ messages in thread
From: Changli Gao @ 2010-03-23 11:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev
On Tue, Mar 23, 2010 at 7:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 17:56 +0800, Changli Gao a écrit :
>> On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>> >
>> > You cannot do this like this, these allocations wont be freed.
>> >
>> > Better would be to take a look at net/core/net-sysfs.c
>> >
>>
>> Sorry, but I don't know why these allocations won't be freed.
>>
>>
>
> Hint : This will be freed only if CONFIG_SYSFS is set
>
got it, thanks. But I just wonder if it is right the life cycle of
rps_map is maintained by sysfs rather than netdev itself.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 6:24 [PATCH] rps: make distributing packets fairly among all the online CPUs default Changli Gao
2010-03-23 7:20 ` Eric Dumazet
@ 2010-03-23 7:27 ` Eric Dumazet
2010-03-23 10:03 ` Changli Gao
1 sibling, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2010-03-23 7:27 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, Tom Herbert, netdev
Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> make distributing packets fairly among all the online CPUs default.
>
> Make distributing packets fairly among all the online CPUs default, then
> users don't need any explicit configuration to get the benefit of RPS.
>
Some people want to dedicate a cpu to network interrupts, cache hits
only, to get low latencies.
RPS is not a win for all workloads.
I think we should not change default without a knob.
Didnt we discussed all this during last months ?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 7:27 ` Eric Dumazet
@ 2010-03-23 10:03 ` Changli Gao
2010-03-23 11:38 ` Eric Dumazet
2010-03-23 14:05 ` Andi Kleen
0 siblings, 2 replies; 12+ messages in thread
From: Changli Gao @ 2010-03-23 10:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
On Tue, Mar 23, 2010 at 3:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>> make distributing packets fairly among all the online CPUs default.
>>
>> Make distributing packets fairly among all the online CPUs default, then
>> users don't need any explicit configuration to get the benefit of RPS.
>>
>
> Some people want to dedicate a cpu to network interrupts, cache hits
> only, to get low latencies.
> RPS is not a win for all workloads.
>
> I think we should not change default without a knob.
>
> Didnt we discussed all this during last months ?
>
Ben Hutchings raised this question, but nobody replied with any message.
I do think distributing packets fairly among all the online CPUs will
helps most of users. I remember the smp_affinity of IRQ is the
online_cpu_mask be default.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 10:03 ` Changli Gao
@ 2010-03-23 11:38 ` Eric Dumazet
2010-03-23 11:58 ` Changli Gao
2010-03-23 14:05 ` Andi Kleen
1 sibling, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2010-03-23 11:38 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
Le mardi 23 mars 2010 à 18:03 +0800, Changli Gao a écrit :
>
> Ben Hutchings raised this question, but nobody replied with any message.
>
> I do think distributing packets fairly among all the online CPUs will
> helps most of users. I remember the smp_affinity of IRQ is the
> online_cpu_mask be default.
>
I know _many_ applications that perform better when all network IRQS are
directed to one CPU only. Single threaded UDP server for example, with
fast answer, or routers, or ...
Many sysadmins tuned their machine to meet this requirement. Installing
2.6.35, if RPS enabled by default will make them unhappy, especially if
CONFIG_SYSFS is not set, since they wont be able to change RPS settings.
If RPS was good for all workloads, activating it would make sense, but
this is not the case.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 11:38 ` Eric Dumazet
@ 2010-03-23 11:58 ` Changli Gao
2010-03-23 13:06 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Changli Gao @ 2010-03-23 11:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
On Tue, Mar 23, 2010 at 7:38 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 18:03 +0800, Changli Gao a écrit :
>
>
> I know _many_ applications that perform better when all network IRQS are
> directed to one CPU only. Single threaded UDP server for example, with
> fast answer, or routers, or ...
Yea, single threaded applicatons can't get much from RPS, but
multi-threaded applications such as apache do. And more and more
applications are programmed with the assumption there are more than
one CPUs/Cores.
>
> Many sysadmins tuned their machine to meet this requirement. Installing
> 2.6.35, if RPS enabled by default will make them unhappy, especially if
> CONFIG_SYSFS is not set, since they wont be able to change RPS settings.
>
> If RPS was good for all workloads, activating it would make sense, but
> this is not the case.
>
The sysadmins you mentioned above are Linux experters, and they know
how to tunning the system to get the best performance, but the default
configuration should be for the ordinary users, who don't know Linux
much.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 11:58 ` Changli Gao
@ 2010-03-23 13:06 ` Eric Dumazet
2010-03-23 19:09 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2010-03-23 13:06 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
Le mardi 23 mars 2010 à 19:58 +0800, Changli Gao a écrit :
> The sysadmins you mentioned above are Linux experters, and they know
> how to tunning the system to get the best performance, but the default
> configuration should be for the ordinary users, who don't know Linux
> much.
>
I strongly NACK your change at this very moment, many devices run
without a linux expert to tune them, thanks.
Ordinary users dont need RPS magic in 2.6.35.
Maybe in 2.6.38+ we can reconsider this, if RPS deployment happens to be
successful, thanks to experts doing their job and reports.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 13:06 ` Eric Dumazet
@ 2010-03-23 19:09 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2010-03-23 19:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiaosuo, therbert, netdev, bhutchings
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 23 Mar 2010 14:06:20 +0100
> Le mardi 23 mars 2010 à 19:58 +0800, Changli Gao a écrit :
>
>> The sysadmins you mentioned above are Linux experters, and they know
>> how to tunning the system to get the best performance, but the default
>> configuration should be for the ordinary users, who don't know Linux
>> much.
>>
>
> I strongly NACK your change at this very moment, many devices run
> without a linux expert to tune them, thanks.
Right, we're not GNOME, we don't screw knowledgable users of
our software by default.
I agree with Eric %100 on this.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
2010-03-23 10:03 ` Changli Gao
2010-03-23 11:38 ` Eric Dumazet
@ 2010-03-23 14:05 ` Andi Kleen
1 sibling, 0 replies; 12+ messages in thread
From: Andi Kleen @ 2010-03-23 14:05 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, David S. Miller, Tom Herbert, netdev, Ben Hutchings
Changli Gao <xiaosuo@gmail.com> writes:
>
> I do think distributing packets fairly among all the online CPUs will
> helps most of users. I remember the smp_affinity of IRQ is the
> online_cpu_mask be default.
The established wisdom is that most apps prefer locality.
The cost of data transfer between CPUs is worse than the
overhead of processing the packet header in the stack.
That's why no system does interrupt RR by default anymore.
Besides if you do such a change you would need extensive benchmarking
to verify that it's a good idea.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-03-23 19:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-23 6:24 [PATCH] rps: make distributing packets fairly among all the online CPUs default Changli Gao
2010-03-23 7:20 ` Eric Dumazet
2010-03-23 9:56 ` Changli Gao
2010-03-23 11:23 ` Eric Dumazet
2010-03-23 11:27 ` Changli Gao
2010-03-23 7:27 ` Eric Dumazet
2010-03-23 10:03 ` Changli Gao
2010-03-23 11:38 ` Eric Dumazet
2010-03-23 11:58 ` Changli Gao
2010-03-23 13:06 ` Eric Dumazet
2010-03-23 19:09 ` David Miller
2010-03-23 14:05 ` Andi Kleen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).