All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] cxl: Force context lock during EEH flow
@ 2017-04-05 11:35 Vaibhav Jain
  2017-04-06  8:32   ` Frederic Barrat
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Vaibhav Jain @ 2017-04-05 11:35 UTC (permalink / raw)
  To: Frederic Barrat, linuxppc-dev
  Cc: Vaibhav Jain, Uma Krishnan, Andrew Donnellan, Ian Munsie,
	Christophe Lombard, Philippe Bergheaud, Greg Kurz, stable

During an eeh event when the cxl card is fenced and card sysfs attr
perst_reloads_same_image is set following warning message is seen in the
kernel logs:

 [   60.622727] Adapter context unlocked with 0 active contexts
 [   60.622762] ------------[ cut here ]------------
 [   60.622771] WARNING: CPU: 12 PID: 627 at
 ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]

Even though this warning is harmless, it clutters the kernel log
during an eeh event. This warning is triggered as the EEH callback
cxl_pci_error_detected doesn't obtain a context-lock before forcibly
detaching all active context and when context-lock is released during
call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
cxl_adapter_context_unlock is triggered.

To fix this warning, we acquire the adapter context-lock via
cxl_adapter_context_lock() in the eeh callback
cxl_pci_error_detected() once all the virtual AFU PHBs are notified
and their contexts detached. The context-lock is released in
cxl_pci_slot_reset() after the adapter is successfully reconfigured
and before we call slot_reset callback on slice attached device-drivers.

Cc: stable@vger.kernel.org
Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
---
Change-Log:

v3..v4
- Moved the call to context-unlock from cxl_pci_resume to
cxl_pci_slot_reset to let cxlflash module activate its master context
during slot reset. (Fred)

v2..v3
- As discussed with Fred removed function
cxl_adapter_context_force_lock() which may potentially expose the code
to deadlock in the future.
-  Other details of changes in cxl_pci_error_detected() to fix an
earlier issue of eeh callbacks not being passed on to all slices, is
being reworked as a separate patch.

v2..v1
- Moved the call to cxl_adapter_context_force_lock() from
cxl_pci_error_detected() to cxl_remove. (Fred)
---
 drivers/misc/cxl/pci.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index b27ea98..dd9a128 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1496,8 +1496,6 @@ static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
 	if ((rc = cxl_native_register_psl_err_irq(adapter)))
 		goto err;
 
-	/* Release the context lock as adapter is configured */
-	cxl_adapter_context_unlock(adapter);
 	return 0;
 
 err:
@@ -1596,6 +1594,9 @@ static struct cxl *cxl_pci_init_adapter(struct pci_dev *dev)
 	if ((rc = cxl_sysfs_adapter_add(adapter)))
 		goto err_put1;
 
+	/* Release the context lock as adapter is configured */
+	cxl_adapter_context_unlock(adapter);
+
 	return adapter;
 
 err_put1:
@@ -1895,6 +1896,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
 		pci_deconfigure_afu(afu);
 	}
+
+	/* should take the context lock here */
+	if (cxl_adapter_context_lock(adapter) != 0)
+		dev_warn(&adapter->dev,
+			 "Couldn't take context lock with %d active-contexts\n",
+			 atomic_read(&adapter->contexts_num));
+
 	cxl_deconfigure_adapter(adapter);
 
 	return result;
@@ -1913,6 +1921,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
 	if (cxl_configure_adapter(adapter, pdev))
 		goto err;
 
+	/*
+	 * Unlock context activation for the adapter. Ideally this should be
+	 * done in cxl_pci_resume but cxlflash module tries to activate the
+	 * master context as part of slot_reset callback.
+	 */
+	cxl_adapter_context_unlock(adapter);
+
 	for (i = 0; i < adapter->slices; i++) {
 		afu = adapter->afu[i];
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-05 11:35 [PATCH v4] cxl: Force context lock during EEH flow Vaibhav Jain
@ 2017-04-06  8:32   ` Frederic Barrat
  2017-04-17 15:02   ` Matthew R. Ochs
  2017-04-17 16:52 ` Uma Krishnan
  2 siblings, 0 replies; 11+ messages in thread
