[PATCH v3 1/3] block: support different tag allocation policy

[PATCH v3 1/3] block: support different tag allocation policy

[Shaohua Li]

The libata tag allocation is using a round-robin policy. Next patch will
make libata use block generic tag allocation, so let's add a policy to
tag allocation.

Currently two policies: FIFO (default) and round-robin.

Cc: Jens Axboe <axboe@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 block/blk-tag.c          | 33 +++++++++++++++++++++++++--------
 drivers/block/osdblk.c   |  2 +-
 drivers/scsi/scsi_scan.c |  3 ++-
 include/linux/blkdev.h   |  8 ++++++--
 include/scsi/scsi_host.h |  3 +++
 include/scsi/scsi_tcq.h  |  3 ++-
 6 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/block/blk-tag.c b/block/blk-tag.c
index a185b86..f0344e6 100644
--- a/block/blk-tag.c
+++ b/block/blk-tag.c
@@ -119,7 +119,7 @@ init_tag_map(struct request_queue *q, struct blk_queue_tag *tags, int depth)
 }
 
 static struct blk_queue_tag *__blk_queue_init_tags(struct request_queue *q,
-						   int depth)
+						int depth, int alloc_policy)
 {
 	struct blk_queue_tag *tags;
 
@@ -131,6 +131,8 @@ static struct blk_queue_tag *__blk_queue_init_tags(struct request_queue *q,
 		goto fail;
 
 	atomic_set(&tags->refcnt, 1);
+	tags->alloc_policy = alloc_policy;
+	tags->next_tag = 0;
 	return tags;
 fail:
 	kfree(tags);
@@ -140,10 +142,11 @@ static struct blk_queue_tag *__blk_queue_init_tags(struct request_queue *q,
 /**
  * blk_init_tags - initialize the tag info for an external tag map
  * @depth:	the maximum queue depth supported
+ * @alloc_policy: tag allocation policy
  **/
-struct blk_queue_tag *blk_init_tags(int depth)
+struct blk_queue_tag *blk_init_tags(int depth, int alloc_policy)
 {
-	return __blk_queue_init_tags(NULL, depth);
+	return __blk_queue_init_tags(NULL, depth, alloc_policy);
 }
 EXPORT_SYMBOL(blk_init_tags);
 
@@ -152,19 +155,20 @@ EXPORT_SYMBOL(blk_init_tags);
  * @q:  the request queue for the device
  * @depth:  the maximum queue depth supported
  * @tags: the tag to use
+ * @alloc_policy: tag allocation policy
  *
  * Queue lock must be held here if the function is called to resize an
  * existing map.
  **/
 int blk_queue_init_tags(struct request_queue *q, int depth,
-			struct blk_queue_tag *tags)
+			struct blk_queue_tag *tags, int alloc_policy)
 {
 	int rc;
 
 	BUG_ON(tags && q->queue_tags && tags != q->queue_tags);
 
 	if (!tags && !q->queue_tags) {
-		tags = __blk_queue_init_tags(q, depth);
+		tags = __blk_queue_init_tags(q, depth, alloc_policy);
 
 		if (!tags)
 			return -ENOMEM;
@@ -344,9 +348,21 @@ int blk_queue_start_tag(struct request_queue *q, struct request *rq)
 	}
 
 	do {
-		tag = find_first_zero_bit(bqt->tag_map, max_depth);
-		if (tag >= max_depth)
-			return 1;
+		if (bqt->alloc_policy == BLK_TAG_ALLOC_FIFO) {
+			tag = find_first_zero_bit(bqt->tag_map, max_depth);
+			if (tag >= max_depth)
+				return 1;
+		} else {
+			int start = bqt->next_tag;
+			int size = min_t(int, bqt->max_depth, max_depth + start);
+			tag = find_next_zero_bit(bqt->tag_map, size, start);
+			if (tag >= size && start + size > bqt->max_depth) {
+				size = start + size - bqt->max_depth;
+				tag = find_first_zero_bit(bqt->tag_map, size);
+			}
+			if (tag >= size)
+				return 1;
+		}
 
 	} while (test_and_set_bit_lock(tag, bqt->tag_map));
 	/*
@@ -354,6 +370,7 @@ int blk_queue_start_tag(struct request_queue *q, struct request *rq)
 	 * See blk_queue_end_tag for details.
 	 */
 
+	bqt->next_tag = (tag + 1) % bqt->max_depth;
 	rq->cmd_flags |= REQ_QUEUED;
 	rq->tag = tag;
 	bqt->tag_index[tag] = rq;
diff --git a/drivers/block/osdblk.c b/drivers/block/osdblk.c
index 79aa179..e229425 100644
--- a/drivers/block/osdblk.c
+++ b/drivers/block/osdblk.c
@@ -423,7 +423,7 @@ static int osdblk_init_disk(struct osdblk_device *osdev)
 	}
 
 	/* switch queue to TCQ mode; allocate tag map */
-	rc = blk_queue_init_tags(q, OSDBLK_MAX_REQ, NULL);
+	rc = blk_queue_init_tags(q, OSDBLK_MAX_REQ, NULL, BLK_TAG_ALLOC_FIFO);
 	if (rc) {
 		blk_cleanup_queue(q);
 		put_disk(disk);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 983aed1..921a8c8 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -290,7 +290,8 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	if (!shost_use_blk_mq(sdev->host) &&
 	    (shost->bqt || shost->hostt->use_blk_tags)) {
 		blk_queue_init_tags(sdev->request_queue,
-				    sdev->host->cmd_per_lun, shost->bqt);
+				    sdev->host->cmd_per_lun, shost->bqt,
+				    shost->hostt->tag_alloc_policy);
 	}
 	scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 92f4b4b..38b095d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -272,7 +272,11 @@ struct blk_queue_tag {
 	int max_depth;			/* what we will send to device */
 	int real_max_depth;		/* what the array can hold */
 	atomic_t refcnt;		/* map can be shared */
+	int alloc_policy;		/* tag allocation policy */
+	int next_tag;			/* next tag */
 };
+#define BLK_TAG_ALLOC_FIFO 0 /* allocate starting from 0 */
+#define BLK_TAG_ALLOC_RR 1 /* allocate starting from last allocated tag */
 
 #define BLK_SCSI_MAX_CMDS	(256)
 #define BLK_SCSI_CMD_PER_LONG	(BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
@@ -1139,11 +1143,11 @@ static inline bool blk_needs_flush_plug(struct task_struct *tsk)
 extern int blk_queue_start_tag(struct request_queue *, struct request *);
 extern struct request *blk_queue_find_tag(struct request_queue *, int);
 extern void blk_queue_end_tag(struct request_queue *, struct request *);
-extern int blk_queue_init_tags(struct request_queue *, int, struct blk_queue_tag *);
+extern int blk_queue_init_tags(struct request_queue *, int, struct blk_queue_tag *, int);
 extern void blk_queue_free_tags(struct request_queue *);
 extern int blk_queue_resize_tags(struct request_queue *, int);
 extern void blk_queue_invalidate_tags(struct request_queue *);
-extern struct blk_queue_tag *blk_init_tags(int);
+extern struct blk_queue_tag *blk_init_tags(int, int);
 extern void blk_free_tags(struct blk_queue_tag *);
 
 static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 019e668..e113c75 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -402,6 +402,9 @@ struct scsi_host_template {
 	 */
 	unsigned char present;
 
+	/* If use block layer to manage tags, this is tag allocation policy */
+	int tag_alloc_policy;
+
 	/*
 	 * Let the block layer assigns tags to all commands.
 	 */
diff --git a/include/scsi/scsi_tcq.h b/include/scsi/scsi_tcq.h
index 9708b28..b27977e 100644
--- a/include/scsi/scsi_tcq.h
+++ b/include/scsi/scsi_tcq.h
@@ -66,7 +66,8 @@ static inline int scsi_init_shared_tag_map(struct Scsi_Host *shost, int depth)
 	 * devices on the shared host (for libata)
 	 */
 	if (!shost->bqt) {
-		shost->bqt = blk_init_tags(depth);
+		shost->bqt = blk_init_tags(depth,
+			shost->hostt->tag_alloc_policy);
 		if (!shost->bqt)
 			return -ENOMEM;
 	}
-- 
1.8.1

[PATCH v3 2/3] blk-mq: add tag allocation policy

[Shaohua Li]

This is the blk-mq part to support tag allocation policy. The default
allocation policy isn't changed (though it's not a strict FIFO). The new
policy is round-robin for libata. But it's a try-best implementation. If
multiple tasks are competing, the tags returned will be mixed (which is
unavoidable even with !mq, as requests from different tasks can be
mixed in queue)

Cc: Jens Axboe <axboe@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 block/blk-mq-tag.c      | 39 ++++++++++++++++++++++++---------------
 block/blk-mq-tag.h      |  4 +++-
 block/blk-mq.c          |  3 ++-
 drivers/scsi/scsi_lib.c |  2 ++
 include/linux/blk-mq.h  |  8 ++++++++
 5 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 60c9d4a..231e1bc 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -140,10 +140,11 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
 	return atomic_read(&hctx->nr_active) < depth;
 }
 
-static int __bt_get_word(struct blk_align_bitmap *bm, unsigned int last_tag)
+static int __bt_get_word(struct blk_align_bitmap *bm, unsigned int last_tag,
+			 bool nowrap)
 {
 	int tag, org_last_tag, end;
-	bool wrap = last_tag != 0;
+	bool wrap = last_tag != 0 && !nowrap;
 
 	org_last_tag = last_tag;
 	end = bm->depth;
@@ -169,6 +170,8 @@ static int __bt_get_word(struct blk_align_bitmap *bm, unsigned int last_tag)
 	return tag;
 }
 
+#define BT_ALLOC_RR(tags) (tags->alloc_policy == BLK_TAG_ALLOC_RR)
+
 /*
  * Straight forward bitmap tag implementation, where each bit is a tag
  * (cleared == free, and set == busy). The small twist is using per-cpu
@@ -181,7 +184,7 @@ static int __bt_get_word(struct blk_align_bitmap *bm, unsigned int last_tag)
  * until the map is exhausted.
  */
 static int __bt_get(struct blk_mq_hw_ctx *hctx, struct blk_mq_bitmap_tags *bt,
-		    unsigned int *tag_cache)
+		    unsigned int *tag_cache, struct blk_mq_tags *tags)
 {
 	unsigned int last_tag, org_last_tag;
 	int index, i, tag;
@@ -193,7 +196,8 @@ static int __bt_get(struct blk_mq_hw_ctx *hctx, struct blk_mq_bitmap_tags *bt,
 	index = TAG_TO_INDEX(bt, last_tag);
 
 	for (i = 0; i < bt->map_nr; i++) {
-		tag = __bt_get_word(&bt->map[index], TAG_TO_BIT(bt, last_tag));
+		tag = __bt_get_word(&bt->map[index], TAG_TO_BIT(bt, last_tag),
+				    BT_ALLOC_RR(tags));
 		if (tag != -1) {
 			tag += (index << bt->bits_per_word);
 			goto done;
@@ -212,7 +216,7 @@ static int __bt_get(struct blk_mq_hw_ctx *hctx, struct blk_mq_bitmap_tags *bt,
 	 * up using the specific cached tag.
 	 */
 done:
-	if (tag == org_last_tag) {
+	if (tag == org_last_tag || unlikely(BT_ALLOC_RR(tags))) {
 		last_tag = tag + 1;
 		if (last_tag >= bt->depth - 1)
 			last_tag = 0;
@@ -241,13 +245,13 @@ static struct bt_wait_state *bt_wait_ptr(struct blk_mq_bitmap_tags *bt,
 static int bt_get(struct blk_mq_alloc_data *data,
 		struct blk_mq_bitmap_tags *bt,
 		struct blk_mq_hw_ctx *hctx,
-		unsigned int *last_tag)
+		unsigned int *last_tag, struct blk_mq_tags *tags)
 {
 	struct bt_wait_state *bs;
 	DEFINE_WAIT(wait);
 	int tag;
 
-	tag = __bt_get(hctx, bt, last_tag);
+	tag = __bt_get(hctx, bt, last_tag, tags);
 	if (tag != -1)
 		return tag;
 
@@ -258,7 +262,7 @@ static int bt_get(struct blk_mq_alloc_data *data,
 	do {
 		prepare_to_wait(&bs->wait, &wait, TASK_UNINTERRUPTIBLE);
 
-		tag = __bt_get(hctx, bt, last_tag);
+		tag = __bt_get(hctx, bt, last_tag, tags);
 		if (tag != -1)
 			break;
 
@@ -273,7 +277,7 @@ static int bt_get(struct blk_mq_alloc_data *data,
 		 * Retry tag allocation after running the hardware queue,
 		 * as running the queue may also have found completions.
 		 */
-		tag = __bt_get(hctx, bt, last_tag);
+		tag = __bt_get(hctx, bt, last_tag, tags);
 		if (tag != -1)
 			break;
 
@@ -304,7 +308,7 @@ static unsigned int __blk_mq_get_tag(struct blk_mq_alloc_data *data)
 	int tag;
 
 	tag = bt_get(data, &data->hctx->tags->bitmap_tags, data->hctx,
-			&data->ctx->last_tag);
+			&data->ctx->last_tag, data->hctx->tags);
 	if (tag >= 0)
 		return tag + data->hctx->tags->nr_reserved_tags;
 
@@ -320,7 +324,8 @@ static unsigned int __blk_mq_get_reserved_tag(struct blk_mq_alloc_data *data)
 		return BLK_MQ_TAG_FAIL;
 	}
 
-	tag = bt_get(data, &data->hctx->tags->breserved_tags, NULL, &zero);
+	tag = bt_get(data, &data->hctx->tags->breserved_tags, NULL, &zero,
+		data->hctx->tags);
 	if (tag < 0)
 		return BLK_MQ_TAG_FAIL;
 
@@ -392,7 +397,8 @@ void blk_mq_put_tag(struct blk_mq_hw_ctx *hctx, unsigned int tag,
 
 		BUG_ON(real_tag >= tags->nr_tags);
 		bt_clear_tag(&tags->bitmap_tags, real_tag);
-		*last_tag = real_tag;
+		if (likely(tags->alloc_policy == BLK_TAG_ALLOC_FIFO))
+			*last_tag = real_tag;
 	} else {
 		BUG_ON(tag >= tags->nr_reserved_tags);
 		bt_clear_tag(&tags->breserved_tags, tag);
@@ -529,10 +535,12 @@ static void bt_free(struct blk_mq_bitmap_tags *bt)
 }
 
 static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct blk_mq_tags *tags,
-						   int node)
+						   int node, int alloc_policy)
 {
 	unsigned int depth = tags->nr_tags - tags->nr_reserved_tags;
 
+	tags->alloc_policy = alloc_policy;
+
 	if (bt_alloc(&tags->bitmap_tags, depth, node, false))
 		goto enomem;
 	if (bt_alloc(&tags->breserved_tags, tags->nr_reserved_tags, node, true))
@@ -546,7 +554,8 @@ static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct blk_mq_tags *tags,
 }
 
 struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
-				     unsigned int reserved_tags, int node)
+				     unsigned int reserved_tags,
+				     int node, int alloc_policy)
 {
 	struct blk_mq_tags *tags;
 
@@ -562,7 +571,7 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
 	tags->nr_tags = total_tags;
 	tags->nr_reserved_tags = reserved_tags;
 
-	return blk_mq_init_bitmap_tags(tags, node);
+	return blk_mq_init_bitmap_tags(tags, node, alloc_policy);
 }
 
 void blk_mq_free_tags(struct blk_mq_tags *tags)
diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h
index a6fa0fc..90767b3 100644
--- a/block/blk-mq-tag.h
+++ b/block/blk-mq-tag.h
@@ -42,10 +42,12 @@ struct blk_mq_tags {
 
 	struct request **rqs;
 	struct list_head page_list;
+
+	int alloc_policy;
 };
 
 
-extern struct blk_mq_tags *blk_mq_init_tags(unsigned int nr_tags, unsigned int reserved_tags, int node);
+extern struct blk_mq_tags *blk_mq_init_tags(unsigned int nr_tags, unsigned int reserved_tags, int node, int alloc_policy);
 extern void blk_mq_free_tags(struct blk_mq_tags *tags);
 
 extern unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2f95747..fffbb83 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1423,7 +1423,8 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct blk_mq_tag_set *set,
 	size_t rq_size, left;
 
 	tags = blk_mq_init_tags(set->queue_depth, set->reserved_tags,
-				set->numa_node);
+				set->numa_node,
+				BLK_MQ_FLAG_TO_ALLOC_POLICY(set->flags));
 	if (!tags)
 		return NULL;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9ea95dd..49ab115 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2188,6 +2188,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 	shost->tag_set.cmd_size = cmd_size;
 	shost->tag_set.numa_node = NUMA_NO_NODE;
 	shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE;
+	shost->tag_set.flags |=
+		BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy);
 	shost->tag_set.driver_data = shost;
 
 	return blk_mq_alloc_tag_set(&shost->tag_set);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 5735e71..623d61d 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -146,6 +146,8 @@ enum {
 	BLK_MQ_F_SG_MERGE	= 1 << 2,
 	BLK_MQ_F_SYSFS_UP	= 1 << 3,
 	BLK_MQ_F_DEFER_ISSUE	= 1 << 4,
+	BLK_MQ_F_ALLOC_POLICY_START_BIT = 8,
+	BLK_MQ_F_ALLOC_POLICY_BITS = 1,
 
 	BLK_MQ_S_STOPPED	= 0,
 	BLK_MQ_S_TAG_ACTIVE	= 1,
@@ -154,6 +156,12 @@ enum {
 
 	BLK_MQ_CPU_WORK_BATCH	= 8,
 };
+#define BLK_MQ_FLAG_TO_ALLOC_POLICY(flags) \
+	((flags >> BLK_MQ_F_ALLOC_POLICY_START_BIT) & \
+		((1 << BLK_MQ_F_ALLOC_POLICY_BITS) - 1))
+#define BLK_ALLOC_POLICY_TO_MQ_FLAG(policy) \
+	((policy & ((1 << BLK_MQ_F_ALLOC_POLICY_BITS) - 1)) \
+		<< BLK_MQ_F_ALLOC_POLICY_START_BIT)
 
 struct request_queue *blk_mq_init_queue(struct blk_mq_tag_set *);
 void blk_mq_finish_init(struct request_queue *q);
-- 
1.8.1

[PATCH v3 3/3] libata: use blk taging

[Shaohua Li]

libata uses its own tag management which is duplication and the
implementation is poor. And if we switch to blk-mq, tag is build-in.
It's time to switch to generic taging.

The SAS driver has its own tag management, and looks we can't directly
map the host controler tag to SATA tag. So I just bypassed the SAS case.

I changed the code/variable name for the tag management of libata to
make it self contained. Only sas will use it. Later if libsas implements
its tag management, the tag management code in libata can be deleted
easily.

Cc: Jens Axboe <axboe@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/ata/libata-core.c | 54 +++++++++++++++++++++++++++++++++++------------
 drivers/ata/libata-scsi.c |  4 +++-
 drivers/ata/libata.h      |  2 +-
 include/linux/libata.h    |  5 +++--
 4 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5c84fb5..695d33d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1525,6 +1525,15 @@ static void ata_qc_complete_internal(struct ata_queued_cmd *qc)
 	complete(waiting);
 }
 
+static bool ata_valid_internal_tag(struct ata_port *ap, struct ata_device *dev,
+				   unsigned int tag)
+{
+	if (!ap->scsi_host)
+		return !test_and_set_bit(tag, &ap->sas_tag_allocated);
+	return !dev->sdev ||
+	       !blk_queue_find_tag(dev->sdev->request_queue, tag);
+}
+
 /**
  *	ata_exec_internal_sg - execute libata internal command
  *	@dev: Device to which the command is sent
@@ -1585,8 +1594,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
 	else
 		tag = 0;
 
-	if (test_and_set_bit(tag, &ap->qc_allocated))
-		BUG();
+	BUG_ON(!ata_valid_internal_tag(ap, dev, tag));
 	qc = __ata_qc_from_tag(ap, tag);
 
 	qc->tag = tag;
@@ -4737,27 +4745,23 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
  *	None.
  */
 
-static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
+static struct ata_queued_cmd *sas_ata_qc_new(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc = NULL;
 	unsigned int max_queue = ap->host->n_tags;
 	unsigned int i, tag;
 
-	/* no command while frozen */
-	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
-		return NULL;
-
-	for (i = 0, tag = ap->last_tag + 1; i < max_queue; i++, tag++) {
+	for (i = 0, tag = ap->sas_last_tag + 1; i < max_queue; i++, tag++) {
 		tag = tag < max_queue ? tag : 0;
 
 		/* the last tag is reserved for internal command. */
 		if (tag == ATA_TAG_INTERNAL)
 			continue;
 
-		if (!test_and_set_bit(tag, &ap->qc_allocated)) {
+		if (!test_and_set_bit(tag, &ap->sas_tag_allocated)) {
 			qc = __ata_qc_from_tag(ap, tag);
 			qc->tag = tag;
-			ap->last_tag = tag;
+			ap->sas_last_tag = tag;
 			break;
 		}
 	}
@@ -4765,6 +4769,24 @@ static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
 	return qc;
 }
 
+static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap, int blktag)
+{
+	struct ata_queued_cmd *qc;
+
+	/* no command while frozen */
+	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
+		return NULL;
+
+	/* SATA will directly use block tag. libsas need its own tag management */
+	if (ap->scsi_host) {
+		qc = __ata_qc_from_tag(ap, blktag);
+		qc->tag = blktag;
+		return qc;
+	}
+
+	return sas_ata_qc_new(ap);
+}
+
 /**
  *	ata_qc_new_init - Request an available ATA command, and initialize it
  *	@dev: Device from whom we request an available command structure
@@ -4773,12 +4795,12 @@ static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
  *	None.
  */
 
-struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev)
+struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int blktag)
 {
 	struct ata_port *ap = dev->link->ap;
 	struct ata_queued_cmd *qc;
 
-	qc = ata_qc_new(ap);
+	qc = ata_qc_new(ap, blktag);
 	if (qc) {
 		qc->scsicmd = NULL;
 		qc->ap = ap;
@@ -4800,6 +4822,12 @@ struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev)
  *	LOCKING:
  *	spin_lock_irqsave(host lock)
  */
+static void sas_ata_qc_free(unsigned int tag, struct ata_port *ap)
+{
+	if (!ap->scsi_host)
+		clear_bit(tag, &ap->sas_tag_allocated);
+}
+
 void ata_qc_free(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap;
@@ -4812,7 +4840,7 @@ void ata_qc_free(struct ata_queued_cmd *qc)
 	tag = qc->tag;
 	if (likely(ata_tag_valid(tag))) {
 		qc->tag = ATA_TAG_POISON;
-		clear_bit(tag, &ap->qc_allocated);
+		sas_ata_qc_free(tag, ap);
 	}
 }
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e364e86..94339c2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -756,7 +756,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
 {
 	struct ata_queued_cmd *qc;
 
-	qc = ata_qc_new_init(dev);
+	qc = ata_qc_new_init(dev, cmd->request->tag);
 	if (qc) {
 		qc->scsicmd = cmd;
 		qc->scsidone = cmd->scsi_done;
@@ -3666,6 +3666,8 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		 */
 		shost->max_host_blocked = 1;
 
+		scsi_init_shared_tag_map(shost, host->n_tags);
+
 		rc = scsi_add_host_with_dma(ap->scsi_host,
 						&ap->tdev, ap->host->dev);
 		if (rc)
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 5f4e0cc..4040513 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -63,7 +63,7 @@ extern struct ata_link *ata_dev_phys_link(struct ata_device *dev);
 extern void ata_force_cbl(struct ata_port *ap);
 extern u64 ata_tf_to_lba(const struct ata_taskfile *tf);
 extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf);
-extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
+extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
 extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 			   u64 block, u32 n_block, unsigned int tf_flags,
 			   unsigned int tag);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2d18241..f234547 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -821,10 +821,10 @@ struct ata_port {
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
 
 	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
-	unsigned long		qc_allocated;
+	unsigned long		sas_tag_allocated; /* for sas tag allocation only */
 	unsigned int		qc_active;
 	int			nr_active_links; /* #links with active qcs */
-	unsigned int		last_tag;	/* track next tag hw expects */
+	unsigned int		sas_last_tag;	/* track next tag hw expects */
 
 	struct ata_link		link;		/* host default link */
 	struct ata_link		*slave_link;	/* see ata_slave_link_init() */
@@ -1344,6 +1344,7 @@ extern struct device_attribute *ata_common_sdev_attrs[];
 	.ioctl			= ata_scsi_ioctl,		\
 	.queuecommand		= ata_scsi_queuecmd,		\
 	.can_queue		= ATA_DEF_QUEUE,		\
+	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,		\
 	.this_id		= ATA_SHT_THIS_ID,		\
 	.cmd_per_lun		= ATA_SHT_CMD_PER_LUN,		\
 	.emulated		= ATA_SHT_EMULATED,		\
-- 
1.8.1

Re: [PATCH v3 3/3] libata: use blk taging

[Christoph Hellwig]

> +static bool ata_valid_internal_tag(struct ata_port *ap, struct ata_device *dev,
> +				   unsigned int tag)
> +{
> +	if (!ap->scsi_host)
> +		return !test_and_set_bit(tag, &ap->sas_tag_allocated);
> +	return !dev->sdev ||
> +	       !blk_queue_find_tag(dev->sdev->request_queue, tag);

How is this supposed to work with blk-mq?

> +}
> +
>  /**
>   *	ata_exec_internal_sg - execute libata internal command
>   *	@dev: Device to which the command is sent
> @@ -1585,8 +1594,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>  	else
>  		tag = 0;
>  
> -	if (test_and_set_bit(tag, &ap->qc_allocated))
> -		BUG();
> +	BUG_ON(!ata_valid_internal_tag(ap, dev, tag));

But given that this is just a single assert we might as well just
remove that whole thing.

> +static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap, int blktag)
> +{
> +	struct ata_queued_cmd *qc;
> +
> +	/* no command while frozen */
> +	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
> +		return NULL;
> +
> +	/* SATA will directly use block tag. libsas need its own tag management */
> +	if (ap->scsi_host) {
> +		qc = __ata_qc_from_tag(ap, blktag);
> +		qc->tag = blktag;
> +		return qc;
> +	}
> +
> +	return sas_ata_qc_new(ap);

Why don't you merge tis into ata_qc_new_init?

> +static void sas_ata_qc_free(unsigned int tag, struct ata_port *ap)
> +{
> +	if (!ap->scsi_host)

Given the sas_ name I'd move the check into the caller.

That beeing said I wonder if you shouldn't do all this at a much
higher level.

__ata_scsi_queuecmd has a two callers, one for sata, and one for sas.
If you pass in the tag from that level the low lever code won't
need any branches for sas vs libata.  The only downside would be
that you now consume a tag for simulated command on SAS, but as that's
already done for SATA it can't be that bad.

> @@ -3666,6 +3666,8 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  		 */
>  		shost->max_host_blocked = 1;
>  
> +		scsi_init_shared_tag_map(shost, host->n_tags);
> +

You need to check the return value here.

Re: [PATCH v3 3/3] libata: use blk taging

[Shaohua Li]

On Fri, Jan 16, 2015 at 12:23:44AM -0800, Christoph Hellwig wrote:
> > +static bool ata_valid_internal_tag(struct ata_port *ap, struct ata_device *dev,
> > +				   unsigned int tag)
> > +{
> > +	if (!ap->scsi_host)
> > +		return !test_and_set_bit(tag, &ap->sas_tag_allocated);
> > +	return !dev->sdev ||
> > +	       !blk_queue_find_tag(dev->sdev->request_queue, tag);
> 
> How is this supposed to work with blk-mq?
> 
> > +}
> > +
> >  /**
> >   *	ata_exec_internal_sg - execute libata internal command
> >   *	@dev: Device to which the command is sent
> > @@ -1585,8 +1594,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
> >  	else
> >  		tag = 0;
> >  
> > -	if (test_and_set_bit(tag, &ap->qc_allocated))
> > -		BUG();
> > +	BUG_ON(!ata_valid_internal_tag(ap, dev, tag));
> 
> But given that this is just a single assert we might as well just
> remove that whole thing.

Ok, I delete it
 
> > +static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap, int blktag)
> > +{
> > +	struct ata_queued_cmd *qc;
> > +
> > +	/* no command while frozen */
> > +	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
> > +		return NULL;
> > +
> > +	/* SATA will directly use block tag. libsas need its own tag management */
> > +	if (ap->scsi_host) {
> > +		qc = __ata_qc_from_tag(ap, blktag);
> > +		qc->tag = blktag;
> > +		return qc;
> > +	}
> > +
> > +	return sas_ata_qc_new(ap);
> 
> Why don't you merge tis into ata_qc_new_init?
> 
> > +static void sas_ata_qc_free(unsigned int tag, struct ata_port *ap)
> > +{
> > +	if (!ap->scsi_host)
> 
> Given the sas_ name I'd move the check into the caller.
> 
> That beeing said I wonder if you shouldn't do all this at a much
> higher level.
> 
> __ata_scsi_queuecmd has a two callers, one for sata, and one for sas.
> If you pass in the tag from that level the low lever code won't
> need any branches for sas vs libata.  The only downside would be
> that you now consume a tag for simulated command on SAS, but as that's
> already done for SATA it can't be that bad.

I can do this in __ata_scsi_queuecmd, but don't see a good place to free
the tag, which really should be done at scsi_done. The whole thing
should be done at a higher level. I moved the ata tag allocation code to
libata-scsi.c to make it clearer.


>From 125d41a001b8555fac42ea5b35dae45049655ab0 Mon Sep 17 00:00:00 2001
Message-Id: <125d41a001b8555fac42ea5b35dae45049655ab0.1421446044.git.shli@fb.com>
In-Reply-To: <81b8ef84122544faa1f37a404cf2e7c71791d961.1421446044.git.shli@fb.com>
References: <81b8ef84122544faa1f37a404cf2e7c71791d961.1421446044.git.shli@fb.com>
From: Shaohua Li <shli@fb.com>
Date: Thu, 18 Dec 2014 09:04:25 -0800
Subject: [PATCH 3/3] libata: use blk taging

libata uses its own tag management which is duplication and the
implementation is poor. And if we switch to blk-mq, tag is build-in.
It's time to switch to generic taging.

The SAS driver has its own tag management, and looks we can't directly
map the host controler tag to SATA tag. So I just bypassed the SAS case.

I changed the code/variable name for the tag management of libata to
make it self contained. Only sas will use it. Later if libsas implements
its tag management, the tag management code in libata can be deleted
easily.

Cc: Jens Axboe <axboe@fb.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/ata/libata-core.c | 67 +++++++++++++----------------------------------
 drivers/ata/libata-scsi.c | 30 ++++++++++++++++++++-
 drivers/ata/libata.h      |  4 ++-
 include/linux/libata.h    |  5 ++--
 4 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5c84fb5..d626605 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1585,8 +1585,6 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
 	else
 		tag = 0;
 
-	if (test_and_set_bit(tag, &ap->qc_allocated))
-		BUG();
 	qc = __ata_qc_from_tag(ap, tag);
 
 	qc->tag = tag;
@@ -4726,66 +4724,36 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
 }
 
 /**
- *	ata_qc_new - Request an available ATA command, for queueing
- *	@ap: target port
- *
- *	Some ATA host controllers may implement a queue depth which is less
- *	than ATA_MAX_QUEUE. So we shouldn't allocate a tag which is beyond
- *	the hardware limitation.
+ *	ata_qc_new_init - Request an available ATA command, and initialize it
+ *	@dev: Device from whom we request an available command structure
  *
  *	LOCKING:
  *	None.
  */
 
-static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
+struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag)
 {
-	struct ata_queued_cmd *qc = NULL;
-	unsigned int max_queue = ap->host->n_tags;
-	unsigned int i, tag;
+	struct ata_port *ap = dev->link->ap;
+	struct ata_queued_cmd *qc;
 
 	/* no command while frozen */
 	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
 		return NULL;
 
-	for (i = 0, tag = ap->last_tag + 1; i < max_queue; i++, tag++) {
-		tag = tag < max_queue ? tag : 0;
-
-		/* the last tag is reserved for internal command. */
-		if (tag == ATA_TAG_INTERNAL)
-			continue;
-
-		if (!test_and_set_bit(tag, &ap->qc_allocated)) {
-			qc = __ata_qc_from_tag(ap, tag);
-			qc->tag = tag;
-			ap->last_tag = tag;
-			break;
-		}
+	/* libsas case */
+	if (!ap->scsi_host) {
+		tag = ata_sas_allocate_tag(ap);
+		if (tag < 0)
+			return NULL;
 	}
 
-	return qc;
-}
-
-/**
- *	ata_qc_new_init - Request an available ATA command, and initialize it
- *	@dev: Device from whom we request an available command structure
- *
- *	LOCKING:
- *	None.
- */
-
-struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev)
-{
-	struct ata_port *ap = dev->link->ap;
-	struct ata_queued_cmd *qc;
-
-	qc = ata_qc_new(ap);
-	if (qc) {
-		qc->scsicmd = NULL;
-		qc->ap = ap;
-		qc->dev = dev;
+	qc = __ata_qc_from_tag(ap, tag);
+	qc->tag = tag;
+	qc->scsicmd = NULL;
+	qc->ap = ap;
+	qc->dev = dev;
 
-		ata_qc_reinit(qc);
-	}
+	ata_qc_reinit(qc);
 
 	return qc;
 }
@@ -4812,7 +4780,8 @@ void ata_qc_free(struct ata_queued_cmd *qc)
 	tag = qc->tag;
 	if (likely(ata_tag_valid(tag))) {
 		qc->tag = ATA_TAG_POISON;
-		clear_bit(tag, &ap->qc_allocated);
+		if (!ap->scsi_host)
+			ata_sas_free_tag(tag, ap);
 	}
 }
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e364e86..59c9d72 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -756,7 +756,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
 {
 	struct ata_queued_cmd *qc;
 
-	qc = ata_qc_new_init(dev);
+	qc = ata_qc_new_init(dev, cmd->request->tag);
 	if (qc) {
 		qc->scsicmd = cmd;
 		qc->scsidone = cmd->scsi_done;
@@ -3666,6 +3666,9 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		 */
 		shost->max_host_blocked = 1;
 
+		if (scsi_init_shared_tag_map(shost, host->n_tags))
+			goto err_add;
+
 		rc = scsi_add_host_with_dma(ap->scsi_host,
 						&ap->tdev, ap->host->dev);
 		if (rc)
@@ -4228,3 +4231,28 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
 	return rc;
 }
 EXPORT_SYMBOL_GPL(ata_sas_queuecmd);
+
+int ata_sas_allocate_tag(struct ata_port *ap)
+{
+	unsigned int max_queue = ap->host->n_tags;
+	unsigned int i, tag;
+
+	for (i = 0, tag = ap->sas_last_tag + 1; i < max_queue; i++, tag++) {
+		tag = tag < max_queue ? tag : 0;
+
+		/* the last tag is reserved for internal command. */
+		if (tag == ATA_TAG_INTERNAL)
+			continue;
+
+		if (!test_and_set_bit(tag, &ap->sas_tag_allocated)) {
+			ap->sas_last_tag = tag;
+			return tag;
+		}
+	}
+	return -1;
+}
+
+void ata_sas_free_tag(unsigned int tag, struct ata_port *ap)
+{
+	clear_bit(tag, &ap->sas_tag_allocated);
+}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 5f4e0cc..8c491cd 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -63,7 +63,7 @@ extern struct ata_link *ata_dev_phys_link(struct ata_device *dev);
 extern void ata_force_cbl(struct ata_port *ap);
 extern u64 ata_tf_to_lba(const struct ata_taskfile *tf);
 extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf);
-extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
+extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
 extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 			   u64 block, u32 n_block, unsigned int tf_flags,
 			   unsigned int tag);
@@ -145,6 +145,8 @@ extern void ata_scsi_dev_rescan(struct work_struct *work);
 extern int ata_bus_probe(struct ata_port *ap);
 extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
 			      unsigned int id, u64 lun);
+int ata_sas_allocate_tag(struct ata_port *ap);
+void ata_sas_free_tag(unsigned int tag, struct ata_port *ap);
 
 
 /* libata-eh.c */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2d18241..f234547 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -821,10 +821,10 @@ struct ata_port {
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
 
 	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
-	unsigned long		qc_allocated;
+	unsigned long		sas_tag_allocated; /* for sas tag allocation only */
 	unsigned int		qc_active;
 	int			nr_active_links; /* #links with active qcs */
-	unsigned int		last_tag;	/* track next tag hw expects */
+	unsigned int		sas_last_tag;	/* track next tag hw expects */
 
 	struct ata_link		link;		/* host default link */
 	struct ata_link		*slave_link;	/* see ata_slave_link_init() */
@@ -1344,6 +1344,7 @@ extern struct device_attribute *ata_common_sdev_attrs[];
 	.ioctl			= ata_scsi_ioctl,		\
 	.queuecommand		= ata_scsi_queuecmd,		\
 	.can_queue		= ATA_DEF_QUEUE,		\
+	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,		\
 	.this_id		= ATA_SHT_THIS_ID,		\
 	.cmd_per_lun		= ATA_SHT_CMD_PER_LUN,		\
 	.emulated		= ATA_SHT_EMULATED,		\
-- 
1.8.1

Re: [PATCH v3 3/3] libata: use blk taging

[Shaohua Li]

Hi Tejun,
are you ok to pick up these patches?

Thanks,
Shaohua

On Fri, Jan 16, 2015 at 02:16:00PM -0800, Shaohua Li wrote:
> On Fri, Jan 16, 2015 at 12:23:44AM -0800, Christoph Hellwig wrote:
> > > +static bool ata_valid_internal_tag(struct ata_port *ap, struct ata_device *dev,
> > > +				   unsigned int tag)
> > > +{
> > > +	if (!ap->scsi_host)
> > > +		return !test_and_set_bit(tag, &ap->sas_tag_allocated);
> > > +	return !dev->sdev ||
> > > +	       !blk_queue_find_tag(dev->sdev->request_queue, tag);
> > 
> > How is this supposed to work with blk-mq?
> > 
> > > +}
> > > +
> > >  /**
> > >   *	ata_exec_internal_sg - execute libata internal command
> > >   *	@dev: Device to which the command is sent
> > > @@ -1585,8 +1594,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
> > >  	else
> > >  		tag = 0;
> > >  
> > > -	if (test_and_set_bit(tag, &ap->qc_allocated))
> > > -		BUG();
> > > +	BUG_ON(!ata_valid_internal_tag(ap, dev, tag));
> > 
> > But given that this is just a single assert we might as well just
> > remove that whole thing.
> 
> Ok, I delete it
>  
> > > +static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap, int blktag)
> > > +{
> > > +	struct ata_queued_cmd *qc;
> > > +
> > > +	/* no command while frozen */
> > > +	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
> > > +		return NULL;
> > > +
> > > +	/* SATA will directly use block tag. libsas need its own tag management */
> > > +	if (ap->scsi_host) {
> > > +		qc = __ata_qc_from_tag(ap, blktag);
> > > +		qc->tag = blktag;
> > > +		return qc;
> > > +	}
> > > +
> > > +	return sas_ata_qc_new(ap);
> > 
> > Why don't you merge tis into ata_qc_new_init?
> > 
> > > +static void sas_ata_qc_free(unsigned int tag, struct ata_port *ap)
> > > +{
> > > +	if (!ap->scsi_host)
> > 
> > Given the sas_ name I'd move the check into the caller.
> > 
> > That beeing said I wonder if you shouldn't do all this at a much
> > higher level.
> > 
> > __ata_scsi_queuecmd has a two callers, one for sata, and one for sas.
> > If you pass in the tag from that level the low lever code won't
> > need any branches for sas vs libata.  The only downside would be
> > that you now consume a tag for simulated command on SAS, but as that's
> > already done for SATA it can't be that bad.
> 
> I can do this in __ata_scsi_queuecmd, but don't see a good place to free
> the tag, which really should be done at scsi_done. The whole thing
> should be done at a higher level. I moved the ata tag allocation code to
> libata-scsi.c to make it clearer.
> 
> 
> From 125d41a001b8555fac42ea5b35dae45049655ab0 Mon Sep 17 00:00:00 2001
> Message-Id: <125d41a001b8555fac42ea5b35dae45049655ab0.1421446044.git.shli@fb.com>
> In-Reply-To: <81b8ef84122544faa1f37a404cf2e7c71791d961.1421446044.git.shli@fb.com>
> References: <81b8ef84122544faa1f37a404cf2e7c71791d961.1421446044.git.shli@fb.com>
> From: Shaohua Li <shli@fb.com>
> Date: Thu, 18 Dec 2014 09:04:25 -0800
> Subject: [PATCH 3/3] libata: use blk taging
> 
> libata uses its own tag management which is duplication and the
> implementation is poor. And if we switch to blk-mq, tag is build-in.
> It's time to switch to generic taging.
> 
> The SAS driver has its own tag management, and looks we can't directly
> map the host controler tag to SATA tag. So I just bypassed the SAS case.
> 
> I changed the code/variable name for the tag management of libata to
> make it self contained. Only sas will use it. Later if libsas implements
> its tag management, the tag management code in libata can be deleted
> easily.
> 
> Cc: Jens Axboe <axboe@fb.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/ata/libata-core.c | 67 +++++++++++++----------------------------------
>  drivers/ata/libata-scsi.c | 30 ++++++++++++++++++++-
>  drivers/ata/libata.h      |  4 ++-
>  include/linux/libata.h    |  5 ++--
>  4 files changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 5c84fb5..d626605 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1585,8 +1585,6 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>  	else
>  		tag = 0;
>  
> -	if (test_and_set_bit(tag, &ap->qc_allocated))
> -		BUG();
>  	qc = __ata_qc_from_tag(ap, tag);
>  
>  	qc->tag = tag;
> @@ -4726,66 +4724,36 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
>  }
>  
>  /**
> - *	ata_qc_new - Request an available ATA command, for queueing
> - *	@ap: target port
> - *
> - *	Some ATA host controllers may implement a queue depth which is less
> - *	than ATA_MAX_QUEUE. So we shouldn't allocate a tag which is beyond
> - *	the hardware limitation.
> + *	ata_qc_new_init - Request an available ATA command, and initialize it
> + *	@dev: Device from whom we request an available command structure
>   *
>   *	LOCKING:
>   *	None.
>   */
>  
> -static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
> +struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag)
>  {
> -	struct ata_queued_cmd *qc = NULL;
> -	unsigned int max_queue = ap->host->n_tags;
> -	unsigned int i, tag;
> +	struct ata_port *ap = dev->link->ap;
> +	struct ata_queued_cmd *qc;
>  
>  	/* no command while frozen */
>  	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
>  		return NULL;
>  
> -	for (i = 0, tag = ap->last_tag + 1; i < max_queue; i++, tag++) {
> -		tag = tag < max_queue ? tag : 0;
> -
> -		/* the last tag is reserved for internal command. */
> -		if (tag == ATA_TAG_INTERNAL)
> -			continue;
> -
> -		if (!test_and_set_bit(tag, &ap->qc_allocated)) {
> -			qc = __ata_qc_from_tag(ap, tag);
> -			qc->tag = tag;
> -			ap->last_tag = tag;
> -			break;
> -		}
> +	/* libsas case */
> +	if (!ap->scsi_host) {
> +		tag = ata_sas_allocate_tag(ap);
> +		if (tag < 0)
> +			return NULL;
>  	}
>  
> -	return qc;
> -}
> -
> -/**
> - *	ata_qc_new_init - Request an available ATA command, and initialize it
> - *	@dev: Device from whom we request an available command structure
> - *
> - *	LOCKING:
> - *	None.
> - */
> -
> -struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev)
> -{
> -	struct ata_port *ap = dev->link->ap;
> -	struct ata_queued_cmd *qc;
> -
> -	qc = ata_qc_new(ap);
> -	if (qc) {
> -		qc->scsicmd = NULL;
> -		qc->ap = ap;
> -		qc->dev = dev;
> +	qc = __ata_qc_from_tag(ap, tag);
> +	qc->tag = tag;
> +	qc->scsicmd = NULL;
> +	qc->ap = ap;
> +	qc->dev = dev;
>  
> -		ata_qc_reinit(qc);
> -	}
> +	ata_qc_reinit(qc);
>  
>  	return qc;
>  }
> @@ -4812,7 +4780,8 @@ void ata_qc_free(struct ata_queued_cmd *qc)
>  	tag = qc->tag;
>  	if (likely(ata_tag_valid(tag))) {
>  		qc->tag = ATA_TAG_POISON;
> -		clear_bit(tag, &ap->qc_allocated);
> +		if (!ap->scsi_host)
> +			ata_sas_free_tag(tag, ap);
>  	}
>  }
>  
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index e364e86..59c9d72 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -756,7 +756,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev,
>  {
>  	struct ata_queued_cmd *qc;
>  
> -	qc = ata_qc_new_init(dev);
> +	qc = ata_qc_new_init(dev, cmd->request->tag);
>  	if (qc) {
>  		qc->scsicmd = cmd;
>  		qc->scsidone = cmd->scsi_done;
> @@ -3666,6 +3666,9 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  		 */
>  		shost->max_host_blocked = 1;
>  
> +		if (scsi_init_shared_tag_map(shost, host->n_tags))
> +			goto err_add;
> +
>  		rc = scsi_add_host_with_dma(ap->scsi_host,
>  						&ap->tdev, ap->host->dev);
>  		if (rc)
> @@ -4228,3 +4231,28 @@ int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
>  	return rc;
>  }
>  EXPORT_SYMBOL_GPL(ata_sas_queuecmd);
> +
> +int ata_sas_allocate_tag(struct ata_port *ap)
> +{
> +	unsigned int max_queue = ap->host->n_tags;
> +	unsigned int i, tag;
> +
> +	for (i = 0, tag = ap->sas_last_tag + 1; i < max_queue; i++, tag++) {
> +		tag = tag < max_queue ? tag : 0;
> +
> +		/* the last tag is reserved for internal command. */
> +		if (tag == ATA_TAG_INTERNAL)
> +			continue;
> +
> +		if (!test_and_set_bit(tag, &ap->sas_tag_allocated)) {
> +			ap->sas_last_tag = tag;
> +			return tag;
> +		}
> +	}
> +	return -1;
> +}
> +
> +void ata_sas_free_tag(unsigned int tag, struct ata_port *ap)
> +{
> +	clear_bit(tag, &ap->sas_tag_allocated);
> +}
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 5f4e0cc..8c491cd 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -63,7 +63,7 @@ extern struct ata_link *ata_dev_phys_link(struct ata_device *dev);
>  extern void ata_force_cbl(struct ata_port *ap);
>  extern u64 ata_tf_to_lba(const struct ata_taskfile *tf);
>  extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf);
> -extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
> +extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
>  extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
>  			   u64 block, u32 n_block, unsigned int tf_flags,
>  			   unsigned int tag);
> @@ -145,6 +145,8 @@ extern void ata_scsi_dev_rescan(struct work_struct *work);
>  extern int ata_bus_probe(struct ata_port *ap);
>  extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
>  			      unsigned int id, u64 lun);
> +int ata_sas_allocate_tag(struct ata_port *ap);
> +void ata_sas_free_tag(unsigned int tag, struct ata_port *ap);
>  
>  
>  /* libata-eh.c */
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 2d18241..f234547 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -821,10 +821,10 @@ struct ata_port {
>  	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
>  
>  	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
> -	unsigned long		qc_allocated;
> +	unsigned long		sas_tag_allocated; /* for sas tag allocation only */
>  	unsigned int		qc_active;
>  	int			nr_active_links; /* #links with active qcs */
> -	unsigned int		last_tag;	/* track next tag hw expects */
> +	unsigned int		sas_last_tag;	/* track next tag hw expects */
>  
>  	struct ata_link		link;		/* host default link */
>  	struct ata_link		*slave_link;	/* see ata_slave_link_init() */
> @@ -1344,6 +1344,7 @@ extern struct device_attribute *ata_common_sdev_attrs[];
>  	.ioctl			= ata_scsi_ioctl,		\
>  	.queuecommand		= ata_scsi_queuecmd,		\
>  	.can_queue		= ATA_DEF_QUEUE,		\
> +	.tag_alloc_policy	= BLK_TAG_ALLOC_RR,		\
>  	.this_id		= ATA_SHT_THIS_ID,		\
>  	.cmd_per_lun		= ATA_SHT_CMD_PER_LUN,		\
>  	.emulated		= ATA_SHT_EMULATED,		\
> -- 
> 1.8.1
> 

Re: [PATCH v3 3/3] libata: use blk taging

[Tejun Heo]

On Thu, Jan 22, 2015 at 01:12:20PM -0800, Shaohua Li wrote:
> Hi Tejun,
> are you ok to pick up these patches?

Yeah, looks fine from libata side.  Please feel free to add

 Acked-by: Tejun Heo <tj@kernel.org>

and route as you see fit.  If these can go through libata, please let
me know.

Thanks.

-- 
tejun

Re: [PATCH v3 3/3] libata: use blk taging

[Jens Axboe]

On 01/23/2015 01:13 PM, Tejun Heo wrote:
> On Thu, Jan 22, 2015 at 01:12:20PM -0800, Shaohua Li wrote:
>> Hi Tejun,
>> are you ok to pick up these patches?
> 
> Yeah, looks fine from libata side.  Please feel free to add
> 
>  Acked-by: Tejun Heo <tj@kernel.org>
> 
> and route as you see fit.  If these can go through libata, please let
> me know.

I'll just take it through the block tree, there's the highest risk of
conflict there with the tagging changes.

-- 
Jens Axboe