[PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[Andrew Donnellan]

If we open a context but do not start it (either because we do not attempt
to start it, or because it fails to start for some reason), we are left
with a context in state OPENED. Previously, cxl_release_context() only
allowed releasing contexts in state CLOSED, so attempting to release an
OPENED context would fail.

In particular, this bug causes available contexts to run out after some EEH
failures, where drivers attempt to release contexts that have failed to
start.

Allow releasing contexts in any state other than STARTED, i.e. OPENED or
CLOSED (we can't release a STARTED context as it's currently using the
hardware).

Cc: stable@vger.kernel.org
Fixes: 6f7f0b3df6d4 ("cxl: Add AFU virtual PHB and kernel API")
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 drivers/misc/cxl/api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 6a768a9..1c520b8 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -59,7 +59,7 @@ EXPORT_SYMBOL_GPL(cxl_get_phys_dev);
 
 int cxl_release_context(struct cxl_context *ctx)
 {
-	if (ctx->status != CLOSED)
+	if (ctx->status == STARTED)
 		return -EBUSY;
 
 	put_device(&ctx->afu->dev);
-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[Ian Munsie]

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[Michael Ellerman]

On Tue, 2015-08-18 at 16:30 +1000, Andrew Donnellan wrote:
> If we open a context but do not start it (either because we do not attempt
> to start it, or because it fails to start for some reason), we are left
> with a context in state OPENED. Previously, cxl_release_context() only
> allowed releasing contexts in state CLOSED, so attempting to release an
> OPENED context would fail.
> 
> In particular, this bug causes available contexts to run out after some EEH
> failures, where drivers attempt to release contexts that have failed to
> start.
> 
> Allow releasing contexts in any state other than STARTED, i.e. OPENED or
> CLOSED (we can't release a STARTED context as it's currently using the
> hardware).
> 
> Cc: stable@vger.kernel.org
> Fixes: 6f7f0b3df6d4 ("cxl: Add AFU virtual PHB and kernel API")
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  drivers/misc/cxl/api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 6a768a9..1c520b8 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -59,7 +59,7 @@ EXPORT_SYMBOL_GPL(cxl_get_phys_dev);
>  
>  int cxl_release_context(struct cxl_context *ctx)
>  {
> -	if (ctx->status != CLOSED)
> +	if (ctx->status == STARTED)
>  		return -EBUSY;

So this doesn't break when you add a new state, is it worth writing it as:

	if (ctx->status >= STARTED)
		return -EBUSY;

?

cheers

Re: [PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[Michael Neuling]

On Tue, 2015-08-18 at 19:19 +1000, Michael Ellerman wrote:
> On Tue, 2015-08-18 at 16:30 +1000, Andrew Donnellan wrote:
> > If we open a context but do not start it (either because we do not atte=
mpt
> > to start it, or because it fails to start for some reason), we are left
> > with a context in state OPENED. Previously, cxl_release_context() only
> > allowed releasing contexts in state CLOSED, so attempting to release an
> > OPENED context would fail.
> >=20
> > In particular, this bug causes available contexts to run out after some=
 EEH
> > failures, where drivers attempt to release contexts that have failed to
> > start.
> >=20
> > Allow releasing contexts in any state other than STARTED, i.e. OPENED o=
r
> > CLOSED (we can't release a STARTED context as it's currently using the
> > hardware).
> >=20
> > Cc: stable@vger.kernel.org
> > Fixes: 6f7f0b3df6d4 ("cxl: Add AFU virtual PHB and kernel API")
> > Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> > Signed-off-by: Daniel Axtens <dja@axtens.net>
> > ---
> >  drivers/misc/cxl/api.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >=20
> > diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> > index 6a768a9..1c520b8 100644
> > --- a/drivers/misc/cxl/api.c
> > +++ b/drivers/misc/cxl/api.c
> > @@ -59,7 +59,7 @@ EXPORT_SYMBOL_GPL(cxl_get_phys_dev);
> > =20
> >  int cxl_release_context(struct cxl_context *ctx)
> >  {
> > -	if (ctx->status !=3D CLOSED)
> > +	if (ctx->status =3D=3D STARTED)
> >  		return -EBUSY;
>=20
> So this doesn't break when you add a new state, is it worth writing it as=
:
>=20
> 	if (ctx->status >=3D STARTED)
> 		return -EBUSY;
>=20
> ?

Yeah I think that would be more future proof, although it won't make a
difference with the current code.

FWIW, looks good to me.

Mikey

Re: [PATCH] cxl: Allow release of contexts which have been OPENED but not STARTED

[Andrew Donnellan]

On 19/08/15 02:23, Michael Neuling wrote:
>> So this doesn't break when you add a new state, is it worth writing it as:
>>
>> 	if (ctx->status >= STARTED)
>> 		return -EBUSY;
>>
>> ?
>
> Yeah I think that would be more future proof, although it won't make a
> difference with the current code.

Sounds reasonable, I'll submit a V2.


Andrew

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited