All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Thu, 13 Aug 2020 10:08:46 +0200	[thread overview]
Message-ID: <877du355o1.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <b55d8b8c-8afc-0046-44b6-514ad012936f@huawei.com>

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

  reply	other threads:[~2020-08-13  8:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11  6:17 [PATCH] genirq/affinity: show managed irq affinity correctly Yunfeng Ye
2020-08-13  8:08 ` Thomas Gleixner [this message]
2020-08-15 14:29   ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877du355o1.fsf@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=hewenliang4@huawei.com \
    --cc=hushiyuan@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yeyunfeng@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.