On Sat, 2017-07-08 at 14:59 -0500, Benjamin Herrenschmidt wrote: > On Thu, 2017-06-29 at 16:54 +1000, Cyril Bur wrote: > > The OPAL calls performed in this driver shouldn't be using > > opal_async_wait_response() as this performs a wait_event() which, on > > long running OPAL calls could result in hung task warnings. wait_event() > > also prevents timely signal delivery which is also undesirable. > > > > This patch also attempts to quieten down the use of dev_err() when > > errors haven't actually occurred and also to return better information up > > the stack rather than always -EIO. > > So what happens when you get a signal then ? The async op is still in > progress, will the callers properly handle that ? > > For example, for a read, OPAL will potentially still be writing to > the destination buffer. Are the callers expecting this ? > > That sounds very broken to me .... >
Correct - I have a v2 that I let stew in my head over the weekend before sending. I think its good. Thanks, Cyril > You should simply limit the amount of flash that can be affected by > a single call to avoid long hung tasks. > > > Signed-off-by: Cyril Bur <cyril...@gmail.com> > > --- > > drivers/mtd/devices/powernv_flash.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/mtd/devices/powernv_flash.c > > b/drivers/mtd/devices/powernv_flash.c > > index 7b41af06f4fe..a7c5ae2b2898 100644 > > --- a/drivers/mtd/devices/powernv_flash.c > > +++ b/drivers/mtd/devices/powernv_flash.c > > @@ -88,19 +88,23 @@ static int powernv_flash_async_op(struct mtd_info *mtd, > > enum flash_op op, > > } > > > > if (rc != OPAL_ASYNC_COMPLETION) { > > - dev_err(dev, "opal_flash_async_op(op=%d) failed (rc %d)\n", > > - op, rc); > > + if (rc != OPAL_BUSY) > > + dev_err(dev, "opal_flash_async_op(op=%d) failed (rc > > %d)\n", > > + op, rc); > > opal_async_release_token(token); > > - rc = -EIO; > > + rc = opal_error_code(rc); > > goto out; > > } > > > > - rc = opal_async_wait_response(token, &msg); > > + rc = opal_async_wait_response_interruptible(token, &msg); > > opal_async_release_token(token); > > mutex_unlock(&info->lock); > > if (rc) { > > - dev_err(dev, "opal async wait failed (rc %d)\n", rc); > > - return -EIO; > > + if (rc == -ERESTARTSYS) > > + rc = -EINTR; > > + else > > + dev_err(dev, "opal async wait failed (rc %d)\n", rc); > > + return rc; > > } > > > > rc = opal_get_async_rc(msg); > > @@ -109,7 +113,7 @@ static int powernv_flash_async_op(struct mtd_info *mtd, > > enum flash_op op, > > if (retlen) > > *retlen = len; > > } else { > > - rc = -EIO; > > + rc = opal_error_code(rc); > > } > > > > return rc;