On Mon, Jul 07, 2014 at 09:49:45AM -0400, Tejun Heo wrote: > Hello, > > On Sun, Jul 06, 2014 at 02:28:40PM +0800, Kevin Hao wrote: > > +int ata_host_set_queue_depth(struct ata_host *host, unsigned int queue_depth) > > Hmmm... is there a reason we're doing this separately when the same > information is available from scsi_host_template->can_queue from > ata_host_register()? This was my first thought. But given that scsi_host_template are the parameters passed to the scsi layer, it seems a bit weird to set the ata_host based a value in that. Anyway if we decide to use can_queue, how about directly use it in ata_qc_new(), just like the following codes? diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 8f3043165048..4792fea79acf 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4728,14 +4728,17 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words) static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap) { struct ata_queued_cmd *qc = NULL; - unsigned int i, tag; + unsigned int i, tag, max_queue; + + max_queue = ap->scsi_host->can_queue; + WARN_ON_ONCE(max_queue > ATA_MAX_QUEUE); /* no command while frozen */ if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) return NULL; - for (i = 0; i < ATA_MAX_QUEUE; i++) { - tag = (i + ap->last_tag + 1) % ATA_MAX_QUEUE; + 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) Thanks, Kevin
Attachment:
pgpZmnkaHxEKr.pgp
Description: PGP signature