From: Frederic Barrat @ 2017-04-06  8:32 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, stable, Uma Krishnan, Ian Munsie,
	Andrew Donnellan, Christophe Lombard, Greg Kurz



Le 05/04/2017 � 13:35, Vaibhav Jain a �crit :
> During an eeh event when the cxl card is fenced and card sysfs attr
> perst_reloads_same_image is set following warning message is seen in the
> kernel logs:
>
>  [   60.622727] Adapter context unlocked with 0 active contexts
>  [   60.622762] ------------[ cut here ]------------
>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>
> Even though this warning is harmless, it clutters the kernel log
> during an eeh event. This warning is triggered as the EEH callback
> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
> detaching all active context and when context-lock is released during
> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
> cxl_adapter_context_unlock is triggered.
>
> To fix this warning, we acquire the adapter context-lock via
> cxl_adapter_context_lock() in the eeh callback
> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
> and their contexts detached. The context-lock is released in
> cxl_pci_slot_reset() after the adapter is successfully reconfigured
> and before we call slot_reset callback on slice attached device-drivers.
>
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---

Pending test result from cxl-flash:
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>



> Change-Log:
>
> v3..v4
> - Moved the call to context-unlock from cxl_pci_resume to
> cxl_pci_slot_reset to let cxlflash module activate its master context
> during slot reset. (Fred)
>
> v2..v3
> - As discussed with Fred removed function
> cxl_adapter_context_force_lock() which may potentially expose the code
> to deadlock in the future.
> -  Other details of changes in cxl_pci_error_detected() to fix an
> earlier issue of eeh callbacks not being passed on to all slices, is
> being reworked as a separate patch.
>
> v2..v1
> - Moved the call to cxl_adapter_context_force_lock() from
> cxl_pci_error_detected() to cxl_remove. (Fred)
> ---
>  drivers/misc/cxl/pci.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index b27ea98..dd9a128 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1496,8 +1496,6 @@ static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
>  	if ((rc = cxl_native_register_psl_err_irq(adapter)))
>  		goto err;
>
> -	/* Release the context lock as adapter is configured */
> -	cxl_adapter_context_unlock(adapter);
>  	return 0;
>
>  err:
> @@ -1596,6 +1594,9 @@ static struct cxl *cxl_pci_init_adapter(struct pci_dev *dev)
>  	if ((rc = cxl_sysfs_adapter_add(adapter)))
>  		goto err_put1;
>
> +	/* Release the context lock as adapter is configured */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	return adapter;
>
>  err_put1:
> @@ -1895,6 +1896,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>  		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>  		pci_deconfigure_afu(afu);
>  	}
> +
> +	/* should take the context lock here */
> +	if (cxl_adapter_context_lock(adapter) != 0)
> +		dev_warn(&adapter->dev,
> +			 "Couldn't take context lock with %d active-contexts\n",
> +			 atomic_read(&adapter->contexts_num));
> +
>  	cxl_deconfigure_adapter(adapter);
>
>  	return result;
> @@ -1913,6 +1921,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>  	if (cxl_configure_adapter(adapter, pdev))
>  		goto err;
>
> +	/*
> +	 * Unlock context activation for the adapter. Ideally this should be
> +	 * done in cxl_pci_resume but cxlflash module tries to activate the
> +	 * master context as part of slot_reset callback.
> +	 */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	for (i = 0; i < adapter->slices; i++) {
>  		afu = adapter->afu[i];
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
@ 2017-04-06  8:32   ` Frederic Barrat
  0 siblings, 0 replies; 11+ messages in thread
From: Frederic Barrat @ 2017-04-06  8:32 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, stable, Uma Krishnan, Ian Munsie,
	Andrew Donnellan, Christophe Lombard, Greg Kurz



Le 05/04/2017 à 13:35, Vaibhav Jain a écrit :
> During an eeh event when the cxl card is fenced and card sysfs attr
> perst_reloads_same_image is set following warning message is seen in the
> kernel logs:
>
>  [   60.622727] Adapter context unlocked with 0 active contexts
>  [   60.622762] ------------[ cut here ]------------
>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>
> Even though this warning is harmless, it clutters the kernel log
> during an eeh event. This warning is triggered as the EEH callback
> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
> detaching all active context and when context-lock is released during
> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
> cxl_adapter_context_unlock is triggered.
>
> To fix this warning, we acquire the adapter context-lock via
> cxl_adapter_context_lock() in the eeh callback
> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
> and their contexts detached. The context-lock is released in
> cxl_pci_slot_reset() after the adapter is successfully reconfigured
> and before we call slot_reset callback on slice attached device-drivers.
>
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---

Pending test result from cxl-flash:
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>



> Change-Log:
>
> v3..v4
> - Moved the call to context-unlock from cxl_pci_resume to
> cxl_pci_slot_reset to let cxlflash module activate its master context
> during slot reset. (Fred)
>
> v2..v3
> - As discussed with Fred removed function
> cxl_adapter_context_force_lock() which may potentially expose the code
> to deadlock in the future.
> -  Other details of changes in cxl_pci_error_detected() to fix an
> earlier issue of eeh callbacks not being passed on to all slices, is
> being reworked as a separate patch.
>
> v2..v1
> - Moved the call to cxl_adapter_context_force_lock() from
> cxl_pci_error_detected() to cxl_remove. (Fred)
> ---
>  drivers/misc/cxl/pci.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index b27ea98..dd9a128 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1496,8 +1496,6 @@ static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
>  	if ((rc = cxl_native_register_psl_err_irq(adapter)))
>  		goto err;
>
> -	/* Release the context lock as adapter is configured */
> -	cxl_adapter_context_unlock(adapter);
>  	return 0;
>
>  err:
> @@ -1596,6 +1594,9 @@ static struct cxl *cxl_pci_init_adapter(struct pci_dev *dev)
>  	if ((rc = cxl_sysfs_adapter_add(adapter)))
>  		goto err_put1;
>
> +	/* Release the context lock as adapter is configured */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	return adapter;
>
>  err_put1:
> @@ -1895,6 +1896,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>  		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>  		pci_deconfigure_afu(afu);
>  	}
> +
> +	/* should take the context lock here */
> +	if (cxl_adapter_context_lock(adapter) != 0)
> +		dev_warn(&adapter->dev,
> +			 "Couldn't take context lock with %d active-contexts\n",
> +			 atomic_read(&adapter->contexts_num));
> +
>  	cxl_deconfigure_adapter(adapter);
>
>  	return result;
> @@ -1913,6 +1921,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>  	if (cxl_configure_adapter(adapter, pdev))
>  		goto err;
>
> +	/*
> +	 * Unlock context activation for the adapter. Ideally this should be
> +	 * done in cxl_pci_resume but cxlflash module tries to activate the
> +	 * master context as part of slot_reset callback.
> +	 */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	for (i = 0; i < adapter->slices; i++) {
>  		afu = adapter->afu[i];
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-06  8:32   ` Frederic Barrat
@ 2017-04-11 10:40     ` Michael Ellerman
  -1 siblings, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2017-04-11 10:40 UTC (permalink / raw)
  To: Frederic Barrat, Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, stable, Uma Krishnan, Ian Munsie,
	Andrew Donnellan, Christophe Lombard, Greg Kurz

Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:

> Le 05/04/2017 à 13:35, Vaibhav Jain a écrit :
>> During an eeh event when the cxl card is fenced and card sysfs attr
>> perst_reloads_same_image is set following warning message is seen in the
>> kernel logs:
>>
>>  [   60.622727] Adapter context unlocked with 0 active contexts
>>  [   60.622762] ------------[ cut here ]------------
>>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>>
>> Even though this warning is harmless, it clutters the kernel log
>> during an eeh event. This warning is triggered as the EEH callback
>> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
>> detaching all active context and when context-lock is released during
>> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
>> cxl_adapter_context_unlock is triggered.
>>
>> To fix this warning, we acquire the adapter context-lock via
>> cxl_adapter_context_lock() in the eeh callback
>> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
>> and their contexts detached. The context-lock is released in
>> cxl_pci_slot_reset() after the adapter is successfully reconfigured
>> and before we call slot_reset callback on slice attached device-drivers.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
>> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>> ---
>
> Pending test result from cxl-flash:
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Still pending ... ?

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
@ 2017-04-11 10:40     ` Michael Ellerman
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2017-04-11 10:40 UTC (permalink / raw)
  To: Frederic Barrat, Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, stable, Uma Krishnan, Ian Munsie,
	Andrew Donnellan, Christophe Lombard, Greg Kurz

Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:

> Le 05/04/2017 =C3=A0 13:35, Vaibhav Jain a =C3=A9crit :
>> During an eeh event when the cxl card is fenced and card sysfs attr
>> perst_reloads_same_image is set following warning message is seen in the
>> kernel logs:
>>
>>  [   60.622727] Adapter context unlocked with 0 active contexts
>>  [   60.622762] ------------[ cut here ]------------
>>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cx=
l]
>>
>> Even though this warning is harmless, it clutters the kernel log
>> during an eeh event. This warning is triggered as the EEH callback
>> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
>> detaching all active context and when context-lock is released during
>> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
>> cxl_adapter_context_unlock is triggered.
>>
>> To fix this warning, we acquire the adapter context-lock via
>> cxl_adapter_context_lock() in the eeh callback
>> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
>> and their contexts detached. The context-lock is released in
>> cxl_pci_slot_reset() after the adapter is successfully reconfigured
>> and before we call slot_reset callback on slice attached device-drivers.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exi=
sts")
>> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>> ---
>
> Pending test result from cxl-flash:
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Still pending ... ?

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-11 10:40     ` Michael Ellerman
  (?)
@ 2017-04-11 11:13     ` Frederic Barrat
  2017-04-13 22:24         ` Matthew R. Ochs
  -1 siblings, 1 reply; 11+ messages in thread
From: Frederic Barrat @ 2017-04-11 11:13 UTC (permalink / raw)
  To: Michael Ellerman, Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, stable, Uma Krishnan, Ian Munsie,
	Andrew Donnellan, Christophe Lombard, Greg Kurz



Le 11/04/2017 à 12:40, Michael Ellerman a écrit :
> Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
>
>> Le 05/04/2017 à 13:35, Vaibhav Jain a écrit :
>>> During an eeh event when the cxl card is fenced and card sysfs attr
>>> perst_reloads_same_image is set following warning message is seen in the
>>> kernel logs:
>>>
>>>  [   60.622727] Adapter context unlocked with 0 active contexts
>>>  [   60.622762] ------------[ cut here ]------------
>>>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>>>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>>>
>>> Even though this warning is harmless, it clutters the kernel log
>>> during an eeh event. This warning is triggered as the EEH callback
>>> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
>>> detaching all active context and when context-lock is released during
>>> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
>>> cxl_adapter_context_unlock is triggered.
>>>
>>> To fix this warning, we acquire the adapter context-lock via
>>> cxl_adapter_context_lock() in the eeh callback
>>> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
>>> and their contexts detached. The context-lock is released in
>>> cxl_pci_slot_reset() after the adapter is successfully reconfigured
>>> and before we call slot_reset callback on slice attached device-drivers.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
>>> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>>> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>>> ---
>>
>> Pending test result from cxl-flash:
>> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>
> Still pending ... ?

Yes, still waiting. It was mentioned in a call with the cxlflash team 
yesterday.

   Fred



>
> cheers
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-11 11:13     ` Frederic Barrat
@ 2017-04-13 22:24         ` Matthew R. Ochs
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2017-04-13 22:24 UTC (permalink / raw)
  To: Frederic Barrat
  Cc: Michael Ellerman, Vaibhav Jain, linuxppc-dev, Philippe Bergheaud,
	stable, Uma Krishnan, Ian Munsie, Andrew Donnellan,
	Christophe Lombard, Greg Kurz

> On Apr 11, 2017, at 6:13 AM, Frederic Barrat <fbarrat@linux.vnet.ibm.com> wrote:
> Le 11/04/2017 à 12:40, Michael Ellerman a écrit :
>> Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
>> 
>>> Le 05/04/2017 à 13:35, Vaibhav Jain a écrit :
>>>> During an eeh event when the cxl card is fenced and card sysfs attr
>>>> perst_reloads_same_image is set following warning message is seen in the
>>>> kernel logs:
>>>> 
>>>> [   60.622727] Adapter context unlocked with 0 active contexts
>>>> [   60.622762] ------------[ cut here ]------------
>>>> [   60.622771] WARNING: CPU: 12 PID: 627 at
>>>> ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>>>> 
>>>> Even though this warning is harmless, it clutters the kernel log
>>>> during an eeh event. This warning is triggered as the EEH callback
>>>> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
>>>> detaching all active context and when context-lock is released during
>>>> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
>>>> cxl_adapter_context_unlock is triggered.
>>>> 
>>>> To fix this warning, we acquire the adapter context-lock via
>>>> cxl_adapter_context_lock() in the eeh callback
>>>> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
>>>> and their contexts detached. The context-lock is released in
>>>> cxl_pci_slot_reset() after the adapter is successfully reconfigured
>>>> and before we call slot_reset callback on slice attached device-drivers.
>>>> 
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
>>>> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>>>> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>>>> ---
>>> 
>>> Pending test result from cxl-flash:
>>> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>> 
>> Still pending ... ?
> 
> Yes, still waiting. It was mentioned in a call with the cxlflash team yesterday.

I have it queued up for us to do on Monday.

-matt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
@ 2017-04-13 22:24         ` Matthew R. Ochs
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2017-04-13 22:24 UTC (permalink / raw)
  To: Frederic Barrat
  Cc: Michael Ellerman, Vaibhav Jain, linuxppc-dev, Philippe Bergheaud,
	stable, Uma Krishnan, Ian Munsie, Andrew Donnellan,
	Christophe Lombard, Greg Kurz

> On Apr 11, 2017, at 6:13 AM, Frederic Barrat =
<fbarrat@linux.vnet.ibm.com> wrote:
> Le 11/04/2017 =C3=A0 12:40, Michael Ellerman a =C3=A9crit :
>> Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
>>=20
>>> Le 05/04/2017 =C3=A0 13:35, Vaibhav Jain a =C3=A9crit :
>>>> During an eeh event when the cxl card is fenced and card sysfs attr
>>>> perst_reloads_same_image is set following warning message is seen =
in the
>>>> kernel logs:
>>>>=20
>>>> [   60.622727] Adapter context unlocked with 0 active contexts
>>>> [   60.622762] ------------[ cut here ]------------
>>>> [   60.622771] WARNING: CPU: 12 PID: 627 at
>>>> ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 =
[cxl]
>>>>=20
>>>> Even though this warning is harmless, it clutters the kernel log
>>>> during an eeh event. This warning is triggered as the EEH callback
>>>> cxl_pci_error_detected doesn't obtain a context-lock before =
forcibly
>>>> detaching all active context and when context-lock is released =
during
>>>> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
>>>> cxl_adapter_context_unlock is triggered.
>>>>=20
>>>> To fix this warning, we acquire the adapter context-lock via
>>>> cxl_adapter_context_lock() in the eeh callback
>>>> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
>>>> and their contexts detached. The context-lock is released in
>>>> cxl_pci_slot_reset() after the adapter is successfully reconfigured
>>>> and before we call slot_reset callback on slice attached =
device-drivers.
>>>>=20
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active =
context exists")
>>>> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>>>> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>>>> ---
>>>=20
>>> Pending test result from cxl-flash:
>>> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>>=20
>> Still pending ... ?
>=20
> Yes, still waiting. It was mentioned in a call with the cxlflash team =
yesterday.

I have it queued up for us to do on Monday.

-matt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-05 11:35 [PATCH v4] cxl: Force context lock during EEH flow Vaibhav Jain
@ 2017-04-17 15:02   ` Matthew R. Ochs
  2017-04-17 15:02   ` Matthew R. Ochs
  2017-04-17 16:52 ` Uma Krishnan
  2 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2017-04-17 15:02 UTC (permalink / raw)
  To: Vaibhav Jain
  Cc: Frederic Barrat, linuxppc-dev, Philippe Bergheaud, stable,
	Uma Krishnan, Ian Munsie, Andrew Donnellan, Christophe Lombard,
	Greg Kurz

> On Apr 5, 2017, at 6:35 AM, Vaibhav Jain <vaibhav@linux.vnet.ibm.com> wrote:
> 
> During an eeh event when the cxl card is fenced and card sysfs attr
> perst_reloads_same_image is set following warning message is seen in the
> kernel logs:
> 
> [   60.622727] Adapter context unlocked with 0 active contexts
> [   60.622762] ------------[ cut here ]------------
> [   60.622771] WARNING: CPU: 12 PID: 627 at
> ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
> 
> Even though this warning is harmless, it clutters the kernel log
> during an eeh event. This warning is triggered as the EEH callback
> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
> detaching all active context and when context-lock is released during
> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
> cxl_adapter_context_unlock is triggered.
> 
> To fix this warning, we acquire the adapter context-lock via
> cxl_adapter_context_lock() in the eeh callback
> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
> and their contexts detached. The context-lock is released in
> cxl_pci_slot_reset() after the adapter is successfully reconfigured
> and before we call slot_reset callback on slice attached device-drivers.
> 
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>

Looks good. Uma should be following up later today after testing completes.

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
@ 2017-04-17 15:02   ` Matthew R. Ochs
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2017-04-17 15:02 UTC (permalink / raw)
  To: Vaibhav Jain
  Cc: Frederic Barrat, linuxppc-dev, Philippe Bergheaud, stable,
	Uma Krishnan, Ian Munsie, Andrew Donnellan, Christophe Lombard,
	Greg Kurz

> On Apr 5, 2017, at 6:35 AM, Vaibhav Jain <vaibhav@linux.vnet.ibm.com> =
wrote:
>=20
> During an eeh event when the cxl card is fenced and card sysfs attr
> perst_reloads_same_image is set following warning message is seen in =
the
> kernel logs:
>=20
> [   60.622727] Adapter context unlocked with 0 active contexts
> [   60.622762] ------------[ cut here ]------------
> [   60.622771] WARNING: CPU: 12 PID: 627 at
> ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 =
[cxl]
>=20
> Even though this warning is harmless, it clutters the kernel log
> during an eeh event. This warning is triggered as the EEH callback
> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
> detaching all active context and when context-lock is released during
> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
> cxl_adapter_context_unlock is triggered.
>=20
> To fix this warning, we acquire the adapter context-lock via
> cxl_adapter_context_lock() in the eeh callback
> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
> and their contexts detached. The context-lock is released in
> cxl_pci_slot_reset() after the adapter is successfully reconfigured
> and before we call slot_reset callback on slice attached =
device-drivers.
>=20
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context =
exists")
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>

Looks good. Uma should be following up later today after testing =
completes.

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] cxl: Force context lock during EEH flow
  2017-04-05 11:35 [PATCH v4] cxl: Force context lock during EEH flow Vaibhav Jain
  2017-04-06  8:32   ` Frederic Barrat
  2017-04-17 15:02   ` Matthew R. Ochs
