All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
To: "pwaskiewicz@jumptrading.com" <pwaskiewicz@jumptrading.com>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>
Cc: "pjwaskiewicz@gmail.com" <pjwaskiewicz@gmail.com>,
	"Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
	"Fijalkowski, Maciej" <maciej.fijalkowski@intel.com>,
	"Dziedziuch, SylwesterX" <sylwesterx.dziedziuch@intel.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe()
Date: Mon, 30 Aug 2021 20:52:41 +0000	[thread overview]
Message-ID: <50c21a769633c8efa07f49fc8b20fdfb544cf3c5.camel@intel.com> (raw)
In-Reply-To: <20210826221916.127243-1-pwaskiewicz@jumptrading.com>

On Thu, 2021-08-26 at 17:19 -0500, PJ Waskiewicz wrote:
> This fixes an error path condition when probe() fails due to the
> default VSI not being available or online yet in the firmware. If
> that happens, the previous teardown path would clear the interrupt
> scheme, which also freed the IRQs with the OS. Then the error path
> for the switch setup (pre-VSI) would attempt to free the OS IRQs
> as well.

Hi PJ,

These comments are from the i40e team.

Yes in case we fail and go to err_vsis label in i40e_probe() we will
call i40e_reset_interrupt_capability twice but this is not a problem.
This is because pci_disable_msi/pci_disable_msix will be called only if
appropriate flags are set on PF and if this function is called ones it
will clear those flags. So even if we call
i40e_reset_interrupt_capability twice we will not disable msi vectors
twice.

The issue here is different however. It is failing in free_irq because
we are trying to free already free vector. This is because setup of
misc irq vectors in i40e_probe is done after i40e_setup_pf_switch. If
i40e_setup_pf_switch fails then we will jump to err_vsis and call
i40e_clear_interrupt_scheme which will try to free those misc irq
vectors which were not yet allocated. We should have the proper fix for
this ready soon.


WARNING: multiple messages have this Message-ID (diff)
From: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe()
Date: Mon, 30 Aug 2021 20:52:41 +0000	[thread overview]
Message-ID: <50c21a769633c8efa07f49fc8b20fdfb544cf3c5.camel@intel.com> (raw)
In-Reply-To: <20210826221916.127243-1-pwaskiewicz@jumptrading.com>

On Thu, 2021-08-26 at 17:19 -0500, PJ Waskiewicz wrote:
> This fixes an error path condition when probe() fails due to the
> default VSI not being available or online yet in the firmware. If
> that happens, the previous teardown path would clear the interrupt
> scheme, which also freed the IRQs with the OS. Then the error path
> for the switch setup (pre-VSI) would attempt to free the OS IRQs
> as well.

Hi PJ,

These comments are from the i40e team.

Yes in case we fail and go to err_vsis label in i40e_probe() we will
call i40e_reset_interrupt_capability twice but this is not a problem.
This is because pci_disable_msi/pci_disable_msix will be called only if
appropriate flags are set on PF and if this function is called ones it
will clear those flags. So even if we call
i40e_reset_interrupt_capability twice we will not disable msi vectors
twice.

The issue here is different however. It is failing in free_irq because
we are trying to free already free vector. This is because setup of
misc irq vectors in i40e_probe is done after i40e_setup_pf_switch. If
i40e_setup_pf_switch fails then we will jump to err_vsis and call
i40e_clear_interrupt_scheme which will try to free those misc irq
vectors which were not yet allocated. We should have the proper fix for
this ready soon.


  reply	other threads:[~2021-08-30 20:52 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 22:19 [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe() PJ Waskiewicz
2021-08-26 22:19 ` [Intel-wired-lan] " PJ Waskiewicz
2021-08-30 20:52 ` Nguyen, Anthony L [this message]
2021-08-30 20:52   ` Nguyen, Anthony L
2021-08-31 20:58   ` PJ Waskiewicz
2021-08-31 20:58     ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-13 19:37     ` PJ Waskiewicz
2021-09-13 19:37       ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-13 20:29       ` Nguyen, Anthony L
2021-09-13 20:29         ` [Intel-wired-lan] " Nguyen, Anthony L
2021-09-14  8:23         ` Dziedziuch, SylwesterX
2021-09-14  8:23           ` [Intel-wired-lan] " Dziedziuch, SylwesterX
2021-09-14 21:40           ` PJ Waskiewicz
2021-09-14 21:40             ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-15  9:53             ` Dziedziuch, SylwesterX
2021-09-15  9:53               ` [Intel-wired-lan] " Dziedziuch, SylwesterX
2021-09-18  2:01               ` PJ Waskiewicz
2021-09-18  2:01                 ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-20  7:48                 ` Dziedziuch, SylwesterX
2021-09-20  7:48                   ` [Intel-wired-lan] " Dziedziuch, SylwesterX
2021-09-21 17:06                   ` PJ Waskiewicz
2021-09-21 17:06                     ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-23 15:17                     ` PJ Waskiewicz
2021-09-23 15:17                       ` [Intel-wired-lan] " PJ Waskiewicz
2021-09-24  7:04                       ` Dziedziuch, SylwesterX
2021-09-24  7:04                         ` [Intel-wired-lan] " Dziedziuch, SylwesterX
  -- strict thread matches above, loose matches on Subject: below --
2021-08-25 19:23 PJ Waskiewicz
2021-08-26  8:03 ` Maciej Fijalkowski
2021-08-26 14:26   ` PJ Waskiewicz

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=50c21a769633c8efa07f49fc8b20fdfb544cf3c5.camel@intel.com \
    --to=anthony.l.nguyen@intel.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=davem@davemloft.net \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=maciej.fijalkowski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pjwaskiewicz@gmail.com \
    --cc=pwaskiewicz@jumptrading.com \
    --cc=sylwesterx.dziedziuch@intel.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.