linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] drivers: xen frontend devices should handle missed backend CLOSING
@ 2012-10-18 10:02 David Vrabel
  2012-10-18 10:03 ` [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING David Vrabel
  0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2012-10-18 10:02 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, David S. Miller, Jens Axboe, linux-pci,
	Bjorn Helgaas, linux-fbdev, Florian Tobias Schandinat,
	linux-input, Dmitry Torokhov
  Cc: David Vrabel

Subsystem maintainers, you can either pick up the relevant driver patch
or ack it to go via Konrad's Xen tree.

The series makes all the Xen frontend drivers handle the backend
transitioning to CLOSED without the frontend having previously seen
the backend in the CLOSING state.

Backends shouldn't do this but some do.  e.g., if the host is
XenServer and the toolstack decides to do a forced shutdown of a VBD,
then the blkfront may miss the CLOSING transition and the /dev/xvdX
device will not be destroyed which prevents it being reused.

I have seen systems that ended up in this state but it's not clear if
this was the actual cause.  However, I think in general it's a good
thing to thing to improve the handling of unexpected state
transitions.

David

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

* [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING
  2012-10-18 10:02 [PATCH 0/5] drivers: xen frontend devices should handle missed backend CLOSING David Vrabel
@ 2012-10-18 10:03 ` David Vrabel
  2012-10-19 12:59   ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2012-10-18 10:03 UTC (permalink / raw)
  To: xen-devel
  Cc: David Vrabel, Konrad Rzeszutek Wilk, linux-kernel, linux-pci,
	Bjorn Helgaas

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: linux-pci@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/xen-pcifront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 0aab85a..a0c7312 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -1068,13 +1068,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
 	case XenbusStateInitialising:
 	case XenbusStateInitWait:
 	case XenbusStateInitialised:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateConnected:
 		pcifront_try_connect(pdev);
 		break;
 
+	case XenbusStateClosed:
+		if (xdev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		dev_warn(&xdev->dev, "backend going away!\n");
 		pcifront_try_disconnect(pdev);
-- 
1.7.2.5


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

* Re: [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING
  2012-10-18 10:03 ` [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING David Vrabel
@ 2012-10-19 12:59   ` Konrad Rzeszutek Wilk
  2012-11-30 18:41     ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-10-19 12:59 UTC (permalink / raw)
  To: David Vrabel, bhelgaas; +Cc: xen-devel, linux-kernel, linux-pci, Bjorn Helgaas

On Thu, Oct 18, 2012 at 11:03:36AM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> CLOSED.  If a backend does transition to CLOSED too soon then the
> frontend may not see the CLOSING state and will not properly shutdown.
> 
> So, treat an unexpected backend CLOSED state the same as CLOSING.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Cc: linux-pci@vger.kernel.org
> Cc: Bjorn Helgaas <bhelgaas@google.com>

Bjorn, do you want me to prep a git pull with this patch
or can I have your Ack to put it my tree and have it part of my
git pull to Linus?

Thx.
> ---
>  drivers/pci/xen-pcifront.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 0aab85a..a0c7312 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -1068,13 +1068,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
>  	case XenbusStateInitialising:
>  	case XenbusStateInitWait:
>  	case XenbusStateInitialised:
> -	case XenbusStateClosed:
>  		break;
>  
>  	case XenbusStateConnected:
>  		pcifront_try_connect(pdev);
>  		break;
>  
> +	case XenbusStateClosed:
> +		if (xdev->state == XenbusStateClosed)
> +			break;
> +		/* Missed the backend's CLOSING state -- fallthrough */
>  	case XenbusStateClosing:
>  		dev_warn(&xdev->dev, "backend going away!\n");
>  		pcifront_try_disconnect(pdev);
> -- 
> 1.7.2.5

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

* Re: [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING
  2012-10-19 12:59   ` Konrad Rzeszutek Wilk
@ 2012-11-30 18:41     ` Bjorn Helgaas
  2012-11-30 21:42       ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2012-11-30 18:41 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: David Vrabel, xen-devel, linux-kernel, linux-pci

On Fri, Oct 19, 2012 at 6:59 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Thu, Oct 18, 2012 at 11:03:36AM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>> CLOSED.  If a backend does transition to CLOSED too soon then the
>> frontend may not see the CLOSING state and will not properly shutdown.
>>
>> So, treat an unexpected backend CLOSED state the same as CLOSING.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>> Cc: linux-pci@vger.kernel.org
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>
> Bjorn, do you want me to prep a git pull with this patch
> or can I have your Ack to put it my tree and have it part of my
> git pull to Linus?

Sorry, I missed this.  I can put it in my -next branch for the v3.8
merge window.  Would that work for you?

>> ---
>>  drivers/pci/xen-pcifront.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
>> index 0aab85a..a0c7312 100644
>> --- a/drivers/pci/xen-pcifront.c
>> +++ b/drivers/pci/xen-pcifront.c
>> @@ -1068,13 +1068,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
>>       case XenbusStateInitialising:
>>       case XenbusStateInitWait:
>>       case XenbusStateInitialised:
>> -     case XenbusStateClosed:
>>               break;
>>
>>       case XenbusStateConnected:
>>               pcifront_try_connect(pdev);
>>               break;
>>
>> +     case XenbusStateClosed:
>> +             if (xdev->state == XenbusStateClosed)
>> +                     break;
>> +             /* Missed the backend's CLOSING state -- fallthrough */
>>       case XenbusStateClosing:
>>               dev_warn(&xdev->dev, "backend going away!\n");
>>               pcifront_try_disconnect(pdev);
>> --
>> 1.7.2.5

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

* Re: [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING
  2012-11-30 18:41     ` Bjorn Helgaas
@ 2012-11-30 21:42       ` Bjorn Helgaas
  2012-11-30 22:39         ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2012-11-30 21:42 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: David Vrabel, xen-devel, linux-kernel, linux-pci

On Fri, Nov 30, 2012 at 11:41 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Oct 19, 2012 at 6:59 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
>> On Thu, Oct 18, 2012 at 11:03:36AM +0100, David Vrabel wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
>>> CLOSED.  If a backend does transition to CLOSED too soon then the
>>> frontend may not see the CLOSING state and will not properly shutdown.
>>>
>>> So, treat an unexpected backend CLOSED state the same as CLOSING.
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> ---
>>> Cc: linux-pci@vger.kernel.org
>>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>>
>> Bjorn, do you want me to prep a git pull with this patch
>> or can I have your Ack to put it my tree and have it part of my
>> git pull to Linus?
>
> Sorry, I missed this.  I can put it in my -next branch for the v3.8
> merge window.  Would that work for you?

I put this in my -next branch, so we'll at least have a chance of
making a linux-next cycle before v3.7 pops.

>>> ---
>>>  drivers/pci/xen-pcifront.c |    5 ++++-
>>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
>>> index 0aab85a..a0c7312 100644
>>> --- a/drivers/pci/xen-pcifront.c
>>> +++ b/drivers/pci/xen-pcifront.c
>>> @@ -1068,13 +1068,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
>>>       case XenbusStateInitialising:
>>>       case XenbusStateInitWait:
>>>       case XenbusStateInitialised:
>>> -     case XenbusStateClosed:
>>>               break;
>>>
>>>       case XenbusStateConnected:
>>>               pcifront_try_connect(pdev);
>>>               break;
>>>
>>> +     case XenbusStateClosed:
>>> +             if (xdev->state == XenbusStateClosed)
>>> +                     break;
>>> +             /* Missed the backend's CLOSING state -- fallthrough */
>>>       case XenbusStateClosing:
>>>               dev_warn(&xdev->dev, "backend going away!\n");
>>>               pcifront_try_disconnect(pdev);
>>> --
>>> 1.7.2.5

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

* Re: [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING
  2012-11-30 21:42       ` Bjorn Helgaas
@ 2012-11-30 22:39         ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-11-30 22:39 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: David Vrabel, xen-devel, linux-kernel, linux-pci

On Fri, Nov 30, 2012 at 02:42:14PM -0700, Bjorn Helgaas wrote:
> On Fri, Nov 30, 2012 at 11:41 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Fri, Oct 19, 2012 at 6:59 AM, Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com> wrote:
> >> On Thu, Oct 18, 2012 at 11:03:36AM +0100, David Vrabel wrote:
> >>> From: David Vrabel <david.vrabel@citrix.com>
> >>>
> >>> Backend drivers shouldn't transistion to CLOSED unless the frontend is
> >>> CLOSED.  If a backend does transition to CLOSED too soon then the
> >>> frontend may not see the CLOSING state and will not properly shutdown.
> >>>
> >>> So, treat an unexpected backend CLOSED state the same as CLOSING.
> >>>
> >>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> >>> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >>> ---
> >>> Cc: linux-pci@vger.kernel.org
> >>> Cc: Bjorn Helgaas <bhelgaas@google.com>
> >>
> >> Bjorn, do you want me to prep a git pull with this patch
> >> or can I have your Ack to put it my tree and have it part of my
> >> git pull to Linus?
> >
> > Sorry, I missed this.  I can put it in my -next branch for the v3.8
> > merge window.  Would that work for you?
> 
> I put this in my -next branch, so we'll at least have a chance of
> making a linux-next cycle before v3.7 pops.

Great. Thx!
> 
> >>> ---
> >>>  drivers/pci/xen-pcifront.c |    5 ++++-
> >>>  1 files changed, 4 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> >>> index 0aab85a..a0c7312 100644
> >>> --- a/drivers/pci/xen-pcifront.c
> >>> +++ b/drivers/pci/xen-pcifront.c
> >>> @@ -1068,13 +1068,16 @@ static void __init_refok pcifront_backend_changed(struct xenbus_device *xdev,
> >>>       case XenbusStateInitialising:
> >>>       case XenbusStateInitWait:
> >>>       case XenbusStateInitialised:
> >>> -     case XenbusStateClosed:
> >>>               break;
> >>>
> >>>       case XenbusStateConnected:
> >>>               pcifront_try_connect(pdev);
> >>>               break;
> >>>
> >>> +     case XenbusStateClosed:
> >>> +             if (xdev->state == XenbusStateClosed)
> >>> +                     break;
> >>> +             /* Missed the backend's CLOSING state -- fallthrough */
> >>>       case XenbusStateClosing:
> >>>               dev_warn(&xdev->dev, "backend going away!\n");
> >>>               pcifront_try_disconnect(pdev);
> >>> --
> >>> 1.7.2.5

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

end of thread, other threads:[~2012-11-30 22:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-18 10:02 [PATCH 0/5] drivers: xen frontend devices should handle missed backend CLOSING David Vrabel
2012-10-18 10:03 ` [PATCH 3/5] xen-pcifront: handle backend CLOSED without CLOSING David Vrabel
2012-10-19 12:59   ` Konrad Rzeszutek Wilk
2012-11-30 18:41     ` Bjorn Helgaas
2012-11-30 21:42       ` Bjorn Helgaas
2012-11-30 22:39         ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).