* [bug report] mtip32xx: convert internal command issue to block IO path
@ 2017-06-23 9:05 Dan Carpenter
2017-06-23 15:18 ` Jens Axboe
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2017-06-23 9:05 UTC (permalink / raw)
To: axboe; +Cc: linux-block
Hello Jens Axboe,
The patch 3f5e6a35774c: "mtip32xx: convert internal command issue to
block IO path" from Apr 28, 2017, leads to the following static
checker warning:
drivers/block/mtip32xx/mtip32xx.c:1067 mtip_exec_internal_command()
warn: 'int_cmd->status' is unsigned
drivers/block/mtip32xx/mtip32xx.c
1065
1066 rv = int_cmd->status;
^^^^^^^^^^^^^^^
This is a new block error code.
1067 if (rv < 0) {
^^^^^^
So this doesn't make sense. This used to be rv <= 0.
1068 if (rv == -ERESTARTSYS) { /* interrupted */
1069 dev_err(&dd->pdev->dev,
1070 "Internal command [%02X] was interrupted after %u ms\n",
1071 fis->command,
1072 jiffies_to_msecs(jiffies - start));
1073 rv = -EINTR;
1074 goto exec_ic_exit;
1075 } else if (rv == 0) /* timeout */
^^^^^^^
This doesn't make sense either.
1076 dev_err(&dd->pdev->dev,
1077 "Internal command did not complete [%02X] within timeout of %lu ms\n",
1078 fis->command, timeout);
1079 else
1080 dev_err(&dd->pdev->dev,
1081 "Internal command [%02X] wait returned code [%d] after %lu ms - unhandled\n",
1082 fis->command, rv, timeout);
1083
1084 if (mtip_check_surprise_removal(dd->pdev) ||
1085 test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
1086 &dd->dd_flag)) {
1087 dev_err(&dd->pdev->dev,
1088 "Internal command [%02X] wait returned due to SR\n",
1089 fis->command);
1090 rv = -ENXIO;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [bug report] mtip32xx: convert internal command issue to block IO path
2017-06-23 9:05 [bug report] mtip32xx: convert internal command issue to block IO path Dan Carpenter
@ 2017-06-23 15:18 ` Jens Axboe
0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2017-06-23 15:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-block
On 06/23/2017 03:05 AM, Dan Carpenter wrote:
> Hello Jens Axboe,
>
> The patch 3f5e6a35774c: "mtip32xx: convert internal command issue to
> block IO path" from Apr 28, 2017, leads to the following static
> checker warning:
>
> drivers/block/mtip32xx/mtip32xx.c:1067 mtip_exec_internal_command()
> warn: 'int_cmd->status' is unsigned
>
> drivers/block/mtip32xx/mtip32xx.c
> 1065
> 1066 rv = int_cmd->status;
> ^^^^^^^^^^^^^^^
> This is a new block error code.
>
> 1067 if (rv < 0) {
> ^^^^^^
> So this doesn't make sense. This used to be rv <= 0.
It's actually two layers of breakage. After the referenced commit, some
of the below were wonky. But the new breakage is:
commit 2a842acab109f40f0d7d10b38e9ca88390628996
Author: Christoph Hellwig <hch@lst.de>
Date: Sat Jun 3 09:38:04 2017 +0200
block: introduce new block status code type
since that means that int_cmd->status is a blk_status_t, it's not an
-Exx value at all. And that patch changed the cmd->status field from an
int to a u8, which is why you are now seeing complaints.
The below should do the trick. All we really care about is whether
->staut is zero or not. Just turn that into EIO, and dump the various
attempts at clever reporting.
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index d8618a71da74..61b046f256ca 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -1063,23 +1063,10 @@ static int mtip_exec_internal_command(struct mtip_port *port,
/* insert request and run queue */
blk_execute_rq(rq->q, NULL, rq, true);
- rv = int_cmd->status;
- if (rv < 0) {
- if (rv == -ERESTARTSYS) { /* interrupted */
- dev_err(&dd->pdev->dev,
- "Internal command [%02X] was interrupted after %u ms\n",
- fis->command,
- jiffies_to_msecs(jiffies - start));
- rv = -EINTR;
- goto exec_ic_exit;
- } else if (rv == 0) /* timeout */
- dev_err(&dd->pdev->dev,
- "Internal command did not complete [%02X] within timeout of %lu ms\n",
- fis->command, timeout);
- else
- dev_err(&dd->pdev->dev,
- "Internal command [%02X] wait returned code [%d] after %lu ms - unhandled\n",
- fis->command, rv, timeout);
+ if (int_cmd->status) {
+ dev_err(&dd->pdev->dev, "Internal command [%02X] failed %d\n",
+ fis->command, int_cmd->status);
+ rv = -EIO;
if (mtip_check_surprise_removal(dd->pdev) ||
test_bit(MTIP_DDF_REMOVE_PENDING_BIT,
--
Jens Axboe
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-06-23 15:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-23 9:05 [bug report] mtip32xx: convert internal command issue to block IO path Dan Carpenter
2017-06-23 15:18 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox