Re: [patch] shared tag queue barrier comment

Previous thread: rtc-ds1742.c should use resource_size_t for base address by David Gibson on Friday, September 14, 2007 - 1:54 am. (4 messages)

Next thread: Intel-Quad on GA-P35-S3 motherboard with 4*2GB by Wojciech Kromer on Friday, September 14, 2007 - 4:50 am. (10 messages)
To: Linux Kernel Mailing List <linux-kernel@...>, Linus Torvalds <torvalds@...>, Jens Axboe <axboe@...>
Date: Friday, September 14, 2007 - 2:41 am

Should add some comments for the tag barriers (they won't be so important
if we can switch over to the explicit _lock bitops, but for now we should
make it clear).

Jens' original patch said a barrier after the test_and_clear_bit was also
required. I can't see why (and it would prevent the use of the _lock bitop).

--
Index: linux-2.6/block/ll_rw_blk.c
===================================================================
--- linux-2.6.orig/block/ll_rw_blk.c
+++ linux-2.6/block/ll_rw_blk.c
@@ -1085,6 +1085,12 @@ void blk_queue_end_tag(struct request_qu

bqt->tag_index[tag] = NULL;

+ /*
+ * We use test_and_clear_bit's memory ordering properties here.
+ * The tag_map bit acts as a lock for tag_index[bit], so we need
+ * a barrer before clearing the bit (precisely: release semantics).
+ * Could use clear_bit_unlock when it is merged.
+ */
if (unlikely(!test_and_clear_bit(tag, bqt->tag_map))) {
printk(KERN_ERR "%s: attempt to clear non-busy tag (%d)\n",
__FUNCTION__, tag);
@@ -1137,6 +1143,10 @@ int blk_queue_start_tag(struct request_q
return 1;

} while (test_and_set_bit(tag, bqt->tag_map));
+ /*
+ * We rely on test_and_set_bit providing lock memory ordering semantics
+ * (could use test_and_set_bit_lock when it is merged).
+ */

rq->cmd_flags |= REQ_QUEUED;
rq->tag = tag;
-

To: Nick Piggin <npiggin@...>
Cc: Linux Kernel Mailing List <linux-kernel@...>, Linus Torvalds <torvalds@...>
Date: Friday, September 14, 2007 - 3:00 am

Yeah, I don't think it's needed either. The important bit was ordering
the ->tag_index[tag] = NULL setting before clearing the bit, and as long
as those two operations are ordered, we are good to go. So the last
memory barrier was not needed.

--
Jens Axboe

-

Previous thread: rtc-ds1742.c should use resource_size_t for base address by David Gibson on Friday, September 14, 2007 - 1:54 am. (4 messages)

Next thread: Intel-Quad on GA-P35-S3 motherboard with 4*2GB by Wojciech Kromer on Friday, September 14, 2007 - 4:50 am. (10 messages)