Re: [PATCH 5/5] mtd: powernv_flash: Use opal_async_wait_response_interruptible()
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2017-07-08 20:01:07
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 .... You should simply limit the amount of flash that can be affected by a single call to avoid long hung tasks.
quoted hunk ↗ jump to hunk
Signed-off-by: Cyril Bur <redacted> --- 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;