Re: Question: request tag usage

[Hannes Reinecke <hare@suse.de>]

On 09/26/2014 12:12 PM, James Bottomley wrote:
> On Fri, 2014-09-26 at 10:20 +0200, Hannes Reinecke wrote:
>> On 09/26/2014 10:03 AM, Christoph Hellwig wrote:
>>> On Fri, Sep 26, 2014 at 08:29:29AM +0200, Hannes Reinecke wrote:
>>>> Hi Christoph,
>>>>
>>>> as discussed it would make sense to use the request->tag in eg
>>>> scmd_printk() to identify the command.
>>>> Which I duly did, only to figure out that the tag is always '-1', ie
>>>> tagging is not in use.
>>>> (Which is okay from the SCSI side, seeing the TCQ is basically a
>>>> SCSI parallel thing).
>>>
>>> tag are still a live part of SAM for every transport, they've only
>>> been renamed to "command identifier" in SAM-4 to confuse everyone.
>>>
>>>> Looking closer I found plenty of code for handling tags in the block
>>>> layer (and the blk-mq stuff, of course), but virtually none of the
>>>> non-SPI driver seems to be using them.
>>>
>>> A quick grep for scsi_activate_tcq disagrees with you.
>>>
>> Yeah, I've noticed after I've written the mail.
>> However, main point still stands: using 'tag' to identify commands
>> is pointless if not all of the LLDDs use tagging ...
> 
> Every non parallel LLD uses tagging; they all use the network request
> response model, so they can no longer hold the bus until they get an
> answer (which was how untagged commands work in SPI), so for most of the
> transports, untagged commands aren't legal.  Of course, some drivers
> roll their own tags instead of using the block ones.
> 
Hmm, I probably will get shot for this, but we _could_ do something
like this:

diff --git a/block/blk-core.c b/block/blk-core.c
index bf930f4..91570d2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2347,7 +2347,8 @@ void blk_start_request(struct request *req)
        req->resid_len = blk_rq_bytes(req);
        if (unlikely(blk_bidi_rq(req)))
                req->next_rq->resid_len = blk_rq_bytes(req->next_rq);
-
+       if (!blk_queue_tagged(req->q))
+               req->tag = (++req->q->last_tag) % req->q->nr_requests;
        BUG_ON(test_bit(REQ_ATOM_COMPLETE, &req->atomic_flags));
        blk_add_timer(req);
 }
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 518b465..e8ce575 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -418,6 +418,7 @@ struct request_queue {

        struct blk_queue_tag    *queue_tags;
        struct list_head        tag_busy_list;
+       unsigned int            last_tag;

        unsigned int            nr_sorted;
        unsigned int            in_flight[2];

I know, I know. But it would give us a rather nice command identifier.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html