linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4] sched/isolation: isolate from handling managed interrupt
@ 2020-01-20  9:16 Ming Lei
  2020-01-22 13:25 ` Thomas Gleixner
  2020-01-22 15:34 ` [tip: irq/core] genirq, sched/isolation: Isolate from handling managed interrupts tip-bot2 for Ming Lei
  0 siblings, 2 replies; 7+ messages in thread
From: Ming Lei @ 2020-01-20  9:16 UTC (permalink / raw)
  To: linux-kernel, Thomas Gleixner
  Cc: Ming Lei, Ingo Molnar, Peter Zijlstra, Peter Xu, Juri Lelli

Userspace can't change managed interrupt's affinity via /proc interface,
however, applications often require the specified isolated CPUs not
disturbed by interrupts.

Add sub-parameter 'managed_irq' for 'isolcpus', so that we can isolate
from handling managed interrupt.

Not select irq effective CPU from isolated CPUs if the interrupt affinity
includes at least one housekeeping CPU. This way guarantees that isolated
CPUs won't be interrupted by managed irq if IO isn't submitted from any
isolated CPU.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
V4:
	- patch style fix
	- avoid unnecessary checks in hk_should_isolate()
V3:
	- add global lock to protect the global temporary cpumask
	- use delayed irq migration as suggested by Thomas 
V2:
	- not allocate cpumask in context with irq_desc::lock held
	- deal with cpu hotplug race with new flag of IRQD_MANAGED_FORCE_MIGRATE
	- use comment doc from Thomas

 .../admin-guide/kernel-parameters.txt         |  9 +++++
 include/linux/sched/isolation.h               |  1 +
 kernel/irq/cpuhotplug.c                       | 19 ++++++++-
 kernel/irq/manage.c                           | 39 ++++++++++++++++++-
 kernel/sched/isolation.c                      |  6 +++
 5 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6ec23e0..e0f18ac866d4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1933,6 +1933,15 @@
 			  <cpu number> begins at 0 and the maximum value is
 			  "number of CPUs in system - 1".
 
+			managed_irq
+			  Isolate from handling managed interrupt. Userspace can't
+			  change managed interrupt's affinity via /proc interface,
+			  however application often requires the specified isolated
+			  CPUs not disturbed by interrupts. This way guarantees that
+			  isolated CPU won't be interrupted if IO isn't submitted
+			  from isolated CPU when managed interrupt is used by IO
+			  drivers.
+
 			The format of <cpu-list> is described above.
 
 
diff --git a/include/linux/sched/isolation.h b/include/linux/sched/isolation.h
index 6c8512d3be88..0fbcbacd1b29 100644
--- a/include/linux/sched/isolation.h
+++ b/include/linux/sched/isolation.h
@@ -13,6 +13,7 @@ enum hk_flags {
 	HK_FLAG_TICK		= (1 << 4),
 	HK_FLAG_DOMAIN		= (1 << 5),
 	HK_FLAG_WQ		= (1 << 6),
+	HK_FLAG_MANAGED_IRQ	= (1 << 7),
 };
 
 #ifdef CONFIG_CPU_ISOLATION
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 6c7ca2e983a5..fbcba938a771 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/ratelimit.h>
 #include <linux/irq.h>
+#include <linux/sched/isolation.h>
 
 #include "internals.h"
 
@@ -171,6 +172,21 @@ void irq_migrate_all_off_this_cpu(void)
 	}
 }
 
+static bool hk_should_isolate(struct irq_data *data,
+			      const struct cpumask *affinity, unsigned int cpu)
+{
+	const struct cpumask *hk_mask;
+
+	if (!housekeeping_enabled(HK_FLAG_MANAGED_IRQ))
+		return false;
+
+	hk_mask = housekeeping_cpumask(HK_FLAG_MANAGED_IRQ);
+	if (cpumask_subset(irq_data_get_effective_affinity_mask(data), hk_mask))
+		return false;
+
+	return cpumask_test_cpu(cpu, hk_mask);
+}
+
 static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
 {
 	struct irq_data *data = irq_desc_get_irq_data(desc);
@@ -190,7 +206,8 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
 	 * CPU then it is already assigned to a CPU in the affinity
 	 * mask. No point in trying to move it around.
 	 */
-	if (!irqd_is_single_target(data))
+	if (!irqd_is_single_target(data) ||
+	    hk_should_isolate(data, affinity, cpu))
 		irq_set_affinity_locked(data, affinity, false);
 }
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1753486b440c..6c0e06c0d6d8 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/sched/rt.h>
 #include <linux/sched/task.h>
