Re: [PATCH 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On Wed, Jul 23, 2014 at 2:03 AM, Alexey Kardashevskiy wrote: > On 07/23/2014 06:11 AM, Tejun Heo wrote: >> Hello, >> >> Can you please test the following patch? > > Tested-by: Alexey Kardashevskiy > > on POWER8 + IBM IPR SCSI. FWIW, I tested Linus's current git tree and the issue is fixed. 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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On Wed, Jul 23, 2014 at 2:03 AM, Alexey Kardashevskiy a...@ozlabs.ru wrote: On 07/23/2014 06:11 AM, Tejun Heo wrote: Hello, Can you please test the following patch? Tested-by: Alexey Kardashevskiy a...@ozlabs.ru on POWER8 + IBM IPR SCSI. FWIW, I tested Linus's current git tree and the issue is fixed. 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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On 07/23/2014 06:11 AM, Tejun Heo wrote:
> Hello,
>
> Can you please test the following patch?
Tested-by: Alexey Kardashevskiy
on POWER8 + IBM IPR SCSI.
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index d19c37a7..773f4e6 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4798,9 +4798,8 @@ 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, max_queue;
> -
> - max_queue = ap->scsi_host->can_queue;
> + unsigned int max_queue = ap->host->n_tags;
> + unsigned int i, tag;
>
> /* no command while frozen */
> if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
> @@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
> *dev,
> {
> spin_lock_init(>lock);
> mutex_init(>eh_mutex);
> + host->n_tags = ATA_MAX_QUEUE;
> host->dev = dev;
> host->ops = ops;
> }
> @@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
> scsi_host_template *sht)
>* 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->n_tags = clamp(sht->can_queue, 1, ATA_MAX_QUEUE);
>
> /* host must have been started */
> if (!(host->flags & ATA_HOST_STARTED)) {
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 5ab4e3a..92abb49 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -593,6 +593,7 @@ struct ata_host {
> struct device *dev;
> void __iomem * const*iomap;
> unsigned intn_ports;
> + unsigned intn_tags; /* nr of NCQ tags */
> void*private_data;
> struct ata_port_operations *ops;
> unsigned long flags;
> --
> 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/
>
--
Alexey
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On 07/23/2014 06:11 AM, Tejun Heo wrote:
Hello,
Can you please test the following patch?
Tested-by: Alexey Kardashevskiy a...@ozlabs.ru
on POWER8 + IBM IPR SCSI.
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d19c37a7..773f4e6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4798,9 +4798,8 @@ 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, max_queue;
-
- max_queue = ap-scsi_host-can_queue;
+ unsigned int max_queue = ap-host-n_tags;
+ unsigned int i, tag;
/* no command while frozen */
if (unlikely(ap-pflags ATA_PFLAG_FROZEN))
@@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
*dev,
{
spin_lock_init(host-lock);
mutex_init(host-eh_mutex);
+ host-n_tags = ATA_MAX_QUEUE;
host-dev = dev;
host-ops = ops;
}
@@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
scsi_host_template *sht)
* 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-n_tags = clamp(sht-can_queue, 1, ATA_MAX_QUEUE);
/* host must have been started */
if (!(host-flags ATA_HOST_STARTED)) {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..92abb49 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -593,6 +593,7 @@ struct ata_host {
struct device *dev;
void __iomem * const*iomap;
unsigned intn_ports;
+ unsigned intn_tags; /* nr of NCQ tags */
void*private_data;
struct ata_port_operations *ops;
unsigned long flags;
--
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/
--
Alexey
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On 07/22/2014 10:51 PM, Mike Qiu wrote:
In ata_sas_port_alloc(), it haven't initialized scsi_host field in
ata_port, although scsi_host is in parameters list and unused in this
function.
With commit 1871ee134b73 ("libata: support the ata host which implements a queue
depth less than 32")
ata_qc_new() try to use scsi_host, while it
is a NULL pointer for ipr IOA and error message shows below:
...
While scsi_host is unused in ata_sas_port_alloc(), better to set it
in ata_sas_port_alloc() instead of in driver.
Signed-off-by: Mike Qiu
---
drivers/ata/libata-scsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0586f66..a472b6f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
ap->flags |= port_info->flags;
ap->ops = port_info->port_ops;
ap->cbl = ATA_CBL_SATA;
+ ap->scsi_host = shost;
What about my patch itself, ata_sas_port_alloc() has "shot" in
parameters list, but unused.
Maybe better to set ap->scsi_host here, it is very convenient, and
drivers, like ipr, may forget to set this field, otherwise "shot" need
to be removed from parameters list I think.
Thanks,
Mike
return ap;
}
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
I have tested with the ipr IOA, passed.
Reviewed-and Tested-by: Mike Qiu
On 07/23/2014 04:11 AM, Tejun Heo wrote:
Hello,
Can you please test the following patch?
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d19c37a7..773f4e6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4798,9 +4798,8 @@ 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, max_queue;
-
- max_queue = ap->scsi_host->can_queue;
+ unsigned int max_queue = ap->host->n_tags;
+ unsigned int i, tag;
/* no command while frozen */
if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
@@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
*dev,
{
spin_lock_init(>lock);
mutex_init(>eh_mutex);
+ host->n_tags = ATA_MAX_QUEUE;
host->dev = dev;
host->ops = ops;
}
@@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
scsi_host_template *sht)
* 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->n_tags = clamp(sht->can_queue, 1, ATA_MAX_QUEUE);
/* host must have been started */
if (!(host->flags & ATA_HOST_STARTED)) {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..92abb49 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -593,6 +593,7 @@ struct ata_host {
struct device *dev;
void __iomem * const*iomap;
unsigned intn_ports;
+ unsigned intn_tags; /* nr of NCQ tags */
void*private_data;
struct ata_port_operations *ops;
unsigned long flags;
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
Hello,
Can you please test the following patch?
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d19c37a7..773f4e6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4798,9 +4798,8 @@ 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, max_queue;
-
- max_queue = ap->scsi_host->can_queue;
+ unsigned int max_queue = ap->host->n_tags;
+ unsigned int i, tag;
/* no command while frozen */
if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
@@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
*dev,
{
spin_lock_init(>lock);
mutex_init(>eh_mutex);
+ host->n_tags = ATA_MAX_QUEUE;
host->dev = dev;
host->ops = ops;
}
@@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
scsi_host_template *sht)
* 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->n_tags = clamp(sht->can_queue, 1, ATA_MAX_QUEUE);
/* host must have been started */
if (!(host->flags & ATA_HOST_STARTED)) {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..92abb49 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -593,6 +593,7 @@ struct ata_host {
struct device *dev;
void __iomem * const*iomap;
unsigned intn_ports;
+ unsigned intn_tags; /* nr of NCQ tags */
void*private_data;
struct ata_port_operations *ops;
unsigned long flags;
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
[ +cc Jesse Brandeburg - similar LKML report (but on x86_64) ]
On 07/22/2014 10:51 AM, Mike Qiu wrote:
> In ata_sas_port_alloc(), it haven't initialized scsi_host field in
> ata_port, although scsi_host is in parameters list and unused in this
> function.
>
> With commit 1871ee134b73 ("libata: support the ata host which implements a
> queue depth less than 32")
> ata_qc_new() try to use scsi_host, while it
> is a NULL pointer for ipr IOA and error message shows below:
>
> Unable to handle kernel paging request for data at address 0x0114
> Faulting instruction address: 0xc05c2580
> Oops: Kernel access of bad area, sig: 11 [#1]
> ...
> NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0
> LR [c05c9384] .ata_scsi_translate+0x44/0x230
> Call Trace:
> 0xc003ad332280 (unreliable)
> .ata_scsi_translate+0x44/0x230
> .ipr_queuecommand+0x2e0/0x780 [ipr]
> .scsi_dispatch_cmd+0xec/0x400
> .scsi_request_fn+0x52c/0x670
> .__blk_run_queue+0x5c/0x80
> .blk_execute_rq_nowait+0xf8/0x1c0
> .blk_execute_rq+0x88/0x150
> .scsi_execute+0xf0/0x1f0
> .scsi_execute_req_flags+0xc4/0x170
> .scsi_probe_and_add_lun+0x2d4/0xe00
> .__scsi_scan_target+0x1a4/0x790
> .scsi_scan_channel.part.3+0x80/0xc0
> .scsi_scan_host_selected+0x1a0/0x240
> .do_scan_async+0x30/0x210
> .async_run_entry_fn+0x78/0x1c0
> .process_one_work+0x1c4/0x4a0
> .worker_thread+0x184/0x600
> .kthread+0x10c/0x130
> .ret_from_kernel_thread+0x58/0x7c
>
> While scsi_host is unused in ata_sas_port_alloc(), better to set it
> in ata_sas_port_alloc() instead of in driver.
>
> Signed-off-by: Mike Qiu
> ---
> drivers/ata/libata-scsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 0586f66..a472b6f 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host
> *host,
> ap->flags |= port_info->flags;
> ap->ops = port_info->port_ops;
> ap->cbl = ATA_CBL_SATA;
> + ap->scsi_host = shost;
>
> return ap;
> }
>
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
[+cc Wendy, Brian King, Stephen]
On 07/22/2014 10:51 PM, Mike Qiu wrote:
In ata_sas_port_alloc(), it haven't initialized scsi_host field in
ata_port, although scsi_host is in parameters list and unused in this
function.
With commit 1871ee134b73 ("libata: support the ata host which implements a queue
depth less than 32")
ata_qc_new() try to use scsi_host, while it
is a NULL pointer for ipr IOA and error message shows below:
Unable to handle kernel paging request for data at address 0x0114
Faulting instruction address: 0xc05c2580
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0
LR [c05c9384] .ata_scsi_translate+0x44/0x230
Call Trace:
0xc003ad332280 (unreliable)
.ata_scsi_translate+0x44/0x230
.ipr_queuecommand+0x2e0/0x780 [ipr]
.scsi_dispatch_cmd+0xec/0x400
.scsi_request_fn+0x52c/0x670
.__blk_run_queue+0x5c/0x80
.blk_execute_rq_nowait+0xf8/0x1c0
.blk_execute_rq+0x88/0x150
.scsi_execute+0xf0/0x1f0
.scsi_execute_req_flags+0xc4/0x170
.scsi_probe_and_add_lun+0x2d4/0xe00
.__scsi_scan_target+0x1a4/0x790
.scsi_scan_channel.part.3+0x80/0xc0
.scsi_scan_host_selected+0x1a0/0x240
.do_scan_async+0x30/0x210
.async_run_entry_fn+0x78/0x1c0
.process_one_work+0x1c4/0x4a0
.worker_thread+0x184/0x600
.kthread+0x10c/0x130
.ret_from_kernel_thread+0x58/0x7c
While scsi_host is unused in ata_sas_port_alloc(), better to set it
in ata_sas_port_alloc() instead of in driver.
Signed-off-by: Mike Qiu
---
drivers/ata/libata-scsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0586f66..a472b6f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
ap->flags |= port_info->flags;
ap->ops = port_info->port_ops;
ap->cbl = ATA_CBL_SATA;
+ ap->scsi_host = shost;
return ap;
}
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
In ata_sas_port_alloc(), it haven't initialized scsi_host field in
ata_port, although scsi_host is in parameters list and unused in this
function.
With commit 1871ee134b73 ("libata: support the ata host which implements a
queue depth less than 32")
ata_qc_new() try to use scsi_host, while it
is a NULL pointer for ipr IOA and error message shows below:
Unable to handle kernel paging request for data at address 0x0114
Faulting instruction address: 0xc05c2580
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0
LR [c05c9384] .ata_scsi_translate+0x44/0x230
Call Trace:
0xc003ad332280 (unreliable)
.ata_scsi_translate+0x44/0x230
.ipr_queuecommand+0x2e0/0x780 [ipr]
.scsi_dispatch_cmd+0xec/0x400
.scsi_request_fn+0x52c/0x670
.__blk_run_queue+0x5c/0x80
.blk_execute_rq_nowait+0xf8/0x1c0
.blk_execute_rq+0x88/0x150
.scsi_execute+0xf0/0x1f0
.scsi_execute_req_flags+0xc4/0x170
.scsi_probe_and_add_lun+0x2d4/0xe00
.__scsi_scan_target+0x1a4/0x790
.scsi_scan_channel.part.3+0x80/0xc0
.scsi_scan_host_selected+0x1a0/0x240
.do_scan_async+0x30/0x210
.async_run_entry_fn+0x78/0x1c0
.process_one_work+0x1c4/0x4a0
.worker_thread+0x184/0x600
.kthread+0x10c/0x130
.ret_from_kernel_thread+0x58/0x7c
While scsi_host is unused in ata_sas_port_alloc(), better to set it
in ata_sas_port_alloc() instead of in driver.
Signed-off-by: Mike Qiu
---
drivers/ata/libata-scsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0586f66..a472b6f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
ap->flags |= port_info->flags;
ap->ops = port_info->port_ops;
ap->cbl = ATA_CBL_SATA;
+ ap->scsi_host = shost;
return ap;
}
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
In ata_sas_port_alloc(), it haven't initialized scsi_host field in ata_port, although scsi_host is in parameters list and unused in this function. With commit 1871ee134b73 (libata: support the ata host which implements a queue depth less than 32) ata_qc_new() try to use scsi_host, while it is a NULL pointer for ipr IOA and error message shows below: Unable to handle kernel paging request for data at address 0x0114 Faulting instruction address: 0xc05c2580 Oops: Kernel access of bad area, sig: 11 [#1] ... NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0 LR [c05c9384] .ata_scsi_translate+0x44/0x230 Call Trace: 0xc003ad332280 (unreliable) .ata_scsi_translate+0x44/0x230 .ipr_queuecommand+0x2e0/0x780 [ipr] .scsi_dispatch_cmd+0xec/0x400 .scsi_request_fn+0x52c/0x670 .__blk_run_queue+0x5c/0x80 .blk_execute_rq_nowait+0xf8/0x1c0 .blk_execute_rq+0x88/0x150 .scsi_execute+0xf0/0x1f0 .scsi_execute_req_flags+0xc4/0x170 .scsi_probe_and_add_lun+0x2d4/0xe00 .__scsi_scan_target+0x1a4/0x790 .scsi_scan_channel.part.3+0x80/0xc0 .scsi_scan_host_selected+0x1a0/0x240 .do_scan_async+0x30/0x210 .async_run_entry_fn+0x78/0x1c0 .process_one_work+0x1c4/0x4a0 .worker_thread+0x184/0x600 .kthread+0x10c/0x130 .ret_from_kernel_thread+0x58/0x7c While scsi_host is unused in ata_sas_port_alloc(), better to set it in ata_sas_port_alloc() instead of in driver. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- drivers/ata/libata-scsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0586f66..a472b6f 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ap-flags |= port_info-flags; ap-ops = port_info-port_ops; ap-cbl = ATA_CBL_SATA; + ap-scsi_host = shost; return ap; } -- 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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
[+cc Wendy, Brian King, Stephen] On 07/22/2014 10:51 PM, Mike Qiu wrote: In ata_sas_port_alloc(), it haven't initialized scsi_host field in ata_port, although scsi_host is in parameters list and unused in this function. With commit 1871ee134b73 (libata: support the ata host which implements a queue depth less than 32) ata_qc_new() try to use scsi_host, while it is a NULL pointer for ipr IOA and error message shows below: Unable to handle kernel paging request for data at address 0x0114 Faulting instruction address: 0xc05c2580 Oops: Kernel access of bad area, sig: 11 [#1] ... NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0 LR [c05c9384] .ata_scsi_translate+0x44/0x230 Call Trace: 0xc003ad332280 (unreliable) .ata_scsi_translate+0x44/0x230 .ipr_queuecommand+0x2e0/0x780 [ipr] .scsi_dispatch_cmd+0xec/0x400 .scsi_request_fn+0x52c/0x670 .__blk_run_queue+0x5c/0x80 .blk_execute_rq_nowait+0xf8/0x1c0 .blk_execute_rq+0x88/0x150 .scsi_execute+0xf0/0x1f0 .scsi_execute_req_flags+0xc4/0x170 .scsi_probe_and_add_lun+0x2d4/0xe00 .__scsi_scan_target+0x1a4/0x790 .scsi_scan_channel.part.3+0x80/0xc0 .scsi_scan_host_selected+0x1a0/0x240 .do_scan_async+0x30/0x210 .async_run_entry_fn+0x78/0x1c0 .process_one_work+0x1c4/0x4a0 .worker_thread+0x184/0x600 .kthread+0x10c/0x130 .ret_from_kernel_thread+0x58/0x7c While scsi_host is unused in ata_sas_port_alloc(), better to set it in ata_sas_port_alloc() instead of in driver. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- drivers/ata/libata-scsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0586f66..a472b6f 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ap-flags |= port_info-flags; ap-ops = port_info-port_ops; ap-cbl = ATA_CBL_SATA; + ap-scsi_host = shost; return ap; } -- 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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
[ +cc Jesse Brandeburg - similar LKML report (but on x86_64) ] On 07/22/2014 10:51 AM, Mike Qiu wrote: In ata_sas_port_alloc(), it haven't initialized scsi_host field in ata_port, although scsi_host is in parameters list and unused in this function. With commit 1871ee134b73 (libata: support the ata host which implements a queue depth less than 32) ata_qc_new() try to use scsi_host, while it is a NULL pointer for ipr IOA and error message shows below: Unable to handle kernel paging request for data at address 0x0114 Faulting instruction address: 0xc05c2580 Oops: Kernel access of bad area, sig: 11 [#1] ... NIP [c05c2580] .ata_qc_new_init+0x30/0x1f0 LR [c05c9384] .ata_scsi_translate+0x44/0x230 Call Trace: 0xc003ad332280 (unreliable) .ata_scsi_translate+0x44/0x230 .ipr_queuecommand+0x2e0/0x780 [ipr] .scsi_dispatch_cmd+0xec/0x400 .scsi_request_fn+0x52c/0x670 .__blk_run_queue+0x5c/0x80 .blk_execute_rq_nowait+0xf8/0x1c0 .blk_execute_rq+0x88/0x150 .scsi_execute+0xf0/0x1f0 .scsi_execute_req_flags+0xc4/0x170 .scsi_probe_and_add_lun+0x2d4/0xe00 .__scsi_scan_target+0x1a4/0x790 .scsi_scan_channel.part.3+0x80/0xc0 .scsi_scan_host_selected+0x1a0/0x240 .do_scan_async+0x30/0x210 .async_run_entry_fn+0x78/0x1c0 .process_one_work+0x1c4/0x4a0 .worker_thread+0x184/0x600 .kthread+0x10c/0x130 .ret_from_kernel_thread+0x58/0x7c While scsi_host is unused in ata_sas_port_alloc(), better to set it in ata_sas_port_alloc() instead of in driver. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- drivers/ata/libata-scsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0586f66..a472b6f 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ap-flags |= port_info-flags; ap-ops = port_info-port_ops; ap-cbl = ATA_CBL_SATA; + ap-scsi_host = shost; return ap; } -- 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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
Hello,
Can you please test the following patch?
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d19c37a7..773f4e6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4798,9 +4798,8 @@ 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, max_queue;
-
- max_queue = ap-scsi_host-can_queue;
+ unsigned int max_queue = ap-host-n_tags;
+ unsigned int i, tag;
/* no command while frozen */
if (unlikely(ap-pflags ATA_PFLAG_FROZEN))
@@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
*dev,
{
spin_lock_init(host-lock);
mutex_init(host-eh_mutex);
+ host-n_tags = ATA_MAX_QUEUE;
host-dev = dev;
host-ops = ops;
}
@@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
scsi_host_template *sht)
* 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-n_tags = clamp(sht-can_queue, 1, ATA_MAX_QUEUE);
/* host must have been started */
if (!(host-flags ATA_HOST_STARTED)) {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..92abb49 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -593,6 +593,7 @@ struct ata_host {
struct device *dev;
void __iomem * const*iomap;
unsigned intn_ports;
+ unsigned intn_tags; /* nr of NCQ tags */
void*private_data;
struct ata_port_operations *ops;
unsigned long flags;
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
I have tested with the ipr IOA, passed.
Reviewed-and Tested-by: Mike Qiu qiud...@linux.vnet.ibm.com
On 07/23/2014 04:11 AM, Tejun Heo wrote:
Hello,
Can you please test the following patch?
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d19c37a7..773f4e6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4798,9 +4798,8 @@ 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, max_queue;
-
- max_queue = ap-scsi_host-can_queue;
+ unsigned int max_queue = ap-host-n_tags;
+ unsigned int i, tag;
/* no command while frozen */
if (unlikely(ap-pflags ATA_PFLAG_FROZEN))
@@ -6094,6 +6093,7 @@ void ata_host_init(struct ata_host *host, struct device
*dev,
{
spin_lock_init(host-lock);
mutex_init(host-eh_mutex);
+ host-n_tags = ATA_MAX_QUEUE;
host-dev = dev;
host-ops = ops;
}
@@ -6179,11 +6179,7 @@ int ata_host_register(struct ata_host *host, struct
scsi_host_template *sht)
* 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-n_tags = clamp(sht-can_queue, 1, ATA_MAX_QUEUE);
/* host must have been started */
if (!(host-flags ATA_HOST_STARTED)) {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..92abb49 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -593,6 +593,7 @@ struct ata_host {
struct device *dev;
void __iomem * const*iomap;
unsigned intn_ports;
+ unsigned intn_tags; /* nr of NCQ tags */
void*private_data;
struct ata_port_operations *ops;
unsigned long flags;
--
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 2/2] libata: Fix NULL pointer of scsi_host in ata_port
On 07/22/2014 10:51 PM, Mike Qiu wrote: In ata_sas_port_alloc(), it haven't initialized scsi_host field in ata_port, although scsi_host is in parameters list and unused in this function. With commit 1871ee134b73 (libata: support the ata host which implements a queue depth less than 32) ata_qc_new() try to use scsi_host, while it is a NULL pointer for ipr IOA and error message shows below: ... While scsi_host is unused in ata_sas_port_alloc(), better to set it in ata_sas_port_alloc() instead of in driver. Signed-off-by: Mike Qiu qiud...@linux.vnet.ibm.com --- drivers/ata/libata-scsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0586f66..a472b6f 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4070,6 +4070,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ap-flags |= port_info-flags; ap-ops = port_info-port_ops; ap-cbl = ATA_CBL_SATA; + ap-scsi_host = shost; What about my patch itself, ata_sas_port_alloc() has shot in parameters list, but unused. Maybe better to set ap-scsi_host here, it is very convenient, and drivers, like ipr, may forget to set this field, otherwise shot need to be removed from parameters list I think. Thanks, Mike return ap; } -- 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/
