Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
Hi, On 07/08/2014 01:25 PM, Hans de Goede wrote: Hi, On 07/08/2014 12:35 PM, Jens Axboe wrote: On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. Thanks, good to hear. It seems this one has not made it into 3.16-rc6 somehow ? Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On 2014-07-21 10:15, Hans de Goede wrote: Hi, On 07/08/2014 01:25 PM, Hans de Goede wrote: Hi, On 07/08/2014 12:35 PM, Jens Axboe wrote: On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. Thanks, good to hear. It seems this one has not made it into 3.16-rc6 somehow ? It'll go out shortly, was on vacation last week. It is queued up in for-linus. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
Hi, On 07/21/2014 10:17 AM, Jens Axboe wrote: On 2014-07-21 10:15, Hans de Goede wrote: Hi, On 07/08/2014 01:25 PM, Hans de Goede wrote: Hi, On 07/08/2014 12:35 PM, Jens Axboe wrote: On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. Thanks, good to hear. It seems this one has not made it into 3.16-rc6 somehow ? It'll go out shortly, was on vacation last week. It is queued up in for-linus. Ok, thanks for taking care of this. I hope you've enjoyed your vacation. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On Mon, Jul 07, 2014 at 02:42:40PM +0200, Hans de Goede wrote: I can't see any sensible way to fix the issue. Struct gendisk keeps a reference to the requeue_queue, and a struct gendisk reference is held as long as the block device is open. So whenever you surprise remove a device that is open (usually by having a filesystem mounted on it), the request_queue and it's reference to the tag structure will vastly outlive the Scsi_Host. Ok, so can we then please move forward with getting your patch merged ? I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On 2014-07-08 11:53, Christoph Hellwig wrote: On Mon, Jul 07, 2014 at 02:42:40PM +0200, Hans de Goede wrote: I can't see any sensible way to fix the issue. Struct gendisk keeps a reference to the requeue_queue, and a struct gendisk reference is held as long as the block device is open. So whenever you surprise remove a device that is open (usually by having a filesystem mounted on it), the request_queue and it's reference to the tag structure will vastly outlive the Scsi_Host. Ok, so can we then please move forward with getting your patch merged ? I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote:
I've posted the patch in reply to another report and added Jens to the
Cc list. As the block maintainer he need to review and merge it.
Which patch is this?
This one:
---
From: Christoph Hellwig h...@lst.de
Subject: block: don't assume last put of shared tags is for the host
There is no inherent reason why the last put of a tag structure must be
the one for the Scsi_Host, as device model objects can be held for
arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single
funtion that just release a references and get rid of the BUG() when the
host reference wasn't the last.
Signed-off-by: Christoph Hellwig h...@lst.de
diff --git a/block/blk-tag.c b/block/blk-tag.c
index 3f33d86..a185b86 100644
--- a/block/blk-tag.c
+++ b/block/blk-tag.c
@@ -27,18 +27,15 @@ struct request *blk_queue_find_tag(struct request_queue *q,
int tag)
EXPORT_SYMBOL(blk_queue_find_tag);
/**
- * __blk_free_tags - release a given set of tag maintenance info
+ * blk_free_tags - release a given set of tag maintenance info
* @bqt: the tag map to free
*
- * Tries to free the specified @bqt. Returns true if it was
- * actually freed and false if there are still references using it
+ * Drop the reference count on @bqt and frees it when the last reference
+ * is dropped.
*/
-static int __blk_free_tags(struct blk_queue_tag *bqt)
+void blk_free_tags(struct blk_queue_tag *bqt)
{
- int retval;
-
- retval = atomic_dec_and_test(bqt-refcnt);
- if (retval) {
+ if (atomic_dec_and_test(bqt-refcnt)) {
BUG_ON(find_first_bit(bqt-tag_map, bqt-max_depth)
bqt-max_depth);
@@ -50,9 +47,8 @@ static int __blk_free_tags(struct blk_queue_tag *bqt)
kfree(bqt);
}
-
- return retval;
}
+EXPORT_SYMBOL(blk_free_tags);
/**
* __blk_queue_free_tags - release tag maintenance info
@@ -69,28 +65,13 @@ void __blk_queue_free_tags(struct request_queue *q)
if (!bqt)
return;
- __blk_free_tags(bqt);
+ blk_free_tags(bqt);
q-queue_tags = NULL;
queue_flag_clear_unlocked(QUEUE_FLAG_QUEUED, q);
}
/**
- * blk_free_tags - release a given set of tag maintenance info
- * @bqt: the tag map to free
- *
- * For externally managed @bqt frees the map. Callers of this
- * function must guarantee to have released all the queues that
- * might have been using this tag map.
- */
-void blk_free_tags(struct blk_queue_tag *bqt)
-{
- if (unlikely(!__blk_free_tags(bqt)))
- BUG();
-}
-EXPORT_SYMBOL(blk_free_tags);
-
-/**
* blk_queue_free_tags - release tag maintenance info
* @q: the request queue for the device
*
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
Hi, On 07/08/2014 12:35 PM, Jens Axboe wrote: On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. Thanks, good to hear. Can you please add a Cc: stable ? This patch should at least be backported to 3.15, where uas support has been declared stable. With uas it is trivial to trigger the BUG_ON (and break any further device hotplugging) since uas allows for tcq, and thus using tags over usb (with all of its hotplug goodness and badness). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On 2014-07-08 13:25, Hans de Goede wrote: Hi, On 07/08/2014 12:35 PM, Jens Axboe wrote: On 2014-07-08 11:56, Christoph Hellwig wrote: On Tue, Jul 08, 2014 at 11:54:14AM +0200, Jens Axboe wrote: I've posted the patch in reply to another report and added Jens to the Cc list. As the block maintainer he need to review and merge it. Which patch is this? This one: --- From: Christoph Hellwig h...@lst.de Subject: block: don't assume last put of shared tags is for the host There is no inherent reason why the last put of a tag structure must be the one for the Scsi_Host, as device model objects can be held for arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single funtion that just release a references and get rid of the BUG() when the host reference wasn't the last. Thanks, applied. I don't think I was ever CC'ed on this one, though. Thanks, good to hear. Can you please add a Cc: stable ? This patch should at least be backported to 3.15, where uas support has been declared stable. With uas it is trivial to trigger the BUG_ON (and break any further device hotplugging) since uas allows for tcq, and thus using tags over usb (with all of its hotplug goodness and badness). I did mark it stable when queuing it up, so should be all set. -- Jens Axboe -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
On Sun, Jul 06, 2014 at 02:24:03PM +0200, Hans de Goede wrote: Hi, On 07/03/2014 08:01 AM, Christoph Hellwig wrote: Hi Hans, please test the path below: Note, I did a similar patch a while back, but then it was decided to try and fix the tear-down ordering instead (which seems to have not completely fixed the issue). I can't see any sensible way to fix the issue. Struct gendisk keeps a reference to the requeue_queue, and a struct gendisk reference is held as long as the block device is open. So whenever you surprise remove a device that is open (usually by having a filesystem mounted on it), the request_queue and it's reference to the tag structure will vastly outlive the Scsi_Host. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas: kernel BUG at block/blk-tag.c:89 is back again :|
Hi Hans,
please test the path below:
---
From: Christoph Hellwig h...@lst.de
Subject: block: don't assume last put of shared tags is for the host
There is no inherent reason why the last put of a tag structure must be
the one for the Scsi_Host, as device model objects can be held for
arbitrary periods. Merge blk_free_tags and __blk_free_tags into a single
funtion that just release a references and get rid of the BUG() when the
host reference wasn't the last.
Signed-off-by: Christoph Hellwig h...@lst.de
diff --git a/block/blk-tag.c b/block/blk-tag.c
index 3f33d86..a185b86 100644
--- a/block/blk-tag.c
+++ b/block/blk-tag.c
@@ -27,18 +27,15 @@ struct request *blk_queue_find_tag(struct request_queue *q,
int tag)
EXPORT_SYMBOL(blk_queue_find_tag);
/**
- * __blk_free_tags - release a given set of tag maintenance info
+ * blk_free_tags - release a given set of tag maintenance info
* @bqt: the tag map to free
*
- * Tries to free the specified @bqt. Returns true if it was
- * actually freed and false if there are still references using it
+ * Drop the reference count on @bqt and frees it when the last reference
+ * is dropped.
*/
-static int __blk_free_tags(struct blk_queue_tag *bqt)
+void blk_free_tags(struct blk_queue_tag *bqt)
{
- int retval;
-
- retval = atomic_dec_and_test(bqt-refcnt);
- if (retval) {
+ if (atomic_dec_and_test(bqt-refcnt)) {
BUG_ON(find_first_bit(bqt-tag_map, bqt-max_depth)
bqt-max_depth);
@@ -50,9 +47,8 @@ static int __blk_free_tags(struct blk_queue_tag *bqt)
kfree(bqt);
}
-
- return retval;
}
+EXPORT_SYMBOL(blk_free_tags);
/**
* __blk_queue_free_tags - release tag maintenance info
@@ -69,28 +65,13 @@ void __blk_queue_free_tags(struct request_queue *q)
if (!bqt)
return;
- __blk_free_tags(bqt);
+ blk_free_tags(bqt);
q-queue_tags = NULL;
queue_flag_clear_unlocked(QUEUE_FLAG_QUEUED, q);
}
/**
- * blk_free_tags - release a given set of tag maintenance info
- * @bqt: the tag map to free
- *
- * For externally managed @bqt frees the map. Callers of this
- * function must guarantee to have released all the queues that
- * might have been using this tag map.
- */
-void blk_free_tags(struct blk_queue_tag *bqt)
-{
- if (unlikely(!__blk_free_tags(bqt)))
- BUG();
-}
-EXPORT_SYMBOL(blk_free_tags);
-
-/**
* blk_queue_free_tags - release tag maintenance info
* @q: the request queue for the device
*
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
