shared tag queue barrier comment

PATCH In review Nick Piggin 2 participants 2 messages Sep 14, 2007 Block layer

Share & tools

Nick Piggin

author

2007-09-14 06:41 UTC #0 lore raw

PATCH 1/1 shared tag queue barrier comment

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).

block/ll_rw_blk.c +10 -0

Index: 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;

Jens Axboe

2007-09-14 07:00 UTC #1 lore raw

Quoted 4 lines · Nick Piggin

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).

Thanks!

Quoted 2 lines

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).

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.

Signature · 1 line

Jens Axboe