RE: [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe()

-----Original Message-----
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Sent: Thursday, August 26, 2021 1:04 AM
To: PJ Waskiewicz <redacted>
Cc: David S . Miller <davem@davemloft.net>; Jesse Brandeburg
[off-list ref]; netdev@vger.kernel.org; intel-wired-
lan@lists.osuosl.org; PJ Waskiewicz [off-list ref]
Subject: Re: [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe()

This message has originated from an EXTERNAL SENDER

On Wed, Aug 25, 2021 at 02:23:21PM -0500, PJ Waskiewicz wrote:

quoted

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.

Nice to hear from you PJ.
Looks like a 'net' candidate, so we need a Fixes: tag.

Likewise, nice to be submitting patches again.

I'll respin and send to net.

Also, you probably should send it to iwl with cc to netdev and Tony will pick
and combine it into his pull request.

Roger that, I'll rearrange the To: and Cc: members.

Patch LGTM.
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Thanks.

[...snip...]

quoted

@@ -4865,7 +4865,8 @@ static void i40e_reset_interrupt_capability(struct

i40e_pf *pf)

quoted

  * @pf: board private structure
  *
  * We go through and clear interrupt specific resources and reset the
structure
- * to pre-load conditions
+ * to pre-load conditions.  OS interrupt teardown must be done
+ separately due
+ * to VSI vs. PF instantiation, and different teardown path requirements.

Small nit: no need for a dot in 'vs.' ?

Apparently this is different between British English and American English (no dot in the former).  Who would have thought...I'll update and drop the '.'.

quoted

  **/
 static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)  { @@
-4880,7 +4881,6 @@ static void i40e_clear_interrupt_scheme(struct

i40e_pf *pf)

quoted

        for (i = 0; i < pf->num_alloc_vsi; i++)
                if (pf->vsi[i])
                        i40e_vsi_free_q_vectors(pf->vsi[i]);
-       i40e_reset_interrupt_capability(pf);
 }

 /**

@@ -10526,6 +10526,7 @@ static void i40e_rebuild(struct i40e_pf *pf, bool

reinit, bool lock_acquired)

quoted

                         */
                        free_irq(pf->pdev->irq, pf);
                        i40e_clear_interrupt_scheme(pf);
+                       i40e_reset_interrupt_capability(pf);
                        if (i40e_restore_interrupt_scheme(pf))
                                goto end_unlock;
                }

@@ -15928,6 +15929,7 @@ static void i40e_remove(struct pci_dev *pdev)
        /* Clear all dynamic memory lists of rings, q_vectors, and VSIs */
        rtnl_lock();
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);
        for (i = 0; i < pf->num_alloc_vsi; i++) {
                if (pf->vsi[i]) {
                        if (!test_bit(__I40E_RECOVERY_MODE,

pf->state)) @@ -16150,6 +16152,7 @@ static void i40e_shutdown(struct

pci_dev *pdev)

quoted

         */
        rtnl_lock();
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);
        rtnl_unlock();

        if (system_state == SYSTEM_POWER_OFF) { @@ -16202,6 +16205,7

@@ static int __maybe_unused i40e_suspend(struct device *dev)
         * to CPU0.
         */
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);

        rtnl_unlock();

--

2.27.0

And probably talk to your IT dep to get rid of the footer :P

I know, that is something I've been working on.  Not sure I have much control over that unfortunately.

-PJ

quoted

________________________________

Note: This email is for the confidential use of the named addressee(s) only

and may contain proprietary, confidential, or privileged information and/or
personal data. If you are not the intended recipient, you are hereby notified
that any review, dissemination, or copying of this email is strictly prohibited,
and requested to notify the sender immediately and destroy this email and
any attachments. Email transmission cannot be guaranteed to be secure or
error-free. The Company, therefore, does not make any guarantees as to
the completeness or accuracy of this email or any attachments. This email is
for informational purposes only and does not constitute a recommendation,
offer, request, or solicitation of any kind to buy, sell, subscribe, redeem, or
perform any type of transaction of a financial product. Personal data, as
defined by applicable data protection and privacy laws, contained in this
email may be processed by the Company, and any of its affiliated or related
companies, for legal, compliance, and/or business-related purposes. You
may have rights regarding your personal data; for information on exercising
these rights or the Company’s treatment of personal data, please email
datarequests@jumptrading.com.

________________________________

Note: This email is for the confidential use of the named addressee(s) only and may contain proprietary, confidential, or privileged information and/or personal data. If you are not the intended recipient, you are hereby notified that any review, dissemination, or copying of this email is strictly prohibited, and requested to notify the sender immediately and destroy this email and any attachments. Email transmission cannot be guaranteed to be secure or error-free. The Company, therefore, does not make any guarantees as to the completeness or accuracy of this email or any attachments. This email is for informational purposes only and does not constitute a recommendation, offer, request, or solicitation of any kind to buy, sell, subscribe, redeem, or perform any type of transaction of a financial product. Personal data, as defined by applicable data protection and privacy laws, contained in this email may be processed by the Company, and any of its affiliated or related companies, for legal, compliance, and/or business-related purposes. You may have rights regarding your personal data; for information on exercising these rights or the Company’s treatment of personal data, please email datarequests@jumptrading.com.