+#include <linux/sched/isolation.h>
 #include <uapi/linux/sched/types.h>
 #include <linux/task_work.h>
 
@@ -217,7 +218,43 @@ int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 	if (!chip || !chip->irq_set_affinity)
 		return -EINVAL;
 
-	ret = chip->irq_set_affinity(data, mask, force);
+	/*
+	 * If this is a managed interrupt and housekeeping is enabled on
+	 * it check whether the requested affinity mask intersects with
+	 * a housekeeping CPU. If so, then remove the isolated CPUs from
+	 * the mask and just keep the housekeeping CPU(s). This prevents
+	 * the affinity setter from routing the interrupt to an isolated
+	 * CPU to avoid that I/O submitted from a housekeeping CPU causes
+	 * interrupts on an isolated one.
+	 *
+	 * If the masks do not intersect or include online CPU(s) then
+	 * keep the requested mask. The isolated target CPUs are only
+	 * receiving interrupts when the I/O operation was submitted
+	 * directly from them.
+	 *
+	 * If all housekeeping CPUs in the affinity mask are offline,
+	 * we will migrate the irq from isolate CPU when any housekeeping
+	 * CPU in the mask becomes online.
+	 */
+	if (irqd_affinity_is_managed(data) &&
+	    housekeeping_enabled(HK_FLAG_MANAGED_IRQ)) {
+		static DEFINE_RAW_SPINLOCK(tmp_mask_lock);
+		static struct cpumask tmp_mask;
+		const struct cpumask *hk_mask, *prog_mask;
+
+		hk_mask = housekeeping_cpumask(HK_FLAG_MANAGED_IRQ);
+
+		raw_spin_lock(&tmp_mask_lock);
+		cpumask_and(&tmp_mask, mask, hk_mask);
+		if (!cpumask_intersects(&tmp_mask, cpu_online_mask))
+			prog_mask = mask;
+		else
+			prog_mask = &tmp_mask;
+		ret = chip->irq_set_affinity(data, prog_mask, force);
+		raw_spin_unlock(&tmp_mask_lock);
+	} else {
+		ret = chip->irq_set_affinity(data, mask, force);
+	}
 	switch (ret) {
 	case IRQ_SET_MASK_OK:
 	case IRQ_SET_MASK_OK_DONE:
diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 9fcb2a695a41..008d6ac2342b 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -163,6 +163,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
 			continue;
 		}
 
+		if (!strncmp(str, "managed_irq,", 12)) {
+			str += 12;
+			flags |= HK_FLAG_MANAGED_IRQ;
+			continue;
+		}
+
 		pr_warn("isolcpus: Error, unknown flag\n");
 		return 0;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V4] sched/isolation: isolate from handling managed interrupt
  2020-01-20  9:16 [PATCH V4] sched/isolation: isolate from handling managed interrupt Ming Lei
@ 2020-01-22 13:25 ` Thomas Gleixner
  2020-02-03 19:21   ` Peter Xu
  2020-01-22 15:34 ` [tip: irq/core] genirq, sched/isolation: Isolate from handling managed interrupts tip-bot2 for Ming Lei
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2020-01-22 13:25 UTC (permalink / raw)
  To: Ming Lei, linux-kernel
  Cc: Ming Lei, Ingo Molnar, Peter Zijlstra, Peter Xu, Juri Lelli

Ming,

Ming Lei <ming.lei@redhat.com> writes:
>  
> +static bool hk_should_isolate(struct irq_data *data,
> +			      const struct cpumask *affinity, unsigned int cpu)
> +{

the *affinity argument is unused.

I'll remove it myself.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tip: irq/core] genirq, sched/isolation: Isolate from handling managed interrupts
  2020-01-20  9:16 [PATCH V4] sched/isolation: isolate from handling managed interrupt Ming Lei
  2020-01-22 13:25 ` Thomas Gleixner
@ 2020-01-22 15:34 ` tip-bot2 for Ming Lei
  1 sibling, 0 replies; 7+ messages in thread
From: tip-bot2 for Ming Lei @ 2020-01-22 15:34 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Ming Lei, Thomas Gleixner, x86, LKML

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     11ea68f553e244851d15793a7fa33a97c46d8271
Gitweb:        https://git.kernel.org/tip/11ea68f553e244851d15793a7fa33a97c46d8271
Author:        Ming Lei <ming.lei@redhat.com>
AuthorDate:    Mon, 20 Jan 2020 17:16:25 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 22 Jan 2020 16:29:49 +01:00

genirq, sched/isolation: Isolate from handling managed interrupts

The affinity of managed interrupts is completely handled in the kernel and
cannot be changed via the /proc/irq/* interfaces from user space. As the
kernel tries to spread out interrupts evenly accross CPUs on x86 to prevent
vector exhaustion, it can happen that a managed interrupt whose affinity
mask contains both isolated and housekeeping CPUs is routed to an isolated
CPU. As a consequence IO submitted on a housekeeping CPU causes interrupts
on the isolated CPU.

Add a new sub-parameter 'managed_irq' for 'isolcpus' and the corresponding
logic in the interrupt affinity selection code.

The subparameter indicates to the interrupt affinity selection logic that
it should try to avoid the above scenario.

This isolation is best effort and only effective if the automatically
assigned interrupt mask of a device queue contains isolated and
housekeeping CPUs. If housekeeping CPUs are online then such interrupts are
directed to the housekeeping CPU so that IO submitted on the housekeeping
CPU cannot disturb the isolated CPU.

If a queue's affinity mask contains only isolated CPUs then this parameter
has no effect on the interrupt routing decision, though interrupts are only
happening when tasks running on those isolated CPUs submit IO. IO submitted
on housekeeping CPUs has no influence on those queues.

If the affinity mask contains both housekeeping and isolated CPUs, but none
of the contained housekeeping CPUs is online, then the interrupt is also
routed to an isolated CPU. Interrupts are only delivered when one of the
isolated CPUs in the affinity mask submits IO. If one of the contained
housekeeping CPUs comes online, the CPU hotplug logic migrates the
interrupt automatically back to the upcoming housekeeping CPU. Depending on
the type of interrupt controller, this can require that at least one
interrupt is delivered to the isolated CPU in order to complete the
migration.

[ tglx: Removed unused parameter, added and edited comments/documentation
  	and rephrased the changelog so it contains more details. ]

Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20200120091625.17912-1-ming.lei@redhat.com

---
 Documentation/admin-guide/kernel-parameters.txt | 26 +++++++++-
 include/linux/sched/isolation.h                 |  1 +-
 kernel/irq/cpuhotplug.c                         | 21 +++++++-
 kernel/irq/manage.c                             | 41 +++++++++++++++-
 kernel/sched/isolation.c                        |  6 ++-
 5 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6e..765e427 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1933,9 +1933,31 @@
 			  <cpu number> begins at 0 and the maximum value is
 			  "number of CPUs in system - 1".
 
-			The format of <cpu-list> is described above.
-
+			managed_irq
+
+			  Isolate from being targeted by managed interrupts
+			  which have an interrupt mask containing isolated
+			  CPUs. The affinity of managed interrupts is
+			  handled by the kernel and cannot be changed via
+			  the /proc/irq/* interfaces.
+
+			  This isolation is best effort and only effective
+			  if the automatically assigned interrupt mask of a
+			  device queue contains isolated and housekeeping
+			  CPUs. If housekeeping CPUs are online then such
+			  interrupts are directed to the housekeeping CPU
+			  so that IO submitted on the housekeeping CPU
+			  cannot disturb the isolated CPU.
+
+			  If a queue's affinity mask contains only isolated
+			  CPUs then this parameter has no effect on the
+			  interrupt routing decision, though interrupts are
+			  only delivered when tasks running on those
+			  isolated CPUs submit IO. IO submitted on
+			  housekeeping CPUs has no influence on those
+			  queues.
 
+			The format of <cpu-list> is described above.
 
 	iucv=		[HW,NET]
 
diff --git a/include/linux/sched/isolation.h b/include/linux/sched/isolation.h
index 6c8512d..0fbcbac 100644
--- a/include/linux/sched/isolation.h
+++ b/include/linux/sched/isolation.h
@@ -13,6 +13,7 @@ enum hk_flags {
 	HK_FLAG_TICK		= (1 << 4),
 	HK_FLAG_DOMAIN		= (1 << 5),
 	HK_FLAG_WQ		= (1 << 6),
+	HK_FLAG_MANAGED_IRQ	= (1 << 7),
 };
 
 #ifdef CONFIG_CPU_ISOLATION
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 6c7ca2e..02236b1 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/ratelimit.h>
 #include <linux/irq.h>
+#include <linux/sched/isolation.h>
 
 #include "internals.h"
 
@@ -171,6 +172,20 @@ void irq_migrate_all_off_this_cpu(void)
 	}
 }
 
+static bool hk_should_isolate(struct irq_data *data, unsigned int cpu)
+{
+	const struct cpumask *hk_mask;
+
+	if (!housekeeping_enabled(HK_FLAG_MANAGED_IRQ))
+		return false;
+
+	hk_mask = housekeeping_cpumask(HK_FLAG_MANAGED_IRQ);
+	if (cpumask_subset(irq_data_get_effective_affinity_mask(data), hk_mask))
+		return false;
+
+	return cpumask_test_cpu(cpu, hk_mask);
+}
+
 static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
 {
 	struct irq_data *data = irq_desc_get_irq_data(desc);
@@ -188,9 +203,11 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
 	/*
 	 * If the interrupt can only be directed to a single target
 	 * CPU then it is already assigned to a CPU in the affinity
-	 * mask. No point in trying to move it around.
+	 * mask. No point in trying to move it around unless the
+	 * isolation mechanism requests to move it to an upcoming
+	 * housekeeping CPU.
 	 */
-	if (!irqd_is_single_target(data))
+	if (!irqd_is_single_target(data) || hk_should_isolate(data, cpu))
 		irq_set_affinity_locked(data, affinity, false);
 }
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index b6c53ab..818b280 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/sched/rt.h>
 #include <linux/sched/task.h>
+#include <linux/sched/isolation.h>
 #include <uapi/linux/sched/types.h>
 #include <linux/task_work.h>
 
@@ -217,7 +218,45 @@ int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 	if (!chip || !chip->irq_set_affinity)
 		return -EINVAL;
 
-	ret = chip->irq_set_affinity(data, mask, force);
+	/*
+	 * If this is a managed interrupt and housekeeping is enabled on
+	 * it check whether the requested affinity mask intersects with
+	 * a housekeeping CPU. If so, then remove the isolated CPUs from
+	 * the mask and just keep the housekeeping CPU(s). This prevents
+	 * the affinity setter from routing the interrupt to an isolated
+	 * CPU to avoid that I/O submitted from a housekeeping CPU causes
+	 * interrupts on an isolated one.
+	 *
+	 * If the masks do not intersect or include online CPU(s) then
+	 * keep the requested mask. The isolated target CPUs are only
+	 * receiving interrupts when the I/O operation was submitted
+	 * directly from them.
+	 *
+	 * If all housekeeping CPUs in the affinity mask are offline, the
+	 * interrupt will be migrated by the CPU hotplug code once a
+	 * housekeeping CPU which belongs to the affinity mask comes
+	 * online.
+	 */
+	if (irqd_affinity_is_managed(data) &&
+	    housekeeping_enabled(HK_FLAG_MANAGED_IRQ)) {
+		const struct cpumask *hk_mask, *prog_mask;
+
+		static DEFINE_RAW_SPINLOCK(tmp_mask_lock);
+		static struct cpumask tmp_mask;
+
+		hk_mask = housekeeping_cpumask(HK_FLAG_MANAGED_IRQ);
+
+		raw_spin_lock(&tmp_mask_lock);
+		cpumask_and(&tmp_mask, mask, hk_mask);
+		if (!cpumask_intersects(&tmp_mask, cpu_online_mask))
+			prog_mask = mask;
+		else
+			prog_mask = &tmp_mask;
+		ret = chip->irq_set_affinity(data, prog_mask, force);
+		raw_spin_unlock(&tmp_mask_lock);
+	} else {
+		ret = chip->irq_set_affinity(data, mask, force);
+	}
 	switch (ret) {
 	case IRQ_SET_MASK_OK:
 	case IRQ_SET_MASK_OK_DONE:
diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index 9fcb2a6..008d6ac 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -163,6 +163,12 @@ static int __init housekeeping_isolcpus_setup(char *str)
 			continue;
 		}
 
+		if (!strncmp(str, "managed_irq,", 12)) {
+			str += 12;
+			flags |= HK_FLAG_MANAGED_IRQ;
+			continue;
+		}
+
 		pr_warn("isolcpus: Error, unknown flag\n");
 		return 0;
 	}

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V4] sched/isolation: isolate from handling managed interrupt
  2020-01-22 13:25 ` Thomas Gleixner
@ 2020-02-03 19:21   ` Peter Xu
  2020-02-03 23:15     ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Xu @ 2020-02-03 19:21 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ming Lei, linux-kernel, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Luiz Capitulino

On Wed, Jan 22, 2020 at 02:25:38PM +0100, Thomas Gleixner wrote:
> Ming,
> 
> Ming Lei <ming.lei@redhat.com> writes:
> >  
> > +static bool hk_should_isolate(struct irq_data *data,
> > +			      const struct cpumask *affinity, unsigned int cpu)
> > +{
> 
> the *affinity argument is unused.
> 
> I'll remove it myself.

Hi, Ming, Thomas,

The new "managed_irq" works for us, thanks for both of your work!

However I just noticed that this new sub-parameter might break users
if applied incorrectly to old kernels, because iiuc "isolcpus="
parameter will not apply at all when there's unknown sub-parameters:

static int __init housekeeping_isolcpus_setup(char *str)
{
	unsigned int flags = 0;

	while (isalpha(*str)) {
                ...
                pr_warn("isolcpus: Error, unknown flag\n");
                return 0;
        }
        ...
}

Then the same kernel parameter will break isolcpus= if the user
reboots and switches to an older kernel.

A solution to this could be that we introduce an isolated parameter
for "managed_irq", then on the old kernels only the new parameter will
be ignored rather than the whole "isolcpus=" parameter, so nothing
will break.

I'm not sure whether it's already too late for this, or if there's any
better alternative.  Just raise this question up to see whether we
still have chance to fix this up.

Thanks,

-- 
Peter Xu


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4] sched/isolation: isolate from handling managed interrupt
  2020-02-03 19:21   ` Peter Xu
@ 2020-02-03 23:15     ` Thomas Gleixner
  2020-02-03 23:57       ` Peter Xu
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2020-02-03 23:15 UTC (permalink / raw)
  To: Peter Xu
  Cc: Ming Lei, linux-kernel, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Luiz Capitulino

Peter Xu <peterx@redhat.com> writes:
> The new "managed_irq" works for us, thanks for both of your work!
>
> However I just noticed that this new sub-parameter might break users
> if applied incorrectly to old kernels, because iiuc "isolcpus="
> parameter will not apply at all when there's unknown sub-parameters:
>
> static int __init housekeeping_isolcpus_setup(char *str)
> {
> 	unsigned int flags = 0;
>
> 	while (isalpha(*str)) {
>                 ...
>                 pr_warn("isolcpus: Error, unknown flag\n");
>                 return 0;
>         }
>         ...
> }
>
> Then the same kernel parameter will break isolcpus= if the user
> reboots and switches to an older kernel.
>
> A solution to this could be that we introduce an isolated parameter
> for "managed_irq", then on the old kernels only the new parameter will
> be ignored rather than the whole "isolcpus=" parameter, so nothing
> will break.
>
> I'm not sure whether it's already too late for this, or if there's any
> better alternative.  Just raise this question up to see whether we
> still have chance to fix this up.

