Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Yunfeng Ye <yeyunfeng@huawei.com>
Cc:     Shiyuan Hu <hushiyuan@huawei.com>,
        Hewenliang <hewenliang4@huawei.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] genirq/affinity: show managed irq affinity correctly
In-Reply-To: <b55d8b8c-8afc-0046-44b6-514ad012936f@huawei.com>
References: <b55d8b8c-8afc-0046-44b6-514ad012936f@huawei.com>
Date:   Thu, 13 Aug 2020 10:08:46 +0200
Message-ID: <877du355o1.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Yunfeng Ye <yeyunfeng@huawei.com> writes:

> The "managed_irq" for isolcpus is supported after the commit
> 11ea68f553e2 ("genirq, sched/isolation: Isolate from handling managed
> interrupts"), but the interrupt affinity shown in proc directory is
> still the original affinity.
>
> So modify the interrupt affinity correctly for managed_irq.

I really have no idea what you are trying to achieve here.

1) Why are you moving the !chip !chip->irq_set_affinity check out of
   irq_do_set_affinity() ?

   Just that the whole computation happens for nothing and then returns
   an error late.

2) Modifying irqdata->common->affinity is wrong to begin with. It's the
   possible affinity mask. Your change causes the managed affinity mask
   to become invalid in the worst case.

      irq->affinity = 0x0C;        // CPU 2 - 3
      hkmask   = 0x07;             // CPU 0 - 2

   Invocation #1:
      online_mask = 0xFF;          // CPU 0 - 7

      cpumask_and(&tmp_mask, mask, hk_mask);
         -->   tmp_mask == 0x04    // CPU 2

      irq->affinity = tmp_mask;	   // CPU 2

   CPU 2 goes offline

   migrate_one_irq()

      affinity = irq->affinity;	  // CPU 2
      online_mask = 0xFB;         // CPU 0-1, 3-7

      if (cpumask_any_and(affinity, cpu_online_mask) >= nr_cpu_ids) {
		/*
		 * If the interrupt is managed, then shut it down and leave
		 * the affinity untouched.
		 */
		if (irqd_affinity_is_managed(d)) {
			irqd_set_managed_shutdown(d);
			irq_shutdown_and_deactivate(desc);
			return false;
		}

  So the interrupt is shut down which is incorrect. The isolation
  logic in irq_do_set_affinity() was clearly designed to prefer
  housekeeping CPUs and not to remove them.
    
You are looking at the wrong file. /proc/irq/$IRQ/smp_affinity* is the
possible mask. If you want to know to which CPU an interrupt is affine
then look at /proc/irq/$IRQ/effective_affinity*

If effective_affinity* is not showing the correct value, then the irq
chip affinity setter is broken and needs to be fixed.

Thanks,

        tglx