In nohz_full mode, by default distribute networking shim
interrupts across the housekeeping cores, not all the cores.
Signed-off-by: Chris Metcalf <[email protected]>
---
The alternate approaches to this might be:
1. "#define housekeeping_mask cpu_online_mask" in the non-nohz_full
arm in <linux/tick.h>, then just unconditionally use
"housekeeping_mask".
2. Provide an accessor that returns the cpumask to use for housekeeping
chores and implement it in the obvious ways for both nohz_full
and non-nohz_full.
The latter seems like arguably the most satisfying approach, but
the patch below is, if nothing else, suitable to push for 4.3
without any further API development work.
Frederic (or others), comments?
drivers/net/ethernet/tile/tilegx.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index a3f7610002aa..7687c62e7d75 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -40,6 +40,7 @@
#include <linux/tcp.h>
#include <linux/net_tstamp.h>
#include <linux/ptp_clock_kernel.h>
+#include <linux/tick.h>
#include <asm/checksum.h>
#include <asm/homecache.h>
@@ -2272,8 +2273,13 @@ static int __init tile_net_init_module(void)
for (i = 0; gxio_mpipe_link_enumerate_mac(i, name, mac) >= 0; i++)
tile_net_dev_init(name, mac);
- if (!network_cpus_init())
+ if (!network_cpus_init()) {
+#ifdef CONFIG_NO_HZ_FULL
+ network_cpus_map = *housekeeping_mask;
+#else
network_cpus_map = *cpu_online_mask;
+#endif
+ }
return 0;
}
--
2.1.2
On Fri, Jul 10, 2015 at 01:33:44PM -0400, Chris Metcalf wrote:
> In nohz_full mode, by default distribute networking shim
> interrupts across the housekeeping cores, not all the cores.
I can't really tell, I have no idea what this driver does. It seems
to be about networking CPUs but I have no idea what we are affining
here. Whether it is task, interrupt, ... And what those affine things do,
if it is safe to do that reduce affinity etc..
I looked at the driver but I can't make my way there. I think you
need a more detailed changelog :-)
> Signed-off-by: Chris Metcalf <[email protected]>
> ---
> The alternate approaches to this might be:
>
> 1. "#define housekeeping_mask cpu_online_mask" in the non-nohz_full
> arm in <linux/tick.h>, then just unconditionally use
> "housekeeping_mask".
Indeed we are doing more and more references on housekeeping_mask, so
we should probably think about an off-case.
Now the nohz-full off-case should rather be cpu_possible_mask than
cpu_online_mask. housekeeping_mask doesn't take into account onlining
at all.
>
> 2. Provide an accessor that returns the cpumask to use for housekeeping
> chores and implement it in the obvious ways for both nohz_full
> and non-nohz_full.
>
> The latter seems like arguably the most satisfying approach, but
> the patch below is, if nothing else, suitable to push for 4.3
> without any further API development work.
I don't know. 1) looks easier.
>
> Frederic (or others), comments?
>
> drivers/net/ethernet/tile/tilegx.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
> index a3f7610002aa..7687c62e7d75 100644
> --- a/drivers/net/ethernet/tile/tilegx.c
> +++ b/drivers/net/ethernet/tile/tilegx.c
> @@ -40,6 +40,7 @@
> #include <linux/tcp.h>
> #include <linux/net_tstamp.h>
> #include <linux/ptp_clock_kernel.h>
> +#include <linux/tick.h>
>
> #include <asm/checksum.h>
> #include <asm/homecache.h>
> @@ -2272,8 +2273,13 @@ static int __init tile_net_init_module(void)
> for (i = 0; gxio_mpipe_link_enumerate_mac(i, name, mac) >= 0; i++)
> tile_net_dev_init(name, mac);
>
> - if (!network_cpus_init())
> + if (!network_cpus_init()) {
> +#ifdef CONFIG_NO_HZ_FULL
> + network_cpus_map = *housekeeping_mask;
> +#else
> network_cpus_map = *cpu_online_mask;
> +#endif
> + }
>
> return 0;
> }
> --
> 2.1.2
>
On 07/10/2015 02:24 PM, Frederic Weisbecker wrote:
> On Fri, Jul 10, 2015 at 01:33:44PM -0400, Chris Metcalf wrote:
>> In nohz_full mode, by default distribute networking shim
>> interrupts across the housekeeping cores, not all the cores.
> I can't really tell, I have no idea what this driver does. It seems
> to be about networking CPUs but I have no idea what we are affining
> here. Whether it is task, interrupt, ... And what those affine things do,
> if it is safe to do that reduce affinity etc..
>
> I looked at the driver but I can't make my way there. I think you
> need a more detailed changelog :-)
Fair enough! :-) See updated changelog to follow.
>> Signed-off-by: Chris Metcalf <[email protected]>
>> ---
>> The alternate approaches to this might be:
>>
>> 1. "#define housekeeping_mask cpu_online_mask" in the non-nohz_full
>> arm in <linux/tick.h>, then just unconditionally use
>> "housekeeping_mask".
> Indeed we are doing more and more references on housekeeping_mask, so
> we should probably think about an off-case.
>
> Now the nohz-full off-case should rather be cpu_possible_mask than
> cpu_online_mask. housekeeping_mask doesn't take into account onlining
> at all.
That suggests that in this case, we might want to default to
something like "housekeeping_mask & cpu_online_mask",
since you really don't want to send irqs to offline cores to
process your packets :-)
The tilegx chips typically don't do cpu offlining anyway, since
we've never really found a usecase, so whatever you boot with
you always have available. We do have support for a bare-metal
mode which you can run on some of the cores, so you may start
with fewer than cpu_possible actually running, but it will always
be that same set of cores.
So this does suggest that my original patch is wrong for that
same reason.
>> 2. Provide an accessor that returns the cpumask to use for housekeeping
>> chores and implement it in the obvious ways for both nohz_full
>> and non-nohz_full.
>>
>> The latter seems like arguably the most satisfying approach, but
>> the patch below is, if nothing else, suitable to push for 4.3
>> without any further API development work.
> I don't know. 1) looks easier.
On reflection, the problem with (1) is that if you are in NO_HZ_FULL
mode but !tick_nohz_full_enabled(), you want to fall back to just
using cpu_possible_mask anyway. So I think a simple accessor that
returns an appropriate cpumask pointer is probably the best bet
(along the lines of the existing is_housekeeping_cpu() accessor).
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
Normally the tilegx networking shim sends irqs to all the cores
to distribute the load of processing incoming-packet interrupts,
so that you can get to multiple Gb's of traffic inbound.
However, in nohz_full mode we don't want to interrupt the
nohz_full cores by default, so we limit the set of cores we use
to only the online housekeeping cores.
To make client code easier to read, we introduce a new nohz_full
accessor, housekeeping_cpumask(), which returns a pointer to the
housekeeping_mask if nohz_full is enabled, and otherwise returns
the cpu_possible_mask.
Signed-off-by: Chris Metcalf <[email protected]>
---
drivers/net/ethernet/tile/tilegx.c | 4 +++-
include/linux/tick.h | 9 +++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index a3f7610002aa..0a15acc075b3 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -40,6 +40,7 @@
#include <linux/tcp.h>
#include <linux/net_tstamp.h>
#include <linux/ptp_clock_kernel.h>
+#include <linux/tick.h>
#include <asm/checksum.h>
#include <asm/homecache.h>
@@ -2273,7 +2274,8 @@ static int __init tile_net_init_module(void)
tile_net_dev_init(name, mac);
if (!network_cpus_init())
- network_cpus_map = *cpu_online_mask;
+ cpumask_and(&network_cpus_map, housekeeping_cpumask(),
+ cpu_online_mask);
return 0;
}
diff --git a/include/linux/tick.h b/include/linux/tick.h
index 3741ba1a652c..d3b00f0b2221 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -160,6 +160,15 @@ static inline void tick_nohz_full_kick_all(void) { }
static inline void __tick_nohz_task_switch(struct task_struct *tsk) { }
#endif
+static inline const struct cpumask *housekeeping_cpumask(void)
+{
+#ifdef CONFIG_NO_HZ_FULL
+ if (tick_nohz_full_enabled())
+ return housekeeping_mask;
+#endif
+ return cpu_possible_mask;
+}
+
static inline bool is_housekeeping_cpu(int cpu)
{
#ifdef CONFIG_NO_HZ_FULL
--
2.1.2
On Fri, Jul 10, 2015 at 03:37:25PM -0400, Chris Metcalf wrote:
> Normally the tilegx networking shim sends irqs to all the cores
> to distribute the load of processing incoming-packet interrupts,
> so that you can get to multiple Gb's of traffic inbound.
>
> However, in nohz_full mode we don't want to interrupt the
> nohz_full cores by default, so we limit the set of cores we use
> to only the online housekeeping cores.
>
> To make client code easier to read, we introduce a new nohz_full
> accessor, housekeeping_cpumask(), which returns a pointer to the
> housekeeping_mask if nohz_full is enabled, and otherwise returns
> the cpu_possible_mask.
>
> Signed-off-by: Chris Metcalf <[email protected]>
> ---
[..]
> +static inline const struct cpumask *housekeeping_cpumask(void)
> +{
> +#ifdef CONFIG_NO_HZ_FULL
> + if (tick_nohz_full_enabled())
> + return housekeeping_mask;
> +#endif
Just a small comment:
We can take these checks out from under a #ifdef CONFIG_NO_HZ_FULL
check, given that are stubbed tick_nohz_full_enabled() defined above.
Josh
On 7/10/2015 6:45 PM, Josh Cartwright wrote:
>> +static inline const struct cpumask *housekeeping_cpumask(void)
>> >+{
>> >+#ifdef CONFIG_NO_HZ_FULL
>> >+ if (tick_nohz_full_enabled())
>> >+ return housekeeping_mask;
>> >+#endif
> Just a small comment:
>
> We can take these checks out from under a #ifdef CONFIG_NO_HZ_FULL
> check, given that are stubbed tick_nohz_full_enabled() defined above.
True for the "if" clause, but the "housekeeping_mask" variable is only defined
under CONFIG_NO_HZ_FULL.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
On Fri, Jul 10, 2015 at 07:06:23PM -0400, Chris Metcalf wrote:
> On 7/10/2015 6:45 PM, Josh Cartwright wrote:
> >>+static inline const struct cpumask *housekeeping_cpumask(void)
> >>>+{
> >>>+#ifdef CONFIG_NO_HZ_FULL
> >>>+ if (tick_nohz_full_enabled())
> >>>+ return housekeeping_mask;
> >>>+#endif
> >Just a small comment:
> >
> >We can take these checks out from under a #ifdef CONFIG_NO_HZ_FULL
> >check, given that are stubbed tick_nohz_full_enabled() defined above.
>
> True for the "if" clause, but the "housekeeping_mask" variable is only defined
> under CONFIG_NO_HZ_FULL.
Indeed! I should have read more carefully.
Sorry for the noise.
Josh
On Fri, Jul 10, 2015 at 03:05:02PM -0400, Chris Metcalf wrote:
> On 07/10/2015 02:24 PM, Frederic Weisbecker wrote:
> >Indeed we are doing more and more references on housekeeping_mask, so
> >we should probably think about an off-case.
> >
> >Now the nohz-full off-case should rather be cpu_possible_mask than
> >cpu_online_mask. housekeeping_mask doesn't take into account onlining
> >at all.
>
> That suggests that in this case, we might want to default to
> something like "housekeeping_mask & cpu_online_mask",
> since you really don't want to send irqs to offline cores to
> process your packets :-)
In any case it must be up to the drivers to do that. Define
housekeeping_mask as a subset of the online mask complicates a lot
of things. It pushes hotplug complexity to the nohz code for no
reasons. It's up to the drivers and subsystems to handle that really.
>
> The tilegx chips typically don't do cpu offlining anyway, since
> we've never really found a usecase, so whatever you boot with
> you always have available. We do have support for a bare-metal
> mode which you can run on some of the cores, so you may start
> with fewer than cpu_possible actually running, but it will always
> be that same set of cores.
And that bare metal mode runs out of Linux?
Note that in nohz_full, The boot CPU can't be offline anyway.
>
> So this does suggest that my original patch is wrong for that
> same reason.
>
> >>2. Provide an accessor that returns the cpumask to use for housekeeping
> >> chores and implement it in the obvious ways for both nohz_full
> >> and non-nohz_full.
> >>
> >>The latter seems like arguably the most satisfying approach, but
> >>the patch below is, if nothing else, suitable to push for 4.3
> >>without any further API development work.
> >I don't know. 1) looks easier.
>
> On reflection, the problem with (1) is that if you are in NO_HZ_FULL
> mode but !tick_nohz_full_enabled(), you want to fall back to just
> using cpu_possible_mask anyway. So I think a simple accessor that
> returns an appropriate cpumask pointer is probably the best bet
> (along the lines of the existing is_housekeeping_cpu() accessor).
Ok!
On Fri, Jul 10, 2015 at 03:37:25PM -0400, Chris Metcalf wrote:
> Normally the tilegx networking shim sends irqs to all the cores
> to distribute the load of processing incoming-packet interrupts,
> so that you can get to multiple Gb's of traffic inbound.
>
> However, in nohz_full mode we don't want to interrupt the
> nohz_full cores by default, so we limit the set of cores we use
> to only the online housekeeping cores.
>
> To make client code easier to read, we introduce a new nohz_full
> accessor, housekeeping_cpumask(), which returns a pointer to the
> housekeeping_mask if nohz_full is enabled, and otherwise returns
> the cpu_possible_mask.
>
> Signed-off-by: Chris Metcalf <[email protected]>
Looks good. Since it modifies nohz core stuff, do you mind if I take it?
Thanks.
> ---
> drivers/net/ethernet/tile/tilegx.c | 4 +++-
> include/linux/tick.h | 9 +++++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
> index a3f7610002aa..0a15acc075b3 100644
> --- a/drivers/net/ethernet/tile/tilegx.c
> +++ b/drivers/net/ethernet/tile/tilegx.c
> @@ -40,6 +40,7 @@
> #include <linux/tcp.h>
> #include <linux/net_tstamp.h>
> #include <linux/ptp_clock_kernel.h>
> +#include <linux/tick.h>
>
> #include <asm/checksum.h>
> #include <asm/homecache.h>
> @@ -2273,7 +2274,8 @@ static int __init tile_net_init_module(void)
> tile_net_dev_init(name, mac);
>
> if (!network_cpus_init())
> - network_cpus_map = *cpu_online_mask;
> + cpumask_and(&network_cpus_map, housekeeping_cpumask(),
> + cpu_online_mask);
>
> return 0;
> }
> diff --git a/include/linux/tick.h b/include/linux/tick.h
> index 3741ba1a652c..d3b00f0b2221 100644
> --- a/include/linux/tick.h
> +++ b/include/linux/tick.h
> @@ -160,6 +160,15 @@ static inline void tick_nohz_full_kick_all(void) { }
> static inline void __tick_nohz_task_switch(struct task_struct *tsk) { }
> #endif
>
> +static inline const struct cpumask *housekeeping_cpumask(void)
> +{
> +#ifdef CONFIG_NO_HZ_FULL
> + if (tick_nohz_full_enabled())
> + return housekeeping_mask;
> +#endif
> + return cpu_possible_mask;
> +}
> +
> static inline bool is_housekeeping_cpu(int cpu)
> {
> #ifdef CONFIG_NO_HZ_FULL
> --
> 2.1.2
>
> On Jul 11, 2015, at 10:44 AM, Frederic Weisbecker <[email protected]> wrote:
>
>> On Fri, Jul 10, 2015 at 03:37:25PM -0400, Chris Metcalf wrote:
>> Normally the tilegx networking shim sends irqs to all the cores
>> to distribute the load of processing incoming-packet interrupts,
>> so that you can get to multiple Gb's of traffic inbound.
>>
>> However, in nohz_full mode we don't want to interrupt the
>> nohz_full cores by default, so we limit the set of cores we use
>> to only the online housekeeping cores.
>>
>> To make client code easier to read, we introduce a new nohz_full
>> accessor, housekeeping_cpumask(), which returns a pointer to the
>> housekeeping_mask if nohz_full is enabled, and otherwise returns
>> the cpu_possible_mask.
>>
>> Signed-off-by: Chris Metcalf <[email protected]>
>
> Looks good. Since it modifies nohz core stuff, do you mind if I take it?
>
> Thanks.
Sure, that's fine. Thanks.
>
>> ---
>> drivers/net/ethernet/tile/tilegx.c | 4 +++-
>> include/linux/tick.h | 9 +++++++++
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
>> index a3f7610002aa..0a15acc075b3 100644
>> --- a/drivers/net/ethernet/tile/tilegx.c
>> +++ b/drivers/net/ethernet/tile/tilegx.c
>> @@ -40,6 +40,7 @@
>> #include <linux/tcp.h>
>> #include <linux/net_tstamp.h>
>> #include <linux/ptp_clock_kernel.h>
>> +#include <linux/tick.h>
>>
>> #include <asm/checksum.h>
>> #include <asm/homecache.h>
>> @@ -2273,7 +2274,8 @@ static int __init tile_net_init_module(void)
>> tile_net_dev_init(name, mac);
>>
>> if (!network_cpus_init())
>> - network_cpus_map = *cpu_online_mask;
>> + cpumask_and(&network_cpus_map, housekeeping_cpumask(),
>> + cpu_online_mask);
>>
>> return 0;
>> }
>> diff --git a/include/linux/tick.h b/include/linux/tick.h
>> index 3741ba1a652c..d3b00f0b2221 100644
>> --- a/include/linux/tick.h
>> +++ b/include/linux/tick.h
>> @@ -160,6 +160,15 @@ static inline void tick_nohz_full_kick_all(void) { }
>> static inline void __tick_nohz_task_switch(struct task_struct *tsk) { }
>> #endif
>>
>> +static inline const struct cpumask *housekeeping_cpumask(void)
>> +{
>> +#ifdef CONFIG_NO_HZ_FULL
>> + if (tick_nohz_full_enabled())
>> + return housekeeping_mask;
>> +#endif
>> + return cpu_possible_mask;
>> +}
>> +
>> static inline bool is_housekeeping_cpu(int cpu)
>> {
>> #ifdef CONFIG_NO_HZ_FULL
>> --
>> 2.1.2
>>
On 07/11/2015 10:30 AM, Frederic Weisbecker wrote:
> On Fri, Jul 10, 2015 at 03:05:02PM -0400, Chris Metcalf wrote:
>> The tilegx chips typically don't do cpu offlining anyway, since
>> we've never really found a usecase, so whatever you boot with
>> you always have available. We do have support for a bare-metal
>> mode which you can run on some of the cores, so you may start
>> with fewer than cpu_possible actually running, but it will always
>> be that same set of cores.
> And that bare metal mode runs out of Linux?
The bare metal environment runs on cpus that have been marked
as unavailable to Linux, so Linux just sees them as permanently
offlined. There is a BME driver (which we haven't upstreamed,
since the BME isn't upstreamed either) that arranges to share
memory between the BME and Linux.
I don't think that many customers are using the BME in any
case. We push all of them towards using our dataplane mode
instead, since it almost always works just as well from a
performance perspective, and is easier to develop code for.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
On Mon, Jul 20, 2015 at 05:22:12PM -0400, Chris Metcalf wrote:
> On 07/11/2015 10:30 AM, Frederic Weisbecker wrote:
> >On Fri, Jul 10, 2015 at 03:05:02PM -0400, Chris Metcalf wrote:
> >>The tilegx chips typically don't do cpu offlining anyway, since
> >>we've never really found a usecase, so whatever you boot with
> >>you always have available. We do have support for a bare-metal
> >>mode which you can run on some of the cores, so you may start
> >>with fewer than cpu_possible actually running, but it will always
> >>be that same set of cores.
> >And that bare metal mode runs out of Linux?
>
> The bare metal environment runs on cpus that have been marked
> as unavailable to Linux, so Linux just sees them as permanently
> offlined. There is a BME driver (which we haven't upstreamed,
> since the BME isn't upstreamed either) that arranges to share
> memory between the BME and Linux.
>
> I don't think that many customers are using the BME in any
> case. We push all of them towards using our dataplane mode
> instead, since it almost always works just as well from a
> performance perspective, and is easier to develop code for.
So bare metal mode is different than dataplane mode, right?
Where bare metal mode offlines the CPU and IIUC dataplane mode
instead uses CPUs that are available to Linux, just isolated
with nohz and various affinity stuff, right?
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
On 07/20/2015 05:49 PM, Frederic Weisbecker wrote:
> On Mon, Jul 20, 2015 at 05:22:12PM -0400, Chris Metcalf wrote:
>> On 07/11/2015 10:30 AM, Frederic Weisbecker wrote:
>>> On Fri, Jul 10, 2015 at 03:05:02PM -0400, Chris Metcalf wrote:
>>>> The tilegx chips typically don't do cpu offlining anyway, since
>>>> we've never really found a usecase, so whatever you boot with
>>>> you always have available. We do have support for a bare-metal
>>>> mode which you can run on some of the cores, so you may start
>>>> with fewer than cpu_possible actually running, but it will always
>>>> be that same set of cores.
>>> And that bare metal mode runs out of Linux?
>> The bare metal environment runs on cpus that have been marked
>> as unavailable to Linux, so Linux just sees them as permanently
>> offlined. There is a BME driver (which we haven't upstreamed,
>> since the BME isn't upstreamed either) that arranges to share
>> memory between the BME and Linux.
>>
>> I don't think that many customers are using the BME in any
>> case. We push all of them towards using our dataplane mode
>> instead, since it almost always works just as well from a
>> performance perspective, and is easier to develop code for.
> So bare metal mode is different than dataplane mode, right?
> Where bare metal mode offlines the CPU and IIUC dataplane mode
> instead uses CPUs that are available to Linux, just isolated
> with nohz and various affinity stuff, right?
Yes, exactly. Cores running the bare metal environment are
NOT running Linux at all, just talking directly to the Tilera
hypervisor.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com