Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
On Tue, Jul 22, 2014 at 7:10 PM, Mike Qiu 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
>>> ---
>>> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
On Tue, Jul 22, 2014 at 7:10 PM, Mike Qiu qiud...@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 qiud...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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
---
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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 qiud...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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 qiud...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] libata: Fix scsi_host can_queue issue in ata_qc_new()
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 qiud...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