@ 2017-04-17 16:52 ` Uma Krishnan
  2 siblings, 0 replies; 11+ messages in thread
From: Uma Krishnan @ 2017-04-17 16:52 UTC (permalink / raw)
  To: Vaibhav Jain, Frederic Barrat, linuxppc-dev
  Cc: Andrew Donnellan, Ian Munsie, Christophe Lombard,
	Philippe Bergheaud, Greg Kurz, stable

On 4/5/2017 6:35 AM, Vaibhav Jain wrote:
> During an eeh event when the cxl card is fenced and card sysfs attr
> perst_reloads_same_image is set following warning message is seen in the
> kernel logs:
>
>  [   60.622727] Adapter context unlocked with 0 active contexts
>  [   60.622762] ------------[ cut here ]------------
>  [   60.622771] WARNING: CPU: 12 PID: 627 at
>  ../drivers/misc/cxl/main.c:325 cxl_adapter_context_unlock+0x60/0x80 [cxl]
>
> Even though this warning is harmless, it clutters the kernel log
> during an eeh event. This warning is triggered as the EEH callback
> cxl_pci_error_detected doesn't obtain a context-lock before forcibly
> detaching all active context and when context-lock is released during
> call to cxl_configure_adapter from cxl_pci_slot_reset, a warning in
> cxl_adapter_context_unlock is triggered.
>
> To fix this warning, we acquire the adapter context-lock via
> cxl_adapter_context_lock() in the eeh callback
> cxl_pci_error_detected() once all the virtual AFU PHBs are notified
> and their contexts detached. The context-lock is released in
> cxl_pci_slot_reset() after the adapter is successfully reconfigured
> and before we call slot_reset callback on slice attached device-drivers.
>
> Cc: stable@vger.kernel.org
> Fixes: 70b565bbdb91("cxl: Prevent adapter reset if an active context exists")
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> Change-Log:
>
> v3..v4
> - Moved the call to context-unlock from cxl_pci_resume to
> cxl_pci_slot_reset to let cxlflash module activate its master context
> during slot reset. (Fred)
>
> v2..v3
> - As discussed with Fred removed function
> cxl_adapter_context_force_lock() which may potentially expose the code
> to deadlock in the future.
> -  Other details of changes in cxl_pci_error_detected() to fix an
> earlier issue of eeh callbacks not being passed on to all slices, is
> being reworked as a separate patch.
>
> v2..v1
> - Moved the call to cxl_adapter_context_force_lock() from
> cxl_pci_error_detected() to cxl_remove. (Fred)
> ---
>  drivers/misc/cxl/pci.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index b27ea98..dd9a128 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1496,8 +1496,6 @@ static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
>  	if ((rc = cxl_native_register_psl_err_irq(adapter)))
>  		goto err;
>
> -	/* Release the context lock as adapter is configured */
> -	cxl_adapter_context_unlock(adapter);
>  	return 0;
>
>  err:
> @@ -1596,6 +1594,9 @@ static struct cxl *cxl_pci_init_adapter(struct pci_dev *dev)
>  	if ((rc = cxl_sysfs_adapter_add(adapter)))
>  		goto err_put1;
>
> +	/* Release the context lock as adapter is configured */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	return adapter;
>
>  err_put1:
> @@ -1895,6 +1896,13 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>  		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>  		pci_deconfigure_afu(afu);
>  	}
> +
> +	/* should take the context lock here */
> +	if (cxl_adapter_context_lock(adapter) != 0)
> +		dev_warn(&adapter->dev,
> +			 "Couldn't take context lock with %d active-contexts\n",
> +			 atomic_read(&adapter->contexts_num));
> +
>  	cxl_deconfigure_adapter(adapter);
>
>  	return result;
> @@ -1913,6 +1921,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>  	if (cxl_configure_adapter(adapter, pdev))
>  		goto err;
>
> +	/*
> +	 * Unlock context activation for the adapter. Ideally this should be
> +	 * done in cxl_pci_resume but cxlflash module tries to activate the
> +	 * master context as part of slot_reset callback.
> +	 */
> +	cxl_adapter_context_unlock(adapter);
> +
>  	for (i = 0; i < adapter->slices; i++) {
>  		afu = adapter->afu[i];
>

Looks good. FV regressions and EEH recoveries (in parallel to I/O) were 
successful.

Tested-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-04-17 16:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05 11:35 [PATCH v4] cxl: Force context lock during EEH flow Vaibhav Jain
2017-04-06  8:32 ` Frederic Barrat
2017-04-06  8:32   ` Frederic Barrat
2017-04-11 10:40   ` Michael Ellerman
2017-04-11 10:40     ` Michael Ellerman
2017-04-11 11:13     ` Frederic Barrat
2017-04-13 22:24       ` Matthew R. Ochs
2017-04-13 22:24         ` Matthew R. Ochs
2017-04-17 15:02 ` Matthew R. Ochs
2017-04-17 15:02   ` Matthew R. Ochs
2017-04-17 16:52 ` Uma Krishnan

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.