linux-kernel - Re: [PATCH] genirq/affinity: show managed irq affinity correctly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Aug 2020 10:08:46 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Yunfeng Ye <yeyunfeng@...wei.com>
Cc:     Shiyuan Hu <hushiyuan@...wei.com>,
        Hewenliang <hewenliang4@...wei.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] genirq/affinity: show managed irq affinity correctly

Yunfeng Ye <yeyunfeng@...wei.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