Add cmd_flags to print_req_error

PATCH In review Balbir Singh 3 participants 3 messages Oct 29, 2018 – Oct 30, 2018 Block layer

Share & tools

Balbir Singh

author

2018-10-29 00:57 UTC #0 lore raw

PATCH 1/1 Add cmd_flags to print_req_error

I ran into a bug where after hibernation due to incompatible backends, the block driver returned BLK_STS_NOTSUPP, with the current message it's hard to find out what the command flags were. Adding req->cmd_flags help make the problem easier to diagnose.

Signed-off-by Balbir Singh <[email protected]>

Reviewed-by Eduardo Valentin <[email protected]>

---

The patch has been just compile tested only.

 block/blk-core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

block/blk-core.c +5 -4

index cdfabc5646da..28b8f93835a7 100644
@@ -256,10 +256,11 @@ static void print_req_error(struct request *req, blk_status_t status)
 	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors)))
 		return;
 
-	printk_ratelimited(KERN_ERR "%s: %s error, dev %s, sector %llu\n",
-			   __func__, blk_errors[idx].name, req->rq_disk ?
-			   req->rq_disk->disk_name : "?",
-			   (unsigned long long)blk_rq_pos(req));
+	printk_ratelimited(KERN_ERR "%s: %s error, dev %s, sector %llu flags %llx\n",
+				__func__, blk_errors[idx].name,
+				req->rq_disk ?  req->rq_disk->disk_name : "?",
+				(unsigned long long)blk_rq_pos(req),
+				(unsigned long long)req->cmd_flags);
 }
 
 static void req_bio_endio(struct request *rq, struct bio *bio,

Signature · 1 line

2.16.2

Jens Axboe

maintainer

2018-10-29 22:39 UTC #1 lore raw

Quoted 5 lines · Balbir Singh

I ran into a bug where after hibernation due to incompatible backends, the block driver returned BLK_STS_NOTSUPP, with the current message it's hard to find out what the command flags were. Adding req->cmd_flags help make the problem easier to diagnose.

I recently did the same thing to debug something, so I like this change. One comment:

block/blk-core.c static void print_req_error(struct request *req, blk_status_t status) · line 263

Quoted 5 lines

block/blk-core.c +5 -0

+	printk_ratelimited(KERN_ERR "%s: %s error, dev %s, sector %llu flags %llx\n",
+				__func__, blk_errors[idx].name,
+				req->rq_disk ?  req->rq_disk->disk_name : "?",
+				(unsigned long long)blk_rq_pos(req),
+				(unsigned long long)req->cmd_flags);

Why are you casting it to unsigned long long for printing? A simple %x should suffice, no cast necessary.

Signature · 1 line

Jens Axboe

Balbir Singh

2018-10-30 02:53 UTC #2 lore raw

Quoted 10 lines · Jens Axboe

Quoted 5 lines · Balbir Singh
I ran into a bug where after hibernation due to incompatible backends, the block driver returned BLK_STS_NOTSUPP, with the current message it's hard to find out what the command flags were. Adding req->cmd_flags help make the problem easier to diagnose.
I recently did the same thing to debug something, so I like this change. One comment:

Quoted 9 lines

block/blk-core.c static void print_req_error(struct request *req, blk_status_t status) · line 263
Quoted 5 lines
block/blk-core.c +5 -0
+	printk_ratelimited(KERN_ERR "%s: %s error, dev %s, sector %llu flags %llx\n",
+				__func__, blk_errors[idx].name,
+				req->rq_disk ?  req->rq_disk->disk_name : "?",
+				(unsigned long long)blk_rq_pos(req),
+				(unsigned long long)req->cmd_flags);
Why are you casting it to unsigned long long for printing? A simple %x should suffice, no cast necessary.

Thanks! resent

I was looking at blk_dump_rq_flags() and tried to do something similar. I agree a %x is sufficient, I was trying to be consistent. We can fix that path up if needed.

Quoted 4 lines

Signature · 1 line
Jens Axboe