On Fri, Jul 11, 2014 at 06:33:46PM +0200, Bartlomiej Zolnierkiewicz wrote: > On Friday, July 11, 2014 06:28:33 PM Bartlomiej Zolnierkiewicz wrote: > > > + max_queue = ap->scsi_host->can_queue; > > > + WARN_ON_ONCE(max_queue > ATA_MAX_QUEUE); > > > > Why not handle this properly by just doing what ata_dev_config_ncq() does and > > using > > > > min(ap->scsi_host->can_queue, ATA_MAX_QUEUE - 1); > > actually this should be > > min(ap->scsi_host->can_queue, ATA_MAX_QUEUE); > > in this function > > > for obtaining max_queue? Actually the ap->scsi_host->can_queue is the max queue supported by the hardware. It should be a bug if it is greater than ATA_MAX_QUEUE. So we don't need to use min() to get the max queue here. As pointed out by Tejun, it makes no sense to do the sanity check for each qc requesting, so I will move the WARN_ON_ONCE() to ata_host_register(). I have a pending patch to free the command tag reserved by the internal command. In that patch the above code in ata_dev_config_ncq() will change to: hdepth = ap->scsi_host->can_queue; Thanks, Kevin
Attachment:
pgp1WBURU8CnE.pgp
Description: PGP signature
