[PATCH] Fix a use-after-free triggered by device removal

[PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

If the put_device() call in scsi_request_fn() drops the sdev refcount
to zero then the spin_lock() call after the put_device() call triggers
a use-after-free. Avoid that by making sure that blk_cleanup_queue()
can only finish after all active scsi_request_fn() calls have returned.

Reported-by: Chanho Min <chanho.min@lge.com>
Cc: James Bottomley <JBottomley@Parallels.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-core.c        |   21 ++++++++++++++++++++-
 block/blk-exec.c        |    2 +-
 block/blk.h             |    2 ++
 drivers/scsi/scsi_lib.c |   10 +---------
 include/linux/blkdev.h  |    5 +++++
 5 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 4b4dbdf..a668b71 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -293,6 +293,24 @@ void blk_sync_queue(struct request_queue *q)
 EXPORT_SYMBOL(blk_sync_queue);
 
 /**
+ * __blk_run_queue_uncond - run a single device queue independent of its state
+ * @q:	The queue to run
+ *
+ * Description:
+ *    Invoke request handling on a queue if there are any pending requests.
+ *    May be used to restart request handling after a request has completed.
+ *    This variant runs the queue whether or not the queue has been
+ *    stopped. Must be called with the queue lock held and interrupts
+ *    disabled. See also @blk_run_queue.
+ */
+void __blk_run_queue_uncond(struct request_queue *q)
+{
+	q->request_fn_active++;
+	q->request_fn(q);
+	q->request_fn_active--;
+}
+
+/**
  * __blk_run_queue - run a single device queue
  * @q:	The queue to run
  *
@@ -305,7 +323,7 @@ void __blk_run_queue(struct request_queue *q)
 	if (unlikely(blk_queue_stopped(q)))
 		return;
 
-	q->request_fn(q);
+	__blk_run_queue_uncond(q);
 }
 EXPORT_SYMBOL(__blk_run_queue);
 
@@ -388,6 +406,7 @@ void blk_drain_queue(struct request_queue *q, bool drain_all)
 			__blk_run_queue(q);
 
 		drain |= q->nr_rqs_elvpriv;
+		drain |= q->request_fn_active;
 
 		/*
 		 * Unfortunately, requests are queued at and tracked from
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 8b6dc5b..f4f6f4d 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -72,7 +72,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 	__blk_run_queue(q);
 	/* the queue is stopped so it won't be run */
 	if (rq->cmd_type == REQ_TYPE_PM_RESUME)
-		q->request_fn(q);
+		__blk_run_queue_uncond(q);
 	spin_unlock_irq(q->queue_lock);
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
diff --git a/block/blk.h b/block/blk.h
index 2a0ea32..0b11e22 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -145,6 +145,8 @@ int blk_try_merge(struct request *rq, struct bio *bio);
 
 void blk_queue_congestion_threshold(struct request_queue *q);
 
+void __blk_run_queue_uncond(struct request_queue *q);
+
 int blk_dev_init(void);
 
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ffd7773..9211e80 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1514,10 +1514,6 @@ static void scsi_request_fn(struct request_queue *q)
 	struct scsi_cmnd *cmd;
 	struct request *req;
 
-	if(!get_device(&sdev->sdev_gendev))
-		/* We must be tearing the block queue down already */
-		return;
-
 	/*
 	 * To start with, we keep looping until the queue is empty, or until
 	 * the host is no longer able to accept any more requests.
@@ -1626,11 +1622,7 @@ out_delay:
 	if (sdev->device_busy == 0)
 		blk_delay_queue(q, SCSI_QUEUE_DELAY);
 out:
-	/* must be careful here...if we trigger the ->remove() function
-	 * we cannot be holding the q lock */
-	spin_unlock_irq(q->queue_lock);
-	put_device(&sdev->sdev_gendev);
-	spin_lock_irq(q->queue_lock);
+	;
 }
 
 u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4a2ab7c..132334e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -377,6 +377,11 @@ struct request_queue {
 
 	unsigned int		nr_sorted;
 	unsigned int		in_flight[2];
+	/*
+	 * Number of active request_fn() calls for those request_fn()
+	 * implementations that unlock the queue_lock, e.g. scsi_request_fn().
+	 */
+	unsigned int		request_fn_active;
 
 	unsigned int		rq_timeout;
 	struct timer_list	timeout;
-- 
1.7.7

Re: [PATCH] Fix a use-after-free triggered by device removal

[Michael Christie]

On Sep 3, 2012, at 9:12 AM, Bart Van Assche <bvanassche@acm.org> wrote:

> If the put_device() call in scsi_request_fn() drops the sdev refcount
> to zero then the spin_lock() call after the put_device() call triggers
> a use-after-free. Avoid that by making sure that blk_cleanup_queue()
> can only finish after all active scsi_request_fn() calls have returned.



If we have this patch
http://marc.info/?l=linux-scsi&m=134453905402413&w=2
it seems we have all the scsi layer callers of the request_fn/*blk_run_queue holding a reference to the device when they make the call. Right, or are there some other places missing?

What are the other places we can call the request_fn without already holding a reference to the device? Is it the block layer? Is that why we need this patch?

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

_suOn 09/06/12 18:27, Michael Christie wrote:
> On Sep 3, 2012, at 9:12 AM, Bart Van Assche <bvanassche@acm.org> wrote:
>> If the put_device() call in scsi_request_fn() drops the sdev refcount
>> to zero then the spin_lock() call after the put_device() call triggers
>> a use-after-free. Avoid that by making sure that blk_cleanup_queue()
>> can only finish after all active scsi_request_fn() calls have returned.
> 
> If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2
> it seems we have all the scsi layer callers of the request_fn/
> *blk_run_queue holding a reference to the device when they make the call.
> Right, or are there some other places missing?
> 
> What are the other places we can call the request_fn without already
> holding a reference to the device? Is it the block layer? Is that why we
> need this patch?

Hello Mike,

The purpose of this patch is indeed to make *blk_run_queue() calls from
the block layer safe. There are several direct or indirect
*blk_run_queue() calls in the block layer where a reference on the queue
is held but not on the sdev, e.g. in the md, dm and bsg drivers.

Bart.

Re: [PATCH] Fix a use-after-free triggered by device removal

[Mike Christie]

On 09/06/2012 12:58 PM, Bart Van Assche wrote:
> _suOn 09/06/12 18:27, Michael Christie wrote:
>> On Sep 3, 2012, at 9:12 AM, Bart Van Assche <bvanassche@acm.org> wrote:
>>> If the put_device() call in scsi_request_fn() drops the sdev refcount
>>> to zero then the spin_lock() call after the put_device() call triggers
>>> a use-after-free. Avoid that by making sure that blk_cleanup_queue()
>>> can only finish after all active scsi_request_fn() calls have returned.
>>
>> If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2
>> it seems we have all the scsi layer callers of the request_fn/
>> *blk_run_queue holding a reference to the device when they make the call.
>> Right, or are there some other places missing?
>>
>> What are the other places we can call the request_fn without already
>> holding a reference to the device? Is it the block layer? Is that why we
>> need this patch?
> 
> Hello Mike,
> 
> The purpose of this patch is indeed to make *blk_run_queue() calls from
> the block layer safe. There are several direct or indirect
> *blk_run_queue() calls in the block layer where a reference on the queue
> is held but not on the sdev, e.g. in the md, dm and bsg drivers.
> 

Is there a race still? If some blk code is calling blk_run_queue
(waiting on the queue lock) but no IO is queued,
blk_drain_queue/blk_cleanup_queue could complete since the drain
counters are zero. Then blk_run_queue could grab the queue lock and call
the request_fn on a freed scsi_device (sdev pointed to by q->queuedata
would be freed so scsi_reuqest_fn would be freed).

Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then
fail IO?), or do we need a check in scsi_request_fn for this. A dead
queue check or maybe null q->queuedata in
scsi_device_dev_release_usercontext (do this with the queue lock held),
then check for null q->queuedata in scsi_request_fn?

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

On 09/06/12 20:14, Mike Christie wrote:
> On 09/06/2012 12:58 PM, Bart Van Assche wrote:
>> _suOn 09/06/12 18:27, Michael Christie wrote:
>>> On Sep 3, 2012, at 9:12 AM, Bart Van Assche <bvanassche@acm.org> wrote:
>>>> If the put_device() call in scsi_request_fn() drops the sdev refcount
>>>> to zero then the spin_lock() call after the put_device() call triggers
>>>> a use-after-free. Avoid that by making sure that blk_cleanup_queue()
>>>> can only finish after all active scsi_request_fn() calls have returned.
>>>
>>> If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2
>>> it seems we have all the scsi layer callers of the request_fn/
>>> *blk_run_queue holding a reference to the device when they make the call.
>>> Right, or are there some other places missing?
>>>
>>> What are the other places we can call the request_fn without already
>>> holding a reference to the device? Is it the block layer? Is that why we
>>> need this patch?
>>
>> The purpose of this patch is indeed to make *blk_run_queue() calls from
>> the block layer safe. There are several direct or indirect
>> *blk_run_queue() calls in the block layer where a reference on the queue
>> is held but not on the sdev, e.g. in the md, dm and bsg drivers.
> 
> Is there a race still? If some blk code is calling blk_run_queue
> (waiting on the queue lock) but no IO is queued,
> blk_drain_queue/blk_cleanup_queue could complete since the drain
> counters are zero. Then blk_run_queue could grab the queue lock and call
> the request_fn on a freed scsi_device (sdev pointed to by q->queuedata
> would be freed so scsi_reuqest_fn would be freed).
> 
> Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then
> fail IO?), or do we need a check in scsi_request_fn for this. A dead
> queue check or maybe null q->queuedata in
> scsi_device_dev_release_usercontext (do this with the queue lock held),
> then check for null q->queuedata in scsi_request_fn?

Yet another scenario is that scsi_remove_host() gets invoked and
finishes after scsi_request_fn() has unlocked the queue and before it
locks the queue again. That's a scenario that can't be handled by adding
more checks at the start of __blk_run_queue() or scsi_request_fn().

Bart.

Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo]

Hello, Bart, Mike.

On Thu, Sep 06, 2012 at 08:52:09PM +0200, Bart Van Assche wrote:
> >> The purpose of this patch is indeed to make *blk_run_queue() calls from
> >> the block layer safe. There are several direct or indirect
> >> *blk_run_queue() calls in the block layer where a reference on the queue
> >> is held but not on the sdev, e.g. in the md, dm and bsg drivers.

Yeah, while writing blk_drain_queue(), I was thinking only about
requests.  I didn't consider control reaching into driver.

> > Is there a race still? If some blk code is calling blk_run_queue
> > (waiting on the queue lock) but no IO is queued,
> > blk_drain_queue/blk_cleanup_queue could complete since the drain
> > counters are zero. Then blk_run_queue could grab the queue lock and call
> > the request_fn on a freed scsi_device (sdev pointed to by q->queuedata
> > would be freed so scsi_reuqest_fn would be freed).
> > 
> > Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then
> > fail IO?), or do we need a check in scsi_request_fn for this. A dead
> > queue check or maybe null q->queuedata in
> > scsi_device_dev_release_usercontext (do this with the queue lock held),
> > then check for null q->queuedata in scsi_request_fn?
> 
> Yet another scenario is that scsi_remove_host() gets invoked and
> finishes after scsi_request_fn() has unlocked the queue and before it
> locks the queue again. That's a scenario that can't be handled by adding
> more checks at the start of __blk_run_queue() or scsi_request_fn().

I think Mike is wondering whether your patch in isolation is enough or
we also need to have DEAD check there too.  The proposed patch can't
handle the case where q->request_fn() is invoked after drain is
complete.  I'm not really sure whether that can happen tho.

Other than that, yeah, I think it's a real problem and we definitely
want to remove get/put_device() from scsi_request_fn().  Refcnting
oneself is often wrong (as shown here too) and generally just sad.

Thanks.

-- 
tejun

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

On 09/07/12 01:20, Tejun Heo wrote:
> I think Mike is wondering whether your patch in isolation is enough or
> we also need to have DEAD check there too.  The proposed patch can't
> handle the case where q->request_fn() is invoked after drain is
> complete.  I'm not really sure whether that can happen tho.

Hello Tejun,

I'm not sure it would be a good idea to add a blk_queue_dead() check in
any of the __blk_run_queue() variants since blk_drain_queue() can invoke
__blk_run_queue() to drain the queue.

Also, as far as I can see the functions that can insert a request into
the queue (blk_insert_cloned_request(), queue_unplugged(),
blk_execute_rq_nowait()) all check whether the queue is dead before
inserting a request. That should be sufficient to prevent that new
requests are queued after QUEUE_FLAG_DEAD has been set.

Bart.

Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo]

Hello,

On Fri, Sep 07, 2012 at 08:57:10AM +0200, Bart Van Assche wrote:
> I'm not sure it would be a good idea to add a blk_queue_dead() check in
> any of the __blk_run_queue() variants since blk_drain_queue() can invoke
> __blk_run_queue() to drain the queue.

Right, we can't cancel requests from block layer which were already
seen by the driver.

> Also, as far as I can see the functions that can insert a request into
> the queue (blk_insert_cloned_request(), queue_unplugged(),
> blk_execute_rq_nowait()) all check whether the queue is dead before
> inserting a request. That should be sufficient to prevent that new
> requests are queued after QUEUE_FLAG_DEAD has been set.

Yes, but does that guarantee that none would call into ->request_fn()?
If so, fine; otherwise, we may need to add another state to prevent
that.

Thanks.

-- 
tejun

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

On 09/11/12 01:38, Tejun Heo wrote:
> On Fri, Sep 07, 2012 at 08:57:10AM +0200, Bart Van Assche wrote:
>> Also, as far as I can see the functions that can insert a request into
>> the queue (blk_insert_cloned_request(), queue_unplugged(),
>> blk_execute_rq_nowait()) all check whether the queue is dead before
>> inserting a request. That should be sufficient to prevent that new
>> requests are queued after QUEUE_FLAG_DEAD has been set.
> 
> Yes, but does that guarantee that none would call into ->request_fn()?
> If so, fine; otherwise, we may need to add another state to prevent
> that.

Good question. As far as I can see calling request_queue.request_fn() is
fine as long as the caller holds a reference on the queue. If e.g.
scsi_request_fn() would get invoked after blk_drain_queue() finished it
will return immediately because it was invoked with an empty request
queue. So we should be fine as long as all blk_run_queue() callers
either hold a reference on the request queue itself or on the sdev that
owns the request queue. As far as I can see if patch
http://marc.info/?l=linux-scsi&m=134453905402413 gets accepted then all
callers in the SCSI core of blk_run_queue() will hold a (direct or
indirect) reference on the request_queue before invoking blk_run_queue()
or __blk_run_queue().

Bart.

Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo]

Hello,

On Tue, Sep 11, 2012 at 08:42:28AM +0200, Bart Van Assche wrote:
> Good question. As far as I can see calling request_queue.request_fn() is
> fine as long as the caller holds a reference on the queue. If e.g.
> scsi_request_fn() would get invoked after blk_drain_queue() finished it
> will return immediately because it was invoked with an empty request
> queue. So we should be fine as long as all blk_run_queue() callers
> either hold a reference on the request queue itself or on the sdev that
> owns the request queue. As far as I can see if patch
> http://marc.info/?l=linux-scsi&m=134453905402413 gets accepted then all
> callers in the SCSI core of blk_run_queue() will hold a (direct or
> indirect) reference on the request_queue before invoking blk_run_queue()
> or __blk_run_queue().

It's been quite a while since I really looked through the code and I'm
feeling a bit dense but what you describe seems like a two-pronged
approach where the drain stalling, when properly done, should be
enough.

The problem at hand IIUC is ->request_fn() being invoked when
request_queue itself is alive but the underlying driver is gone.  We
already make sure that a new request is not queued once drain is
complete but there's no guarantee about calling into ->request_fn()
and this is what you want to fix, right?

I think this is something which the block layer proper should handle
correctly and expose sane interface.  ie. if the caller has
request_queue reference, it should be safe to call __blk_run_queue()
no matter what.  As long as SCSI follows proper shutdown procedure, it
shouldn't need to worry about this.

Am I hopelessly confused somewhere?

Thanks.

-- 
tejun

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

On 09/12/12 22:53, Tejun Heo wrote:
> The problem at hand IIUC is ->request_fn() being invoked when
> request_queue itself is alive but the underlying driver is gone.  We
> already make sure that a new request is not queued once drain is
> complete but there's no guarantee about calling into ->request_fn()
> and this is what you want to fix, right?

Actually it's a slightly different issue that I want to address, namely
that with the current implementation of the block layer and the SCSI
core it's possible that blk_cleanup_queue() finishes after
scsi_request_fn() has unlocked the queue lock and before it obtains the
queue lock again.

The reason I'm proposing to add a counter in the block layer and not in
the SCSI core is because I think it would be useful for other
request-based block drivers too to be able to unlock the queue inside
their ->request_fn(). That would allow to reduce lock contention on the
request_queue lock for low-latency block drivers.

Bart.

Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo]

Hello, Bart.

On Thu, Sep 13, 2012 at 09:26:59AM +0200, Bart Van Assche wrote:
> On 09/12/12 22:53, Tejun Heo wrote:
> > The problem at hand IIUC is ->request_fn() being invoked when
> > request_queue itself is alive but the underlying driver is gone.  We
> > already make sure that a new request is not queued once drain is
> > complete but there's no guarantee about calling into ->request_fn()
> > and this is what you want to fix, right?
> 
> Actually it's a slightly different issue that I want to address, namely
> that with the current implementation of the block layer and the SCSI
> core it's possible that blk_cleanup_queue() finishes after
> scsi_request_fn() has unlocked the queue lock and before it obtains the
> queue lock again.
> 
> The reason I'm proposing to add a counter in the block layer and not in
> the SCSI core is because I think it would be useful for other
> request-based block drivers too to be able to unlock the queue inside
> their ->request_fn(). That would allow to reduce lock contention on the
> request_queue lock for low-latency block drivers.

Oh yeah, I definitely think this is something which needs to be solved
from the block layer but I'm hoping this could cover the case Chanho
is trying to solve too.  They're different but similar problems - you
don't want blk_cleanup_queue() to finish while someone is executing
inside it and you don't want anyone to enter it after
blk_cleanup_queue() is finished, so I really think we should have
block layer solution which fixes both problems.  That should be
possible, right?

Thanks.

-- 
tejun

Re: [PATCH] Fix a use-after-free triggered by device removal

[Bart Van Assche]

On 09/13/12 18:53, Tejun Heo wrote:
> Oh yeah, I definitely think this is something which needs to be solved
> from the block layer but I'm hoping this could cover the case Chanho
> is trying to solve too.  They're different but similar problems - you
> don't want blk_cleanup_queue() to finish while someone is executing
> inside it and you don't want anyone to enter it after
> blk_cleanup_queue() is finished, so I really think we should have
> block layer solution which fixes both problems.  That should be
> possible, right?

If I do not receive further feedback I'll start testing the patch below
(on top of the patch at the start of this thread):

[PATCH] Avoid that request_fn() gets invoked after draining the queue finished

---
 block/blk-core.c       |    4 ++++
 include/linux/blkdev.h |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a668b71..575b7c4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -305,6 +305,9 @@ EXPORT_SYMBOL(blk_sync_queue);
  */
 void __blk_run_queue_uncond(struct request_queue *q)
 {
+	if (unlikely(blk_queue_drained(q)))
+		return;
+
 	q->request_fn_active++;
 	q->request_fn(q);
 	q->request_fn_active--;
@@ -532,6 +535,7 @@ void blk_cleanup_queue(struct request_queue *q)
 	blk_sync_queue(q);
 
 	spin_lock_irq(lock);
+	queue_flag_set(QUEUE_FLAG_DRAINED, q);
 	if (q->queue_lock != &q->__queue_lock)
 		q->queue_lock = &q->__queue_lock;
 	spin_unlock_irq(lock);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 132334e..ceebc39 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -456,6 +456,7 @@ struct request_queue {
 #define QUEUE_FLAG_ADD_RANDOM  16	/* Contributes to random pool */
 #define QUEUE_FLAG_SECDISCARD  17	/* supports SECDISCARD */
 #define QUEUE_FLAG_SAME_FORCE  18	/* force complete on same CPU */
+#define QUEUE_FLAG_DRAINED     19	/* queue tear-down finished */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -526,6 +527,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define blk_queue_tagged(q)	test_bit(QUEUE_FLAG_QUEUED, &(q)->queue_flags)
 #define blk_queue_stopped(q)	test_bit(QUEUE_FLAG_STOPPED, &(q)->queue_flags)
 #define blk_queue_dead(q)	test_bit(QUEUE_FLAG_DEAD, &(q)->queue_flags)
+#define blk_queue_drained(q)	test_bit(QUEUE_FLAG_DRAINED, &(q)->queue_flags)
 #define blk_queue_bypass(q)	test_bit(QUEUE_FLAG_BYPASS, &(q)->queue_flags)
 #define blk_queue_nomerges(q)	test_bit(QUEUE_FLAG_NOMERGES, &(q)->queue_flags)
 #define blk_queue_noxmerges(q)	\
-- 
1.7.7

Re: [PATCH] Fix a use-after-free triggered by device removal

[Tejun Heo]

Hello,

On Thu, Sep 13, 2012 at 08:27:29PM +0200, Bart Van Assche wrote:
> If I do not receive further feedback I'll start testing the patch below
> (on top of the patch at the start of this thread):

Yeah, it generally seems fine to me.  This makes Chanho's patch
unnecessary, right?  Just some minor points.

> @@ -456,6 +456,7 @@ struct request_queue {
>  #define QUEUE_FLAG_ADD_RANDOM  16	/* Contributes to random pool */
>  #define QUEUE_FLAG_SECDISCARD  17	/* supports SECDISCARD */
>  #define QUEUE_FLAG_SAME_FORCE  18	/* force complete on same CPU */
> +#define QUEUE_FLAG_DRAINED     19	/* queue tear-down finished */

This is rather confusing to me.  Maybe we can have QUEUE_FLAG_DYING
and QUEUE_FLAG_DEAD?  Or at least explain the involve state changes in
detail.

Thanks.

-- 
tejun