No, really. The basic guarantee is that your new kernel is going to work
fine with the previous command line, but making a guarantee that new
command line options still work on an old kernel are just creating a
horrible mess. So if that command line interface was not designed to
handle unknown arguments in the first place, you better fix that.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4] sched/isolation: isolate from handling managed interrupt
  2020-02-03 23:15     ` Thomas Gleixner
@ 2020-02-03 23:57       ` Peter Xu
  2020-02-04  9:13         ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Xu @ 2020-02-03 23:57 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ming Lei, linux-kernel, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Luiz Capitulino

On Mon, Feb 03, 2020 at 11:15:50PM +0000, Thomas Gleixner wrote:
> Peter Xu <peterx@redhat.com> writes:
> > The new "managed_irq" works for us, thanks for both of your work!
> >
> > However I just noticed that this new sub-parameter might break users
> > if applied incorrectly to old kernels, because iiuc "isolcpus="
> > parameter will not apply at all when there's unknown sub-parameters:
> >
> > static int __init housekeeping_isolcpus_setup(char *str)
> > {
> > 	unsigned int flags = 0;
> >
> > 	while (isalpha(*str)) {
> >                 ...
> >                 pr_warn("isolcpus: Error, unknown flag\n");
> >                 return 0;
> >         }
> >         ...
> > }
> >
> > Then the same kernel parameter will break isolcpus= if the user
> > reboots and switches to an older kernel.
> >
> > A solution to this could be that we introduce an isolated parameter
> > for "managed_irq", then on the old kernels only the new parameter will
> > be ignored rather than the whole "isolcpus=" parameter, so nothing
> > will break.
> >
> > I'm not sure whether it's already too late for this, or if there's any
> > better alternative.  Just raise this question up to see whether we
> > still have chance to fix this up.
> 
> No, really. The basic guarantee is that your new kernel is going to work
> fine with the previous command line, but making a guarantee that new
> command line options still work on an old kernel are just creating a
> horrible mess. So if that command line interface was not designed to
> handle unknown arguments in the first place, you better fix that.

Hi, Thomas,

Just to make sure I understand it right: are you suggesting that we
fix up housekeeping_isolcpus_setup() to be able to skip unknown sub
parameters?

Thanks,

-- 
Peter Xu


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4] sched/isolation: isolate from handling managed interrupt
  2020-02-03 23:57       ` Peter Xu
@ 2020-02-04  9:13         ` Thomas Gleixner
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Gleixner @ 2020-02-04  9:13 UTC (permalink / raw)
  To: Peter Xu
  Cc: Ming Lei, linux-kernel, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Luiz Capitulino

Peter,

Peter Xu <peterx@redhat.com> writes:
> On Mon, Feb 03, 2020 at 11:15:50PM +0000, Thomas Gleixner wrote:
>> No, really. The basic guarantee is that your new kernel is going to work
>> fine with the previous command line, but making a guarantee that new
>> command line options still work on an old kernel are just creating a
>> horrible mess. So if that command line interface was not designed to
>> handle unknown arguments in the first place, you better fix that.
>
> Hi, Thomas,
>
> Just to make sure I understand it right: are you suggesting that we
> fix up housekeeping_isolcpus_setup() to be able to skip unknown sub
> parameters?

Exactly.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-02-04  9:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20  9:16 [PATCH V4] sched/isolation: isolate from handling managed interrupt Ming Lei
2020-01-22 13:25 ` Thomas Gleixner
2020-02-03 19:21   ` Peter Xu
2020-02-03 23:15     ` Thomas Gleixner
2020-02-03 23:57       ` Peter Xu
2020-02-04  9:13         ` Thomas Gleixner
2020-01-22 15:34 ` [tip: irq/core] genirq, sched/isolation: Isolate from handling managed interrupts tip-bot2 for Ming Lei

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