Re: [PATCH RFC v2 03/18] scsi: core: Implement reserved command handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/15/22 09:35, John Garry wrote:
On 15/06/2022 00:43, Damien Le Moal wrote:
On 6/15/22 03:20, Bart Van Assche wrote:
On 6/13/22 00:01, Damien Le Moal wrote:
On 6/9/22 19:29, John Garry wrote:
+    /*
+     * This determines how many commands the HBA will set aside
+     * for internal commands. This number will be added to
+     * @can_queue to calcumate the maximum number of simultaneous

s/calcumate/calculate

But this is weird. For SATA, can_queue is 32. Having reserved commands,
that number needs to stay the same. We cannot have more than 32 tags.
I think keeping can_queue as the max queue depth with at most
nr_reserved_cmds tags reserved is better.

+     * commands sent to the host.
+     */
+    int nr_reserved_cmds;

+1 for Damien's request. I also prefer to keep can_queue as the maximum
queue depth, whether or not nr_reserved_cmds has been set.

For non SATA drives, I still think that is a good idea. However, for SATA,
we always have the internal tag command that is special. With John's
change, it would have to be reserved but that means we are down to 31 max
QD,

My intention is to keep regular tag depth at 32 for SATA. We add an extra tag as a reserved tag. Indeed, this is called a 'tag', but it's just really the placeholder for what will be the ATA_TAG_INTERNAL request.

About how we set scsi_host.can_queue, in this series we set .can_queue as max regular tags, and the handling is as follows:

scsi_mq_setup_tags():
tag_set->queue_depth = shost->can_queue + shost->nr_reserved_cmds
tag_set->reserved_tags = shost->nr_reserved_cmds

So we honour the rule that blk_mq_tag_set.queue_depth is the total tag depth, including reserved.

Incidentally I think Christoph prefers to keep .can_queue at total max tags including reserved: https://lore.kernel.org/linux-scsi/337339b7-6f4a-a25c-f11c-7f701b42d6a8@xxxxxxx/

so going backward several years... That internal tag for ATA does not
need to be reserved since this command is always used when the drive is
idle and no other NCQ commands are on-going.

So do you mean that ATA_TAG_INTERNAL qc is used for other commands apart from internal commands?

Well.

The problem is that 'ATA_TAG_INTERNAL' currently is overloaded to
a) signal internal commands
b) 'magic' tag when looking up commands

My proposal would be to separate these use-cases, and use a flag (eg ATA_QCFLAG_INTERNAL) to determine internal commands.

The we'll be needing an internal tag-lookup map
(NCQ tag -> blk-mq tag) for ata_qc_from_tag() to retrieve the command corresponding to a driver tag.

I guess we'll need that anyway with libsas, as there we're working with a budget tag which has no relationship with the NCQ tag whatsoever ...

But with that we can kill 'ATA_TAG_INTERNAL', and let the driver figure out how to allocate internal tags.

Cheers,

Hannes
--
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@xxxxxxx			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux