From: Liu Ping Fan <kernelfans@gmail.com>
To: linux-scsi@vger.kernel.org
Cc: Adaptec OEM Raid Solutions <aacraid@adaptec.com>,
Jens Axboe <axboe@kernel.dk>, Paolo Bonzini <pbonzini@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Jeff Moyer <jmoyer@redhat.com>
Subject: [RFC 2/9] scsi: ensure request is dequeue when finishing scmd
Date: Fri, 30 May 2014 16:15:40 +0800 [thread overview]
Message-ID: <1401437747-2097-3-git-send-email-pingfank@linux.vnet.ibm.com> (raw)
In-Reply-To: <1401437747-2097-1-git-send-email-pingfank@linux.vnet.ibm.com>
When a timer requeue a request, it clears out REQ_ATOM_COMPLETE bit.
Hence if there is an in-flight scmd asks scsi-eh to handle an error by
joining the eh_entry list, then when scsi_error_handler() handles a scmd,
the scmd->request may be still on request_queue. This will trigger
the BUG_ON(!list_empty(&req->queuelist)) in __blk_put_request().
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
note: I hit this bug in my test, and the above comment is my guess. Hope for
more comments about it.
---
block/blk.h | 1 -
drivers/scsi/scsi_error.c | 1 +
drivers/scsi/scsi_lib.c | 20 +++++++++++++++++++-
drivers/scsi/scsi_priv.h | 1 +
include/linux/blkdev.h | 1 +
5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/block/blk.h b/block/blk.h
index c90e1d8..a9cad62 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -29,7 +29,6 @@ int blk_rq_append_bio(struct request_queue *q, struct request *rq,
struct bio *bio);
void blk_queue_bypass_start(struct request_queue *q);
void blk_queue_bypass_end(struct request_queue *q);
-void blk_dequeue_request(struct request *rq);
void __blk_queue_free_tags(struct request_queue *q);
bool __blk_end_bidi_request(struct request *rq, int error,
unsigned int nr_bytes, unsigned int bidi_bytes);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c5f49cf..3b8b95b 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2068,6 +2068,7 @@ void scsi_eh_flush_done_q(struct list_head *done_q)
SCSI_LOG_ERROR_RECOVERY(3, printk("%s: flush finish"
" cmd: %p\n",
current->comm, scmd));
+ scsi_queue_dequeue(scmd);
scsi_finish_command(scmd);
}
}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 7fb2afe..e117579 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -210,6 +210,21 @@ void scsi_queue_insert(struct scsi_cmnd *cmd, int reason)
{
__scsi_queue_insert(cmd, reason, 1);
}
+
+void scsi_queue_dequeue(struct scsi_cmnd *cmd)
+{
+ struct scsi_device *device = cmd->device;
+ struct request_queue *q = device->request_queue;
+ struct request *req = cmd->request;
+ unsigned long flags;
+
+ if (list_empty(&cmd->request->queuelist))
+ return;
+ spin_lock_irqsave(q->queue_lock, flags);
+ blk_dequeue_request(req);
+ spin_unlock_irqrestore(q->queue_lock, flags);
+}
+
/**
* scsi_execute - insert request and wait for the result
* @sdev: scsi device
@@ -859,6 +874,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
*/
req->next_rq->resid_len = scsi_in(cmd)->resid;
+ scsi_queue_dequeue(cmd);
scsi_release_buffers(cmd);
blk_end_request_all(req, 0);
@@ -1038,8 +1054,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
}
if (blk_end_request_err(req, error))
scsi_requeue_command(q, cmd);
- else
+ else {
+ scsi_queue_dequeue(cmd);
scsi_next_command(cmd);
+ }
break;
case ACTION_REPREP:
/* Unprep the request and put it back at the head of the queue.
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index f079a59..601b964 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -84,6 +84,7 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd);
extern int scsi_maybe_unblock_host(struct scsi_device *sdev);
extern void scsi_device_unbusy(struct scsi_device *sdev);
extern void scsi_queue_insert(struct scsi_cmnd *cmd, int reason);
+extern void scsi_queue_dequeue(struct scsi_cmnd *cmd);
extern void scsi_next_command(struct scsi_cmnd *cmd);
extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
extern void scsi_run_host_queues(struct Scsi_Host *shost);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8168524..ee7ddcd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -916,6 +916,7 @@ static inline unsigned int blk_rq_count_bios(struct request *rq)
*/
extern struct request *blk_peek_request(struct request_queue *q);
extern void blk_start_request(struct request *rq);
+extern void blk_dequeue_request(struct request *rq);
extern struct request *blk_fetch_request(struct request_queue *q);
/*
--
1.8.1.4
next prev parent reply other threads:[~2014-05-30 8:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-30 8:15 [RFC 0/9] fix for the race issue between scsi timer and in-flight scmd Liu Ping Fan
2014-05-30 8:15 ` [RFC 1/9] block: make timeout_list protectd by REQ_ATOM_COMPLETE bit Liu Ping Fan
2014-05-30 8:15 ` Liu Ping Fan [this message]
2014-05-30 8:15 ` [RFC 3/9] scsi: introduce new internal flag SUCCESS_REMOVE Liu Ping Fan
2014-05-30 8:15 ` [RFC 4/9] blk: change the prototype of blk_complete_request() Liu Ping Fan
2014-05-30 8:15 ` Liu Ping Fan
2014-05-30 8:15 ` [RFC 6/9] blk: split the reclaim of req from blk_finish_request() Liu Ping Fan
2014-05-30 8:15 ` [RFC 7/9] scsi: adopt ref on scsi_cmnd to avoid a race on request Liu Ping Fan
2014-05-30 8:15 ` [RFC 8/9] scsi: virtscsi: work around to abort a scmd Liu Ping Fan
2014-05-30 8:15 ` [RFC 9/9] scsi: ibmvscsi: return SUCCESS_REMOVE when finding a abort cmd Liu Ping Fan
2014-05-30 8:26 ` [RFC 0/9] fix for the race issue between scsi timer and in-flight scmd Paolo Bonzini
2014-05-30 8:31 ` liu ping fan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1401437747-2097-3-git-send-email-pingfank@linux.vnet.ibm.com \
--to=kernelfans@gmail.com \
--cc=aacraid@adaptec.com \
--cc=axboe@kernel.dk \
--cc=jmoyer@redhat.com \
--cc=linux-scsi@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).