* [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() @ 2014-07-22 14:50 Mike Qiu 2014-07-22 15:42 ` Tejun Heo 0 siblings, 1 reply; 4+ messages in thread From: Mike Qiu @ 2014-07-22 14:50 UTC (permalink / raw) To: linux-kernel, linux-ide; +Cc: tj, haokexin, Mike Qiu The can_queue in scsi_host can be more than ATA_MAX_QUEUE (32), for example, in ipr, it can be 100 or more. Also, some drivers, like ipr driver, haven't filled the field scsi_host in ata_port, and will lead a call trace, so add check for that. Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com> --- drivers/ata/libata-core.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 259d879..a5b9c70 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4734,7 +4734,10 @@ static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap) struct ata_queued_cmd *qc = NULL; unsigned int i, tag, max_queue; - max_queue = ap->scsi_host->can_queue; + if (ap->scsi_host && ap->scsi_host->can_queue <= ATA_MAX_QUEUE) + max_queue = ap->scsi_host->can_queue; + else + max_queue = ATA_MAX_QUEUE; /* no command while frozen */ if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) @@ -6109,16 +6112,6 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; - /* - * The max queue supported by hardware must not be greater than - * ATA_MAX_QUEUE. - */ - if (sht->can_queue > ATA_MAX_QUEUE) { - dev_err(host->dev, "BUG: the hardware max queue is too large\n"); - WARN_ON(1); - return -EINVAL; - } - /* host must have been started */ if (!(host->flags & ATA_HOST_STARTED)) { dev_err(host->dev, "BUG: trying to register unstarted host\n"); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() 2014-07-22 14:50 [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() Mike Qiu @ 2014-07-22 15:42 ` Tejun Heo 2014-07-23 2:10 ` Mike Qiu 0 siblings, 1 reply; 4+ messages in thread From: Tejun Heo @ 2014-07-22 15:42 UTC (permalink / raw) To: Mike Qiu; +Cc: linux-kernel, linux-ide, haokexin, Dan Williams Hello, (cc'ing Dan) On Tue, Jul 22, 2014 at 10:50:19AM -0400, Mike Qiu wrote: > The can_queue in scsi_host can be more than ATA_MAX_QUEUE (32), > for example, in ipr, it can be 100 or more. > > Also, some drivers, like ipr driver, haven't filled the field > scsi_host in ata_port, and will lead a call trace, so add > check for that. > > Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com> > --- > drivers/ata/libata-core.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 259d879..a5b9c70 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -4734,7 +4734,10 @@ static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap) > struct ata_queued_cmd *qc = NULL; > unsigned int i, tag, max_queue; > > - max_queue = ap->scsi_host->can_queue; > + if (ap->scsi_host && ap->scsi_host->can_queue <= ATA_MAX_QUEUE) > + max_queue = ap->scsi_host->can_queue; > + else > + max_queue = ATA_MAX_QUEUE; > > /* no command while frozen */ > if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) > @@ -6109,16 +6112,6 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) > { > int i, rc; > > - /* > - * The max queue supported by hardware must not be greater than > - * ATA_MAX_QUEUE. > - */ > - if (sht->can_queue > ATA_MAX_QUEUE) { > - dev_err(host->dev, "BUG: the hardware max queue is too large\n"); > - WARN_ON(1); > - return -EINVAL; > - } > - So, ummm, I really don't like that we're adding the conditionals to the hot path (yeah, its implementation is slow but still). Maybe we need to store the chosen queue depth after all? Dan? Thanks. -- tejun ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() 2014-07-22 15:42 ` Tejun Heo @ 2014-07-23 2:10 ` Mike Qiu 2014-07-25 17:01 ` Dan Williams 0 siblings, 1 reply; 4+ messages in thread From: Mike Qiu @ 2014-07-23 2:10 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-kernel, linux-ide, haokexin, Dan Williams On 07/22/2014 11:42 PM, Tejun Heo wrote: > Hello, > > (cc'ing Dan) > > On Tue, Jul 22, 2014 at 10:50:19AM -0400, Mike Qiu wrote: >> The can_queue in scsi_host can be more than ATA_MAX_QUEUE (32), >> for example, in ipr, it can be 100 or more. >> >> Also, some drivers, like ipr driver, haven't filled the field >> scsi_host in ata_port, and will lead a call trace, so add >> check for that. >> >> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com> >> --- >> drivers/ata/libata-core.c | 15 ++++----------- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c >> index 259d879..a5b9c70 100644 >> --- a/drivers/ata/libata-core.c >> +++ b/drivers/ata/libata-core.c >> @@ -4734,7 +4734,10 @@ static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap) >> struct ata_queued_cmd *qc = NULL; >> unsigned int i, tag, max_queue; >> >> - max_queue = ap->scsi_host->can_queue; >> + if (ap->scsi_host && ap->scsi_host->can_queue <= ATA_MAX_QUEUE) >> + max_queue = ap->scsi_host->can_queue; >> + else >> + max_queue = ATA_MAX_QUEUE; >> >> /* no command while frozen */ >> if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) >> @@ -6109,16 +6112,6 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) >> { >> int i, rc; >> >> - /* >> - * The max queue supported by hardware must not be greater than >> - * ATA_MAX_QUEUE. >> - */ >> - if (sht->can_queue > ATA_MAX_QUEUE) { >> - dev_err(host->dev, "BUG: the hardware max queue is too large\n"); >> - WARN_ON(1); >> - return -EINVAL; >> - } >> - > So, ummm, I really don't like that we're adding the conditionals to > the hot path (yeah, its implementation is slow but still). Maybe we Yes, agree ..., not a good idea to do this... Thanks Mike > need to store the chosen queue depth after all? Dan? > > Thanks. > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() 2014-07-23 2:10 ` Mike Qiu @ 2014-07-25 17:01 ` Dan Williams 0 siblings, 0 replies; 4+ messages in thread From: Dan Williams @ 2014-07-25 17:01 UTC (permalink / raw) To: Mike Qiu Cc: Tejun Heo, linux-kernel@vger.kernel.org, IDE/ATA development list, Kevin Hao On Tue, Jul 22, 2014 at 7:10 PM, Mike Qiu <qiudayu@linux.vnet.ibm.com> wrote: > On 07/22/2014 11:42 PM, Tejun Heo wrote: >> >> Hello, >> >> (cc'ing Dan) >> >> On Tue, Jul 22, 2014 at 10:50:19AM -0400, Mike Qiu wrote: >>> >>> The can_queue in scsi_host can be more than ATA_MAX_QUEUE (32), >>> for example, in ipr, it can be 100 or more. >>> >>> Also, some drivers, like ipr driver, haven't filled the field >>> scsi_host in ata_port, and will lead a call trace, so add >>> check for that. >>> >>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com> >>> --- >>> drivers/ata/libata-core.c | 15 ++++----------- >>> 1 file changed, 4 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c >>> index 259d879..a5b9c70 100644 >>> --- a/drivers/ata/libata-core.c >>> +++ b/drivers/ata/libata-core.c >>> @@ -4734,7 +4734,10 @@ static struct ata_queued_cmd *ata_qc_new(struct >>> ata_port *ap) >>> struct ata_queued_cmd *qc = NULL; >>> unsigned int i, tag, max_queue; >>> - max_queue = ap->scsi_host->can_queue; >>> + if (ap->scsi_host && ap->scsi_host->can_queue <= ATA_MAX_QUEUE) >>> + max_queue = ap->scsi_host->can_queue; >>> + else >>> + max_queue = ATA_MAX_QUEUE; >>> /* no command while frozen */ >>> if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) >>> @@ -6109,16 +6112,6 @@ int ata_host_register(struct ata_host *host, >>> struct scsi_host_template *sht) >>> { >>> int i, rc; >>> - /* >>> - * The max queue supported by hardware must not be greater than >>> - * ATA_MAX_QUEUE. >>> - */ >>> - if (sht->can_queue > ATA_MAX_QUEUE) { >>> - dev_err(host->dev, "BUG: the hardware max queue is too >>> large\n"); >>> - WARN_ON(1); >>> - return -EINVAL; >>> - } >>> - >> >> So, ummm, I really don't like that we're adding the conditionals to >> the hot path (yeah, its implementation is slow but still). Maybe we > > > Yes, agree ..., not a good idea to do this... > ...also, seems incomplete given ata_port.qcmd[] is still limited to ATA_MAX_QUEUE. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-25 17:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-22 14:50 [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new() Mike Qiu 2014-07-22 15:42 ` Tejun Heo 2014-07-23 2:10 ` Mike Qiu 2014-07-25 17:01 ` Dan Williams
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox