[PATCH v3 1/7] libsas: Use static sas event pool to appease sas event lost
Now libsas hotplug work is static, every sas event type has its own
static work, LLDD driver queue the hotplug work into shost->work_q.
If LLDD driver burst post lots hotplug events to libsas, the hotplug
events may pending in the workqueue like
shost->work_q
new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] ->
processing
|<---wait worker to process>|
In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
to shost->work_q, but this work is already pending, so it would be lost.
Finally, libsas delete the related sas port and sas devices, but LLDD driver
expect libsas add the sas port and devices(last sas event).
This patch and use static sas event work pool to appease this issue, since
it's static work pool, it won't make memory exhaust.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_event.c| 208 -
drivers/scsi/libsas/sas_init.c | 6 --
drivers/scsi/libsas/sas_internal.h | 3 +
drivers/scsi/libsas/sas_phy.c | 48 +++--
drivers/scsi/libsas/sas_port.c | 18 ++--
include/scsi/libsas.h | 16 +--
6 files changed, 216 insertions(+), 83 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index c0d0d97..a1370bd 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,13 +27,20 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static DEFINE_SPINLOCK(sas_event_lock);
+
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
int sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
int rc = 0;
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
- return 0;
+ return rc;
+ rc = 1;
if (test_bit(SAS_HA_DRAINING, &ha->state)) {
/* add it to the defer list, if not already pending */
if (list_empty(&sw->drain_node))
@@ -44,19 +51,15 @@ int sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
return rc;
}
-static int sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static int sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
int rc = 0;
+ unsigned long flags;
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
-
- spin_lock_irqsave(&ha->lock, flags);
- rc = sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ rc = sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
return rc;
}
@@ -64,6 +67,8 @@ static int sas_queue_event(int event, unsigned long *pending,
void __sas_drain_work(struct sas_ha_struct *ha)
{
+ int ret;
+ unsigned long flags;
struct workqueue_struct *wq = ha->core.shost->work_q;
struct sas_work *sw, *_sw;
@@ -78,7 +83,12 @@ void __sas_drain_work(struct sas_ha_struct *ha)
clear_bit(SAS_HA_DRAINING, &ha->state);
list_for_each_entry_safe(sw, _sw, &ha->defer_q, drain_node) {
list_del_init(&sw->drain_node);
- sas_queue_work(ha, sw);
+ ret = sas_queue_work(ha, sw);
+ if (ret != 1) {
+ spin_lock_irqsave(&sas_event_lock, flags);
+ sw->used = false;
+ spin_unlock_irqrestore(&sas_event_lock, flags);
+ }
}
spin_unlock_irq(&ha->lock);
}
@@ -119,51 +129,197 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
+static void sas_free_ha_event(struct sas_ha_event *event)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&sas_event_lock, flags);
+ event->work.used = false;
+ spin_unlock_irqrestore(&sas_event_lock, flags);
+}
+
+static void sas_free_port_event(struct asd_sas_event *event)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&sas_event_lock, flags);
+ event->work.used = false;
+ spin_unlock_irqrestore(&sas_event_lock, flags);
+}
+
+static void sas_free_phy_event(struct asd_sas_event *event)
+{
+ unsigned long flags;
+
[PATCH v3 5/7] libsas: add a new workqueue to run probe/destruct discovery event
Sometimes, we want sync libsas probe or destruct in sas discovery work,
like when libsas revalidate domain. We need to split probe and destruct
work from the scsi host workqueue.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_discover.c | 13 -
drivers/scsi/libsas/sas_init.c | 8
include/scsi/libsas.h | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 5d4a3a8..a25d648 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -559,7 +559,18 @@ static void sas_chain_work(struct sas_ha_struct *ha,
struct sas_work *sw)
* not racing against draining
*/
sas_port_get(port);
- ret = scsi_queue_work(ha->core.shost, &sw->work);
+ /*
+* discovery event probe and destruct would be called in other
+* discovery event like discover domain and revalidate domain
+* events, in some cases, we need to sync execute probe and destruct
+* events, so run probe and destruct discover events except in a new
+* workqueue.
+*/
+ if (ev->type == DISCE_PROBE || ev->type == DISCE_DESTRUCT)
+ ret = queue_work(ha->disc_q, &sw->work);
+ else
+ ret = scsi_queue_work(ha->core.shost, &sw->work);
+
if (ret != 1)
sas_port_put(port);
}
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 2f3b736..df1d78b 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -152,6 +152,13 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
if (!sas_ha->event_q)
goto Undo_ports;
+ snprintf(name, 64, "%s_disc_q", dev_name(sas_ha->dev));
+ sas_ha->disc_q = create_singlethread_workqueue(name);
+ if(!sas_ha->disc_q) {
+ destroy_workqueue(sas_ha->event_q);
+ goto Undo_ports;
+ }
+
INIT_LIST_HEAD(&sas_ha->eh_done_q);
INIT_LIST_HEAD(&sas_ha->eh_ata_q);
@@ -187,6 +194,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
__sas_drain_work(sas_ha);
mutex_unlock(&sas_ha->drain_mutex);
destroy_workqueue(sas_ha->event_q);
+ destroy_workqueue(sas_ha->disc_q);
return 0;
}
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c2ef05e..4bcb9fe 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -406,6 +406,7 @@ struct sas_ha_struct {
struct device *dev; /* should be set */
struct module *lldd_module; /* should be set */
struct workqueue_struct *event_q;
+ struct workqueue_struct *disc_q;
u8 *sas_addr; /* must be set */
u8 hashed_sas_addr[HASHED_SAS_ADDR_SIZE];
--
2.5.0
[PATCH v3 6/7] libsas: add wait-complete support to sync discovery event
Introduce a sync flag to tag discovery event whether need to
sync execute, per-event wait-complete ensure sync.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_discover.c | 8 ++--
drivers/scsi/libsas/sas_expander.c | 12 +++-
drivers/scsi/libsas/sas_internal.h | 27 +++
include/scsi/libsas.h | 2 ++
4 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index a25d648..d68f8dd 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -378,6 +378,7 @@ void sas_unregister_dev(struct asd_sas_port *port, struct
domain_device *dev)
list_del_init(&dev->disco_list_node);
sas_rphy_free(dev->rphy);
sas_unregister_common_dev(port, dev);
+ sas_disc_cancel_sync(&port->disc.disc_work[DISCE_DESTRUCT]);
return;
}
@@ -541,6 +542,7 @@ static void sas_discover_common_fn(struct work_struct *work)
struct asd_sas_port *port = ev->port;
sas_event_fns[ev->type](work);
+ sas_disc_wakeup(ev);
sas_port_put(port);
}
@@ -571,8 +573,10 @@ static void sas_chain_work(struct sas_ha_struct *ha,
struct sas_work *sw)
else
ret = scsi_queue_work(ha->core.shost, &sw->work);
- if (ret != 1)
+ if (ret != 1) {
sas_port_put(port);
+ sas_disc_cancel_sync(ev);
+ }
}
static void sas_chain_event(int event, unsigned long *pending,
@@ -592,9 +596,9 @@ int sas_discover_event(struct asd_sas_port *port, enum
discover_event ev)
{
struct sas_discovery *disc;
+ disc = &port->disc;
if (!port)
return 0;
- disc = &port->disc;
BUG_ON(ev >= DISC_NUM_EVENTS);
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 570b2cb..9d26c28 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -822,14 +822,18 @@ static struct domain_device *sas_ex_discover_end_dev(
list_add_tail(&child->disco_list_node,
&parent->port->disco_list);
+ sas_disc_wait_init(child->port, DISCE_PROBE);
res = sas_discover_sata(child);
if (res) {
+
sas_disc_cancel_sync(&child->port->disc.disc_work[DISCE_PROBE]);
SAS_DPRINTK("sas_discover_sata() for device %16llx at "
"%016llx:0x%x returned 0x%x\n",
SAS_ADDR(child->sas_addr),
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+ sas_disc_wait_completion(child->port, DISCE_PROBE);
+
} else
#endif
if (phy->attached_tproto & SAS_PROTOCOL_SSP) {
@@ -847,14 +851,17 @@ static struct domain_device *sas_ex_discover_end_dev(
list_add_tail(&child->disco_list_node,
&parent->port->disco_list);
+ sas_disc_wait_init(child->port, DISCE_PROBE);
res = sas_discover_end_dev(child);
if (res) {
+
sas_disc_cancel_sync(&child->port->disc.disc_work[DISCE_PROBE]);
SAS_DPRINTK("sas_discover_end_dev() for device %16llx "
"at %016llx:0x%x returned 0x%x\n",
SAS_ADDR(child->sas_addr),
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+ sas_disc_wait_completion(child->port, DISCE_PROBE);
} else {
SAS_DPRINTK("target proto 0x%x at %016llx:0x%x not handled\n",
phy->attached_tproto, SAS_ADDR(parent->sas_addr),
@@ -1890,8 +1897,11 @@ static void sas_unregister_devs_sas_addr(struct
domain_device *parent,
if (child->dev_type == SAS_EDGE_EXPANDER_DEVICE
||
child->dev_type ==
SAS_FANOUT_EXPANDER_DEVICE)
sas_unregister_ex_tree(parent->port,
child);
- else
+ else {
+ sas_disc_wait_init(parent->port,
DISCE_DESTRUCT);
sas_unregister_dev(parent->port, child);
+ sas_disc_wait_completion(parent->port,
DISCE_DESTRUCT);
+ }
[PATCH v3 4/7] libsas: add sas event wait-complete support
Introduce wait-complete for libsas sas event processing,
execute sas port create/destruct in sync.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_discover.c | 41 --
drivers/scsi/libsas/sas_internal.h | 34 +++
drivers/scsi/libsas/sas_port.c | 4
include/scsi/libsas.h | 5 -
4 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..5d4a3a8 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -525,16 +525,43 @@ static void sas_revalidate_domain(struct work_struct
*work)
mutex_unlock(&ha->disco_mutex);
}
+static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
+ [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
+ [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+ [DISCE_PROBE] = sas_probe_devices,
+ [DISCE_SUSPEND] = sas_suspend_devices,
+ [DISCE_RESUME] = sas_resume_devices,
+ [DISCE_DESTRUCT] = sas_destruct_devices,
+};
+
+/* a simple wrapper for sas discover event funtions */
+static void sas_discover_common_fn(struct work_struct *work)
+{
+ struct sas_discovery_event *ev = to_sas_discovery_event(work);
+ struct asd_sas_port *port = ev->port;
+
+ sas_event_fns[ev->type](work);
+ sas_port_put(port);
+}
+
+
/* -- Events -- */
static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
+ int ret;
+ struct sas_discovery_event *ev = to_sas_discovery_event(&sw->work);
+ struct asd_sas_port *port = ev->port;
+
/* chained work is not subject to SA_HA_DRAINING or
* SAS_HA_REGISTERED, because it is either submitted in the
* workqueue, or known to be submitted from a context that is
* not racing against draining
*/
- scsi_queue_work(ha->core.shost, &sw->work);
+ sas_port_get(port);
+ ret = scsi_queue_work(ha->core.shost, &sw->work);
+ if (ret != 1)
+ sas_port_put(port);
}
static void sas_chain_event(int event, unsigned long *pending,
@@ -575,18 +602,10 @@ void sas_init_disc(struct sas_discovery *disc, struct
asd_sas_port *port)
{
int i;
- static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
- [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
- [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
- [DISCE_PROBE] = sas_probe_devices,
- [DISCE_SUSPEND] = sas_suspend_devices,
- [DISCE_RESUME] = sas_resume_devices,
- [DISCE_DESTRUCT] = sas_destruct_devices,
- };
-
disc->pending = 0;
for (i = 0; i < DISC_NUM_EVENTS; i++) {
- INIT_SAS_WORK(&disc->disc_work[i].work, sas_event_fns[i]);
+ INIT_SAS_WORK(&disc->disc_work[i].work, sas_discover_common_fn);
disc->disc_work[i].port = port;
+ disc->disc_work[i].type = i;
}
}
diff --git a/drivers/scsi/libsas/sas_internal.h
b/drivers/scsi/libsas/sas_internal.h
index f03ce64..890b5d26 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -100,6 +100,40 @@ void sas_free_device(struct kref *kref);
extern const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS];
extern const work_func_t sas_port_event_fns[PORT_NUM_EVENTS];
+static void sas_complete_event(struct kref *kref)
+{
+ struct asd_sas_port *port = container_of(kref, struct asd_sas_port,
ref);
+
+ complete_all(&port->completion);
+}
+
+static inline void sas_port_put(struct asd_sas_port *port)
+{
+ if (port->is_sync)
+ kref_put(&port->ref, sas_complete_event);
+}
+
+static inline void sas_port_wait_init(struct asd_sas_port *port)
+{
+ init_completion(&port->completion);
+ kref_init(&port->ref);
+ port->is_sync = true;
+}
+
+static inline void sas_port_wait_completion(
+ struct asd_sas_port *port)
+{
+ sas_port_put(port);
+ wait_for_completion(&port->completion);
+ port->is_sync = false;
+}
+
+static inline void sas_port_get(struct asd_sas_port *port)
+{
+ if (port && port->is_sync)
+ kref_get(&port->ref);
+}
+
#ifdef CONFIG_SCSI_SAS_HOST_SMP
extern int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req,
struct request *rsp);
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 9326628..d589adb 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -191,7 +191,9 @@ static void sas_form_port(struct asd_sas
[PATCH v3 3/7] libsas: Use new workqueue to run sas event
Now all libsas works are queued to scsi host workqueue,
include sas event work post by LLDD and sas discovery
work, and a sas hotplug flow may be divided into several
works, e.g libsas receive a PORTE_BYTES_DMAED event,
now we process it as following steps:
sas_form_port --- run in work in shot workq
sas_discover_domain --- run in another work in shost workq
...
sas_probe_devices --- run in new work in shost workq
We found during hot-add a device, libsas may need run several
works in same workqueue to add device in system, the process is
not atomic, it may interrupt by other sas event works, like
PHYE_LOSS_OF_SIGNAL. Finally, we would found lots unexpected
errors. This patch is preparation of execute libsas sas event
in sync.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_event.c | 4 ++--
drivers/scsi/libsas/sas_init.c | 7 +++
include/scsi/libsas.h | 1 +
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index a1370bd..a72a089 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -46,7 +46,7 @@ int sas_queue_work(struct sas_ha_struct *ha, struct sas_work
*sw)
if (list_empty(&sw->drain_node))
list_add(&sw->drain_node, &ha->defer_q);
} else
- rc = scsi_queue_work(ha->core.shost, &sw->work);
+ rc = queue_work(ha->event_q, &sw->work);
return rc;
}
@@ -69,7 +69,7 @@ void __sas_drain_work(struct sas_ha_struct *ha)
{
int ret;
unsigned long flags;
- struct workqueue_struct *wq = ha->core.shost->work_q;
+ struct workqueue_struct *wq = ha->event_q;
struct sas_work *sw, *_sw;
set_bit(SAS_HA_DRAINING, &ha->state);
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index c227a8b..2f3b736 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -115,6 +115,7 @@ void sas_hae_reset(struct work_struct *work)
int sas_register_ha(struct sas_ha_struct *sas_ha)
{
+ char name[64];
int error = 0;
mutex_init(&sas_ha->disco_mutex);
@@ -146,6 +147,11 @@ int sas_register_ha(struct sas_ha_struct *sas_ha)
goto Undo_ports;
}
+ snprintf(name, 64, "%s_event_q", dev_name(sas_ha->dev));
+ sas_ha->event_q = create_singlethread_workqueue(name);
+ if (!sas_ha->event_q)
+ goto Undo_ports;
+
INIT_LIST_HEAD(&sas_ha->eh_done_q);
INIT_LIST_HEAD(&sas_ha->eh_ata_q);
@@ -180,6 +186,7 @@ int sas_unregister_ha(struct sas_ha_struct *sas_ha)
mutex_lock(&sas_ha->drain_mutex);
__sas_drain_work(sas_ha);
mutex_unlock(&sas_ha->drain_mutex);
+ destroy_workqueue(sas_ha->event_q);
return 0;
}
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 628f48b..a01ca42 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -402,6 +402,7 @@ struct sas_ha_struct {
char *sas_ha_name;
struct device *dev; /* should be set */
struct module *lldd_module; /* should be set */
+ struct workqueue_struct *event_q;
u8 *sas_addr; /* must be set */
u8 hashed_sas_addr[HASHED_SAS_ADDR_SIZE];
--
2.5.0
[PATCH v3 2/7] libsas: remove unused port_gone_completion
No one uses the port_gone_completion in struct asd_sas_port,
clean it out.
Signed-off-by: Yijing Wang
---
include/scsi/libsas.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index c41328d..628f48b 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -263,8 +263,6 @@ struct sas_discovery {
/* The port struct is Class:RW, driver:RO */
struct asd_sas_port {
/* private: */
- struct completion port_gone_completion;
-
struct sas_discovery disc;
struct domain_device *port_dev;
spinlock_t dev_list_lock;
--
2.5.0
[PATCH v3 7/7] libsas: release disco mutex during waiting in sas_ex_discover_end_dev
Disco mutex was introudced to prevent domain rediscovery competing
with ata error handling(87c8331). If we have already hold the lock
in sas_revalidate_domain and sync executing probe, deadlock caused,
because, sas_probe_sata() also need hold disco_mutex. Since disco mutex
use to prevent revalidata domain happen during ata error handler,
it should be safe to release disco mutex when sync probe, because
no new revalidate domain event would be process until the sync return,
and the current sas revalidate domain finish.
Signed-off-by: Yijing Wang
CC: John Garry
CC: Johannes Thumshirn
CC: Ewan Milne
CC: Christoph Hellwig
CC: Tomas Henzl
CC: Dan Williams
---
drivers/scsi/libsas/sas_expander.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index 9d26c28..077024e 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -776,6 +776,7 @@ static struct domain_device *sas_ex_discover_end_dev(
struct ex_phy *phy = &parent_ex->ex_phy[phy_id];
struct domain_device *child = NULL;
struct sas_rphy *rphy;
+ bool prev_lock;
int res;
if (phy->attached_sata_host || phy->attached_sata_ps)
@@ -803,6 +804,7 @@ static struct domain_device *sas_ex_discover_end_dev(
sas_ex_get_linkrate(parent, child, phy);
sas_device_set_phy(child, phy->port);
+ prev_lock = mutex_is_locked(&child->port->ha->disco_mutex);
#ifdef CONFIG_SCSI_SAS_ATA
if ((phy->attached_tproto & SAS_PROTOCOL_STP) ||
phy->attached_sata_dev) {
res = sas_get_ata_info(child, phy);
@@ -832,7 +834,11 @@ static struct domain_device *sas_ex_discover_end_dev(
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+ if (prev_lock)
+ mutex_unlock(&child->port->ha->disco_mutex);
sas_disc_wait_completion(child->port, DISCE_PROBE);
+ if (prev_lock)
+ mutex_lock(&child->port->ha->disco_mutex);
} else
#endif
@@ -861,7 +867,11 @@ static struct domain_device *sas_ex_discover_end_dev(
SAS_ADDR(parent->sas_addr), phy_id, res);
goto out_list_del;
}
+ if (prev_lock)
+ mutex_unlock(&child->port->ha->disco_mutex);
sas_disc_wait_completion(child->port, DISCE_PROBE);
+ if (prev_lock)
+ mutex_lock(&child->port->ha->disco_mutex);
} else {
SAS_DPRINTK("target proto 0x%x at %016llx:0x%x not handled\n",
phy->attached_tproto, SAS_ADDR(parent->sas_addr),
--
2.5.0
[PATCH v3 0/7] Enhance libsas hotplug feature
This patchset is based Johannes's patch "scsi: sas: scsi_queue_work can fail, so make callers aware" Now the libsas hotplug has some issues, Dan Williams report a similar bug here before https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html The issues we have found 1. if LLDD burst reports lots of phy-up/phy-down sas events, some events may lost because a same sas events is pending now, finally libsas topo may different the hardware. 2. receive a phy down sas event, libsas call sas_deform_port to remove devices, it would first delete the sas port, then put a destruction discovery event in a new work, and queue it at the tail of workqueue, once the sas port be deleted, its children device will be deleted too, when the destruction work start, it will found the target device has been removed, and report a sysfs warnning. 3. since a hotplug process will be devided into several works, if a phy up sas event insert into phydown works, like destruction work ---> PORTE_BYTES_DMAED (sas_form_port) >PHYE_LOSS_OF_SIGNAL the hot remove flow would broken by PORTE_BYTES_DMAED event, it's not we expected, and issues would occur. The first patch fix the sas events lost, and the second one introudce wait-complete to fix the hotplug order issues. v2->v3: some code improvements suggested by Johannes and John, split v2 patch 2 into several small pathes. v1->v2: some code improvements suggested by John Garry Yijing Wang (7): libsas: Use static sas event pool to appease sas event lost libsas: remove unused port_gone_completion libsas: Use new workqueue to run sas event libsas: add sas event wait-complete support libsas: add a new workqueue to run probe/destruct discovery event libsas: add wait-complete support to sync discovery event libsas: release disco mutex during waiting in sas_ex_discover_end_dev drivers/scsi/libsas/sas_discover.c | 58 +++--- drivers/scsi/libsas/sas_event.c| 212 - drivers/scsi/libsas/sas_expander.c | 22 +++- drivers/scsi/libsas/sas_init.c | 21 ++-- drivers/scsi/libsas/sas_internal.h | 64 +++ drivers/scsi/libsas/sas_phy.c | 48 +++-- drivers/scsi/libsas/sas_port.c | 22 ++-- include/scsi/libsas.h | 27 +++-- 8 files changed, 373 insertions(+), 101 deletions(-) -- 2.5.0
[Resend][PATCH v2 2/2] libsas: Enhance libsas hotplug
Libsas complete a hotplug event notified by LLDD in several works,
for example, if libsas receive a PHYE_LOSS_OF_SIGNAL, we process it
in following steps:
notify_phy_event[interrupt context]
sas_queue_event [queue work on shost->work_q]
sas_phye_loss_of_signal [running in shost->work_q]
sas_deform_port [remove sas port]
sas_unregister_dev
sas_discover_event [queue destruct
work on shost->work_q tail]
In above case, complete whole hotplug in two works, remove sas port first, then
put the destruction of device in another work and queue it on in the tail of
workqueue, since sas port is the parent of the children rphy device, so if
remove
sas port first, the children rphy device would also be deleted, when the
destruction
work coming, it would find the target has been removed already, and report a
sysfs warning calltrace.
queue tail queue head
DISCE_DESTRUCT> PORTE_BYTES_DMAED event ->PHYE_LOSS_OF_SIGNAL[running]
There are other hotplug issues in current framework, in above case, if there is
hotadd sas event queued between hotremove works, the hotplug order would be
broken
and unexpected issues would happen.
In this patch, we try to solve these issues in following steps:
1. create a new workqueue used to run sas event work, instead of scsi host
workqueue,
because we may block sas event work, we cannot block the normal scsi works.
When libsas receive a phy down event, sas_deform_port would be called, and
now we
block sas_deform_port and wait for destruction work finish, in
sas_destruct_devices,
we may wait ata error handler, it would take a long time, so if do all stuff
in scsi
host workq, libsas may block other scsi works too long.
2. create a new workqueue used to run sas discovery events work, instead of
scsi host
workqueue, because in some cases, eg. in revalidate domain event, we may
unregister
a sas device and discover new one, we must sync the execution, wait the
remove process
finish, then start a new discovery. So we must put the probe and destruct
discovery
events in a new workqueue to avoid deadlock.
3. introudce a asd_sas_port level wait-complete and a sas_discovery level
wait-complete
we use former wait-complete to achieve a sas event atomic process and use
latter to
make a sas discovery sync.
4. remove disco_mutex in sas_revalidate_domain, since now sas_revalidate_domain
sync
the destruct discovery event execution, it's no need to lock disco mutex
there.
Signed-off-by: Yijing Wang
---
drivers/scsi/libsas/sas_discover.c | 58 ++--
drivers/scsi/libsas/sas_event.c| 2 +-
drivers/scsi/libsas/sas_expander.c | 9 +-
drivers/scsi/libsas/sas_init.c | 23 +-
drivers/scsi/libsas/sas_internal.h | 61 ++
drivers/scsi/libsas/sas_port.c | 4 +++
include/scsi/libsas.h | 9 ++
7 files changed, 148 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..43e8a1e 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -503,11 +503,10 @@ static void sas_revalidate_domain(struct work_struct
*work)
struct domain_device *ddev = port->port_dev;
/* prevent revalidation from finding sata links in recovery */
- mutex_lock(&ha->disco_mutex);
if (test_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state)) {
SAS_DPRINTK("REVALIDATION DEFERRED on port %d, pid:%d\n",
port->id, task_pid_nr(current));
- goto out;
+ return;
}
clear_bit(DISCE_REVALIDATE_DOMAIN, &port->disc.pending);
@@ -521,20 +520,57 @@ static void sas_revalidate_domain(struct work_struct
*work)
SAS_DPRINTK("done REVALIDATING DOMAIN on port %d, pid:%d, res 0x%x\n",
port->id, task_pid_nr(current), res);
- out:
- mutex_unlock(&ha->disco_mutex);
+}
+
+static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
+ [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
+ [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+ [DISCE_PROBE] = sas_probe_devices,
+ [DISCE_SUSPEND] = sas_suspend_devices,
+ [DISCE_RESUME] = sas_resume_devices,
+ [DISCE_DESTRUCT] = sas_destruct_devices,
+};
+
+/* a simple wrapper for sas discover event funtions */
+static void sas_discover_common_fn(struct work_struct *work)
+{
+ struct sas_discovery_event *ev = to_sas_discovery_event(work);
+ struct asd_sas_port *port = ev->port;
+
+ sas_event_fns[ev->type](work);
+ sas_unbusy_port(port);
}
/* -- Events --
[Resend][PATCH v2 1/2] libsas: Don't process sas events in static works
Now libsas hotplug work is static, LLDD driver queue
the hotplug work into shost->work_q. If LLDD driver
burst post lots hotplug events to libsas, the hotplug
events may pending in the workqueue like
shost->work_q
new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] ->
processing
|<---wait worker to process>|
In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
to shost->work_q, but this work is already pending, so it would be lost.
Finally, libsas delete the related sas port and sas devices, but LLDD driver
expect libsas add the sas port and devices(last sas event).
This patch remove the static defined hotplug work, and use dynamic work to
avoid missing hotplug events.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_event.c| 88 +++---
drivers/scsi/libsas/sas_init.c | 6 ---
drivers/scsi/libsas/sas_internal.h | 3 ++
drivers/scsi/libsas/sas_phy.c | 45 ---
drivers/scsi/libsas/sas_port.c | 18
include/scsi/libsas.h | 10 +
6 files changed, 84 insertions(+), 86 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index aadbd53..06c5c4b 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,6 +27,10 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
@@ -40,17 +44,14 @@ void sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
scsi_queue_work(ha->core.shost, &sw->work);
}
-static void sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static void sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
+ unsigned long flags;
- spin_lock_irqsave(&ha->lock, flags);
- sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
}
@@ -111,52 +112,87 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
+static void sas_ha_event_worker(struct work_struct *work)
+{
+ struct sas_ha_event *ev = to_sas_ha_event(work);
+
+ sas_ha_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_port_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_port_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_phy_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_phy_event_fns[ev->type](work);
+ kfree(ev);
+}
+
static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
{
+ struct sas_ha_event *ev;
+
BUG_ON(event >= HA_NUM_EVENTS);
- sas_queue_event(event, &sas_ha->pending,
- &sas_ha->ha_events[event].work, sas_ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_ha_event_worker);
+ ev->ha = sas_ha;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, sas_ha);
}
static void notify_port_event(struct asd_sas_phy *phy, enum port_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PORT_NUM_EVENTS);
- sas_queue_event(event, &phy->port_events_pending,
- &phy->port_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_port_event_worker);
+ ev->phy = phy;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, ha);
}
void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PHY_NUM_EVENTS);
- sas_queue_
[Resend][PATCH v2 0/2] Enhance libsas hotplug feature
Now the libsas hotplug has some issues, Dan Williams report a similar bug here before https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html The issues we have found 1. if LLDD burst reports lots of phy-up/phy-down sas events, some events may lost because a same sas events is pending now, finally libsas topo may different the hardware. 2. receive a phy down sas event, libsas call sas_deform_port to remove devices, it would first delete the sas port, then put a destruction discovery event in a new work, and queue it at the tail of workqueue, once the sas port be deleted, its children device will be deleted too, when the destruction work start, it will found the target device has been removed, and report a sysfs warnning. 3. since a hotplug process will be devided into several works, if a phy up sas event insert into phydown works, like destruction work ---> PORTE_BYTES_DMAED (sas_form_port) >PHYE_LOSS_OF_SIGNAL the hot remove flow would broken by PORTE_BYTES_DMAED event, it's not we expected, and issues would occur. The first patch fix the sas events lost, and the second one introudce wait-complete to fix the hotplug order issues. v1->v2: some code improvements suggested by John Garry Yijing Wang (2): libsas: Don't process sas events in static works libsas: Enhance libsas hotplug drivers/scsi/libsas/sas_discover.c | 58 +--- drivers/scsi/libsas/sas_event.c| 90 ++ drivers/scsi/libsas/sas_expander.c | 9 +++- drivers/scsi/libsas/sas_init.c | 29 +--- drivers/scsi/libsas/sas_internal.h | 64 +++ drivers/scsi/libsas/sas_phy.c | 45 --- drivers/scsi/libsas/sas_port.c | 22 +- include/scsi/libsas.h | 19 8 files changed, 232 insertions(+), 104 deletions(-) -- 2.5.0
[PATCH v2 2/2] libsas: Enhance libsas hotplug
Libsas complete a hotplug event notified by LLDD in several works,
for example, if libsas receive a PHYE_LOSS_OF_SIGNAL, we process it
in following steps:
notify_phy_event[interrupt context]
sas_queue_event [queue work on shost->work_q]
sas_phye_loss_of_signal [running in shost->work_q]
sas_deform_port [remove sas port]
sas_unregister_dev
sas_discover_event [queue destruct
work on shost->work_q tail]
In above case, complete whole hotplug in two works, remove sas port first, then
put the destruction of device in another work and queue it on in the tail of
workqueue, since sas port is the parent of the children rphy device, so if
remove
sas port first, the children rphy device would also be deleted, when the
destruction
work coming, it would find the target has been removed already, and report a
sysfs warning calltrace.
queue tail queue head
DISCE_DESTRUCT> PORTE_BYTES_DMAED event ->PHYE_LOSS_OF_SIGNAL[running]
There are other hotplug issues in current framework, in above case, if there is
hotadd sas event queued between hotremove works, the hotplug order would be
broken
and unexpected issues would happen.
In this patch, we try to solve these issues in following steps:
1. create a new workqueue used to run sas event work, instead of scsi host
workqueue,
because we may block sas event work, we cannot block the normal scsi works.
When libsas receive a phy down event, sas_deform_port would be called, and
now we
block sas_deform_port and wait for destruction work finish, in
sas_destruct_devices,
we may wait ata error handler, it would take a long time, so if do all stuff
in scsi
host workq, libsas may block other scsi works too long.
2. create a new workqueue used to run sas discovery events work, instead of
scsi host
workqueue, because in some cases, eg. in revalidate domain event, we may
unregister
a sas device and discover new one, we must sync the execution, wait the
remove process
finish, then start a new discovery. So we must put the probe and destruct
discovery
events in a new workqueue to avoid deadlock.
3. introudce a asd_sas_port level wait-complete and a sas_discovery level
wait-complete
we use former wait-complete to achieve a sas event atomic process and use
latter to
make a sas discovery sync.
4. remove disco_mutex in sas_revalidate_domain, since now sas_revalidate_domain
sync
the destruct discovery event execution, it's no need to lock disco mutex
there.
Signed-off-by: Yijing Wang
---
drivers/scsi/libsas/sas_discover.c | 58 ++--
drivers/scsi/libsas/sas_event.c| 2 +-
drivers/scsi/libsas/sas_expander.c | 9 +-
drivers/scsi/libsas/sas_init.c | 23 +-
drivers/scsi/libsas/sas_internal.h | 61 ++
drivers/scsi/libsas/sas_port.c | 4 +++
include/scsi/libsas.h | 9 ++
7 files changed, 148 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..43e8a1e 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -503,11 +503,10 @@ static void sas_revalidate_domain(struct work_struct
*work)
struct domain_device *ddev = port->port_dev;
/* prevent revalidation from finding sata links in recovery */
- mutex_lock(&ha->disco_mutex);
if (test_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state)) {
SAS_DPRINTK("REVALIDATION DEFERRED on port %d, pid:%d\n",
port->id, task_pid_nr(current));
- goto out;
+ return;
}
clear_bit(DISCE_REVALIDATE_DOMAIN, &port->disc.pending);
@@ -521,20 +520,57 @@ static void sas_revalidate_domain(struct work_struct
*work)
SAS_DPRINTK("done REVALIDATING DOMAIN on port %d, pid:%d, res 0x%x\n",
port->id, task_pid_nr(current), res);
- out:
- mutex_unlock(&ha->disco_mutex);
+}
+
+static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
+ [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
+ [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+ [DISCE_PROBE] = sas_probe_devices,
+ [DISCE_SUSPEND] = sas_suspend_devices,
+ [DISCE_RESUME] = sas_resume_devices,
+ [DISCE_DESTRUCT] = sas_destruct_devices,
+};
+
+/* a simple wrapper for sas discover event funtions */
+static void sas_discover_common_fn(struct work_struct *work)
+{
+ struct sas_discovery_event *ev = to_sas_discovery_event(work);
+ struct asd_sas_port *port = ev->port;
+
+ sas_event_fns[ev->type](work);
+ sas_unbusy_port(port);
}
/* -- Events --
[PATCH v2 0/2] Enhance libsas hotplug feature
Now the libsas hotplug has some issues, Dan Williams report a similar bug here before https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html The issues we have found 1. if LLDD burst reports lots of phy-up/phy-down sas events, some events may lost because a same sas events is pending now, finally libsas topo may different the hardware. 2. receive a phy down sas event, libsas call sas_deform_port to remove devices, it would first delete the sas port, then put a destruction discovery event in a new work, and queue it at the tail of workqueue, once the sas port be deleted, its children device will be deleted too, when the destruction work start, it will found the target device has been removed, and report a sysfs warnning. 3. since a hotplug process will be devided into several works, if a phy up sas event insert into phydown works, like destruction work ---> PORTE_BYTES_DMAED (sas_form_port) >PHYE_LOSS_OF_SIGNAL the hot remove flow would broken by PORTE_BYTES_DMAED event, it's not we expected, and issues would occur. The first patch fix the sas events lost, and the second one introudce wait-complete to fix the hotplug order issues. v1->v2: some code improvements suggested by John Garry Yijing Wang (2): libsas: Don't process sas events in static works libsas: Enhance libsas hotplug drivers/scsi/libsas/sas_discover.c | 58 +--- drivers/scsi/libsas/sas_event.c| 90 ++ drivers/scsi/libsas/sas_expander.c | 9 +++- drivers/scsi/libsas/sas_init.c | 29 +--- drivers/scsi/libsas/sas_internal.h | 64 +++ drivers/scsi/libsas/sas_phy.c | 45 --- drivers/scsi/libsas/sas_port.c | 22 +- include/scsi/libsas.h | 19 8 files changed, 232 insertions(+), 104 deletions(-) -- 2.5.0
[PATCH v2 1/2] libsas: Don't process sas events in static works
Now libsas hotplug work is static, LLDD driver queue
the hotplug work into shost->work_q. If LLDD driver
burst post lots hotplug events to libsas, the hotplug
events may pending in the workqueue like
shost->work_q
new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] ->
processing
|<---wait worker to process>|
In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
to shost->work_q, but this work is already pending, so it would be lost.
Finally, libsas delete the related sas port and sas devices, but LLDD driver
expect libsas add the sas port and devices(last sas event).
This patch remove the static defined hotplug work, and use dynamic work to
avoid missing hotplug events.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_event.c| 88 +++---
drivers/scsi/libsas/sas_init.c | 6 ---
drivers/scsi/libsas/sas_internal.h | 3 ++
drivers/scsi/libsas/sas_phy.c | 45 ---
drivers/scsi/libsas/sas_port.c | 18
include/scsi/libsas.h | 10 +
6 files changed, 84 insertions(+), 86 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index aadbd53..06c5c4b 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,6 +27,10 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
@@ -40,17 +44,14 @@ void sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
scsi_queue_work(ha->core.shost, &sw->work);
}
-static void sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static void sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
+ unsigned long flags;
- spin_lock_irqsave(&ha->lock, flags);
- sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
}
@@ -111,52 +112,87 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
+static void sas_ha_event_worker(struct work_struct *work)
+{
+ struct sas_ha_event *ev = to_sas_ha_event(work);
+
+ sas_ha_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_port_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_port_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_phy_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_phy_event_fns[ev->type](work);
+ kfree(ev);
+}
+
static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
{
+ struct sas_ha_event *ev;
+
BUG_ON(event >= HA_NUM_EVENTS);
- sas_queue_event(event, &sas_ha->pending,
- &sas_ha->ha_events[event].work, sas_ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_ha_event_worker);
+ ev->ha = sas_ha;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, sas_ha);
}
static void notify_port_event(struct asd_sas_phy *phy, enum port_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PORT_NUM_EVENTS);
- sas_queue_event(event, &phy->port_events_pending,
- &phy->port_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_port_event_worker);
+ ev->phy = phy;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, ha);
}
void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PHY_NUM_EVENTS);
- sas_queue_
[PATCH 2/2] libsas: Enhance libsas hotplug
Libsas complete a hotplug event notified by LLDD in several works,
for example, if libsas receive a PHYE_LOSS_OF_SIGNAL, we process it
in following steps:
notify_phy_event[interrupt context]
sas_queue_event [queue work on shost->work_q]
sas_phye_loss_of_signal [running in shost->work_q]
sas_deform_port [remove sas port]
sas_unregister_dev
sas_discover_event [queue destruct
work on shost->work_q tail]
In above case, complete whole hotplug in two works, remove sas port first, then
put the destruction of device in another work and queue it on in the tail of
workqueue, since sas port is the parent of the children rphy device, so if
remove
sas port first, the children rphy device would also be deleted, when the
destruction
work coming, it would find the target has been removed already, and report a
sysfs warning calltrace.
queue tail queue head
DISCE_DESTRUCT> PORTE_BYTES_DMAED event ->PHYE_LOSS_OF_SIGNAL[running]
There are other hotplug issues in current framework, in above case, if there is
hotadd sas event queued between hotremove works, the hotplug order would be
broken
and unexpected issues would happen.
In this patch, we try to solve these issues in following steps:
1. create a new workqueue used to run sas event work, instead of scsi host
workqueue,
because we may block sas event work, we cannot block the normal scsi works.
2. create a new workqueue used to run sas discovery events work, instead of
scsi host
workqueue, because in some cases, eg. in revalidate domain event, we may
unregister
a sas device and discover new one, we must sync the execution, wait the
remove process
finish, then start a new discovery. So we must put the probe and destruct
discovery
events in a new workqueue to avoid deadlock.
3. introudce a asd_sas_port level wait-complete and a sas_discovery level
wait-complete
we use former wait-complete to achieve a sas event atomic process and use
latter to
make a sas discovery sync.
4. remove disco_mutex in sas_revalidate_domain, since now sas_revalidate_domain
sync
the destruct discovery event execution, it's no need to lock disco mutex
there.
Signed-off-by: Yijing Wang
---
drivers/scsi/libsas/sas_discover.c | 58 --
drivers/scsi/libsas/sas_event.c| 2 +-
drivers/scsi/libsas/sas_expander.c | 9 +-
drivers/scsi/libsas/sas_init.c | 31 +++-
drivers/scsi/libsas/sas_internal.h | 50
drivers/scsi/libsas/sas_port.c | 4 +++
include/scsi/libsas.h | 11 +++-
7 files changed, 146 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..43e8a1e 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -503,11 +503,10 @@ static void sas_revalidate_domain(struct work_struct
*work)
struct domain_device *ddev = port->port_dev;
/* prevent revalidation from finding sata links in recovery */
- mutex_lock(&ha->disco_mutex);
if (test_bit(SAS_HA_ATA_EH_ACTIVE, &ha->state)) {
SAS_DPRINTK("REVALIDATION DEFERRED on port %d, pid:%d\n",
port->id, task_pid_nr(current));
- goto out;
+ return;
}
clear_bit(DISCE_REVALIDATE_DOMAIN, &port->disc.pending);
@@ -521,20 +520,57 @@ static void sas_revalidate_domain(struct work_struct
*work)
SAS_DPRINTK("done REVALIDATING DOMAIN on port %d, pid:%d, res 0x%x\n",
port->id, task_pid_nr(current), res);
- out:
- mutex_unlock(&ha->disco_mutex);
+}
+
+static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
+ [DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
+ [DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
+ [DISCE_PROBE] = sas_probe_devices,
+ [DISCE_SUSPEND] = sas_suspend_devices,
+ [DISCE_RESUME] = sas_resume_devices,
+ [DISCE_DESTRUCT] = sas_destruct_devices,
+};
+
+/* a simple wrapper for sas discover event funtions */
+static void sas_discover_common_fn(struct work_struct *work)
+{
+ struct sas_discovery_event *ev = to_sas_discovery_event(work);
+ struct asd_sas_port *port = ev->port;
+
+ sas_event_fns[ev->type](work);
+ sas_unbusy_port(port);
}
/* -- Events -- */
static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
+ int ret;
+ struct sas_discovery_event *ev = to_sas_discovery_event(&sw->work);
+ struct asd_sas_port *port = ev->port;
+
/* chained work is not subject to SA_HA_DRAINING or
* SAS_HA_REGISTERED
[PATCH 1/2] libsas: Don't process sas events in static works
Now libsas hotplug work is static, LLDD driver queue
the hotplug work into shost->work_q. If LLDD driver
burst post lots hotplug events to libsas, the hotplug
events may pending in the workqueue like
shost->work_q
new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] ->
processing
|<---wait worker to process>|
In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
to shost->work_q, but this work is already pending, so it would be lost.
Finally, libsas delete the related sas port and sas devices, but LLDD driver
expect libsas add the sas port and devices(last sas event).
This patch remove the static defined hotplug work, and use dynamic work to
avoid missing hotplug events.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_event.c| 88 +++---
drivers/scsi/libsas/sas_init.c | 6 ---
drivers/scsi/libsas/sas_internal.h | 3 ++
drivers/scsi/libsas/sas_phy.c | 45 ---
drivers/scsi/libsas/sas_port.c | 18
include/scsi/libsas.h | 10 +
6 files changed, 84 insertions(+), 86 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index aadbd53..06c5c4b 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,6 +27,10 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
@@ -40,17 +44,14 @@ void sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
scsi_queue_work(ha->core.shost, &sw->work);
}
-static void sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static void sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
+ unsigned long flags;
- spin_lock_irqsave(&ha->lock, flags);
- sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
}
@@ -111,52 +112,87 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
+static void sas_ha_event_worker(struct work_struct *work)
+{
+ struct sas_ha_event *ev = to_sas_ha_event(work);
+
+ sas_ha_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_port_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_port_event_fns[ev->type](work);
+ kfree(ev);
+}
+
+static void sas_phy_event_worker(struct work_struct *work)
+{
+ struct asd_sas_event *ev = to_asd_sas_event(work);
+
+ sas_phy_event_fns[ev->type](work);
+ kfree(ev);
+}
+
static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
{
+ struct sas_ha_event *ev;
+
BUG_ON(event >= HA_NUM_EVENTS);
- sas_queue_event(event, &sas_ha->pending,
- &sas_ha->ha_events[event].work, sas_ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_ha_event_worker);
+ ev->ha = sas_ha;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, sas_ha);
}
static void notify_port_event(struct asd_sas_phy *phy, enum port_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PORT_NUM_EVENTS);
- sas_queue_event(event, &phy->port_events_pending,
- &phy->port_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_port_event_worker);
+ ev->phy = phy;
+ ev->type = event;
+ sas_queue_event(event, &ev->work, ha);
}
void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PHY_NUM_EVENTS);
- sas_queue_
[PATCH 0/2] Enhance libsas hotplug feature
Now the libsas hotplug has some issues, Dan Williams report a similar bug here before https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html The issues we have found 1. if LLDD burst reports lots of phy-up/phy-down sas events, some events may lost because a same sas events is pending now, finally libsas topo may different the hardware. 2. receive a phy down sas event, libsas call sas_deform_port to remove devices, it would first delete the sas port, then put a destruction discovery event in a new work, and queue it at the tail of workqueue, once the sas port be deleted, its children device will be deleted too, when the destruction work start, it will found the target device has been removed, and report a sysfs warnning. 3. since a hotplug process will be devided into several works, if a phy up sas event insert into phydown works, like destruction work ---> PORTE_BYTES_DMAED (sas_form_port) >PHYE_LOSS_OF_SIGNAL the hot remove flow would broken by PORTE_BYTES_DMAED event, it's not we expected, and issues would occur. The first patch fix the sas events lost, and the second one introudce wait-complete to fix the hotplug order issues. Yijing Wang (2): libsas: Don't process sas events in static works libsas: Enhance libsas hotplug drivers/scsi/libsas/sas_discover.c | 58 +--- drivers/scsi/libsas/sas_event.c| 90 ++ drivers/scsi/libsas/sas_expander.c | 9 +++- drivers/scsi/libsas/sas_init.c | 37 +--- drivers/scsi/libsas/sas_internal.h | 53 ++ drivers/scsi/libsas/sas_phy.c | 45 --- drivers/scsi/libsas/sas_port.c | 22 +- include/scsi/libsas.h | 21 + 8 files changed, 230 insertions(+), 105 deletions(-) -- 2.5.0
[RFC][PATCH v2 1/2] libsas: Alloc dynamic work to avoid missing sas events
Now libsas hotplug work is static, LLDD driver queue
the hotplug work into shost->work_q. If LLDD driver
burst post lots hotplug event to libsas, the hotplug
events may pending in the workqueue like
shost->workq
tail | PHYE_LOSS_OF_SIGNAL | PORTE_BYTES_DMAED | head
In this case, if a new PORTE_BYTES_DMAED event coming,
it would be lost, because we can not queue a work which
is already pending in the workqueue, also libsas has a
pending bit to avoid queue the same event.
The lost hotplug event make something confusing, e.g.
we have sas disks connected hardware, but we can not
found them in kernel.
This patch remove the static defined hotplug work,
and use dynamic work to avoid missing hotplug events.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_event.c| 61 ++
drivers/scsi/libsas/sas_init.c | 5 +---
drivers/scsi/libsas/sas_internal.h | 3 ++
drivers/scsi/libsas/sas_phy.c | 50 +--
drivers/scsi/libsas/sas_port.c | 23 +++---
include/scsi/libsas.h | 8 -
6 files changed, 66 insertions(+), 84 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index aadbd53..091f5c4 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,6 +27,10 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
@@ -40,17 +44,14 @@ void sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
scsi_queue_work(ha->core.shost, &sw->work);
}
-static void sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static void sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
+ unsigned long flags;
- spin_lock_irqsave(&ha->lock, flags);
- sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
}
@@ -111,52 +112,60 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
{
+ struct sas_ha_event *ev;
+
BUG_ON(event >= HA_NUM_EVENTS);
- sas_queue_event(event, &sas_ha->pending,
- &sas_ha->ha_events[event].work, sas_ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_ha_event_fns[event]);
+ ev->ha = sas_ha;
+ sas_queue_event(event, &ev->work, sas_ha);
}
static void notify_port_event(struct asd_sas_phy *phy, enum port_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PORT_NUM_EVENTS);
- sas_queue_event(event, &phy->port_events_pending,
- &phy->port_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_port_event_fns[event]);
+ ev->phy = phy;
+ sas_queue_event(event, &ev->work, ha);
}
void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PHY_NUM_EVENTS);
- sas_queue_event(event, &phy->phy_events_pending,
- &phy->phy_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_phy_event_fns[event]);
+ ev->phy = phy;
+ sas_queue_event(event, &ev->work, ha);
}
int sas_init_events(struct sas_ha_struct *sas_ha)
{
- static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
- [HAE_RESET] = sas_hae_reset,
- };
-
- int i;
-
- for (i = 0; i < HA_NUM_EVENTS; i++) {
- INIT_SAS_WORK(&sas_ha->ha_events[i].work
[RFC][PATCH v2 0/2] Improve libsas hotplug
v1-v2: Fix memory allocation issue in interrupt context. Yijing Wang (2): libsas: Alloc dynamic work to avoid missing sas events libsas: Fix hotplug issue in libsas drivers/scsi/libsas/sas_ata.c | 34 ++--- drivers/scsi/libsas/sas_discover.c | 245 ++-- drivers/scsi/libsas/sas_event.c | 61 + drivers/scsi/libsas/sas_expander.c | 54 ++-- drivers/scsi/libsas/sas_init.c | 31 - drivers/scsi/libsas/sas_internal.h | 45 ++- drivers/scsi/libsas/sas_phy.c | 50 +++- drivers/scsi/libsas/sas_port.c | 35 -- drivers/scsi/libsas/sas_scsi_host.c | 23 include/scsi/libsas.h | 13 +- include/scsi/sas_ata.h | 4 +- 11 files changed, 404 insertions(+), 191 deletions(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC][PATCH v2 2/2] libsas: Fix hotplug issue in libsas
Now the libsas hotplug has some issues, Dan Williams report
a similar bug here:
https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html
The root cause of the issues is we use one workqueue(shost->work_q) to
process libsas event, and we divide a hot-on or hot-remove flow to several
events to process. E.g. we start a new work and queue it into the same
workqueue in sas_deform_port() to remove the children devices after
the sas port. So if there is one hot-on event between remove sas port
and destruct the children devices, some unexpected errors would
be caused.
This patch modify hotplug event process mechanism to solve the
hotplug problems in libsas. We move device add/del operation to
a new workqueue(named sas_dev_wq).
And we use sas_port_alloc_num to replace sas_port_alloc function
because when discovery is concurrently executing with the device
adding or destroying, the old sas port resource may have not
completely deleted, the new sas port resource of the same name
will be created, and this will cause calltrace about sysfs
device node.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_ata.c | 34 ++---
drivers/scsi/libsas/sas_discover.c | 245 ++--
drivers/scsi/libsas/sas_expander.c | 54 ++--
drivers/scsi/libsas/sas_init.c | 26 +++-
drivers/scsi/libsas/sas_internal.h | 46 ++-
drivers/scsi/libsas/sas_port.c | 12 +-
drivers/scsi/libsas/sas_scsi_host.c | 23
include/scsi/libsas.h | 5 +-
include/scsi/sas_ata.h | 4 +-
9 files changed, 340 insertions(+), 109 deletions(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 763f012..877efa8 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -619,32 +619,22 @@ static int sas_get_ata_command_set(struct domain_device
*dev)
return ata_dev_classify(&tf);
}
-void sas_probe_sata(struct asd_sas_port *port)
+void sas_probe_sata_device(struct domain_device *dev)
{
- struct domain_device *dev, *n;
-
- mutex_lock(&port->ha->disco_mutex);
- list_for_each_entry(dev, &port->disco_list, disco_list_node) {
- if (!dev_is_sata(dev))
- continue;
-
- ata_sas_async_probe(dev->sata_dev.ap);
- }
- mutex_unlock(&port->ha->disco_mutex);
+ struct asd_sas_port *port = dev->port;
- list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node) {
- if (!dev_is_sata(dev))
- continue;
+ if (!port || !port->ha || !dev_is_sata(dev))
+ return;
- sas_ata_wait_eh(dev);
+ ata_sas_async_probe(dev->sata_dev.ap);
- /* if libata could not bring the link up, don't surface
-* the device
-*/
- if (ata_dev_disabled(sas_to_ata_dev(dev)))
- sas_fail_probe(dev, __func__, -ENODEV);
- }
+ sas_ata_wait_eh(dev);
+ /* if libata could not bring the link up, don't surface
+* the device
+*/
+ if (ata_dev_disabled(sas_to_ata_dev(dev)))
+ sas_fail_probe(dev, __func__, -ENODEV);
}
static void sas_ata_flush_pm_eh(struct asd_sas_port *port, const char *func)
@@ -729,7 +719,7 @@ int sas_discover_sata(struct domain_device *dev)
if (res)
return res;
- sas_discover_event(dev->port, DISCE_PROBE);
+ sas_notify_device_event(dev, SAS_DEVICE_ADD);
return 0;
}
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..ea57c66 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -34,6 +34,12 @@
#include
#include "../scsi_sas_internal.h"
+
+static void sas_unregister_common_dev(struct asd_sas_port *port,
+ struct domain_device *dev);
+static void sas_unregister_fail_dev(struct asd_sas_port *port,
+ struct domain_device *dev);
+
/* -- Basic task processing for discovery purposes -- */
void sas_init_dev(struct domain_device *dev)
@@ -158,11 +164,8 @@ static int sas_get_port_device(struct asd_sas_port *port)
if (dev_is_sata(dev) || dev->dev_type == SAS_END_DEVICE)
list_add_tail(&dev->disco_list_node, &port->disco_list);
- else {
- spin_lock_irq(&port->dev_list_lock);
- list_add_tail(&dev->dev_list_node, &port->dev_list);
- spin_unlock_irq(&port->dev_list_lock);
- }
+ else
+ list_add_tail(&dev->dev_list_node, &port->expander_list);
spin_lock_irq(&port->phy_list_lock);
list_for_each_entry
[RFC][PATCH v1 2/2] libsas: Fix hotplug issue in libsas
Now the libsas hotplug has some issues, Dan Williams report
a similar bug here:
https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg39187.html
The root cause of the issues is we use one workqueue(shost->work_q) to
process libsas event, and we divide a hot-on or hot-remove flow to several
events to process. E.g. we start a new work and queue it into the same
workqueue in sas_deform_port() to remove the children devices after
the sas port. So if there is one hot-on event between remove sas port
and destruct the children devices, some unexpected errors would
be caused.
This patch modify hotplug event process mechanism to solve the
hotplug problems in libsas. We move device add/del operation to
a new workqueue(named sas_dev_wq).
And we use sas_port_alloc_num to replace sas_port_alloc function
because when discovery is concurrently executing with the device
adding or destroying, the old sas port resource may have not
completely deleted, the new sas port resource of the same name
will be created, and this will cause calltrace about sysfs
device node.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_ata.c | 34 ++---
drivers/scsi/libsas/sas_discover.c | 245 +-
drivers/scsi/libsas/sas_expander.c | 54 ++--
drivers/scsi/libsas/sas_init.c | 26 -
drivers/scsi/libsas/sas_internal.h | 46 ++-
drivers/scsi/libsas/sas_port.c | 12 ++-
drivers/scsi/libsas/sas_scsi_host.c | 23
include/scsi/libsas.h |5 +-
include/scsi/sas_ata.h |4 +-
9 files changed, 340 insertions(+), 109 deletions(-)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 763f012..877efa8 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -619,32 +619,22 @@ static int sas_get_ata_command_set(struct domain_device
*dev)
return ata_dev_classify(&tf);
}
-void sas_probe_sata(struct asd_sas_port *port)
+void sas_probe_sata_device(struct domain_device *dev)
{
- struct domain_device *dev, *n;
-
- mutex_lock(&port->ha->disco_mutex);
- list_for_each_entry(dev, &port->disco_list, disco_list_node) {
- if (!dev_is_sata(dev))
- continue;
-
- ata_sas_async_probe(dev->sata_dev.ap);
- }
- mutex_unlock(&port->ha->disco_mutex);
+ struct asd_sas_port *port = dev->port;
- list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node) {
- if (!dev_is_sata(dev))
- continue;
+ if (!port || !port->ha || !dev_is_sata(dev))
+ return;
- sas_ata_wait_eh(dev);
+ ata_sas_async_probe(dev->sata_dev.ap);
- /* if libata could not bring the link up, don't surface
-* the device
-*/
- if (ata_dev_disabled(sas_to_ata_dev(dev)))
- sas_fail_probe(dev, __func__, -ENODEV);
- }
+ sas_ata_wait_eh(dev);
+ /* if libata could not bring the link up, don't surface
+* the device
+*/
+ if (ata_dev_disabled(sas_to_ata_dev(dev)))
+ sas_fail_probe(dev, __func__, -ENODEV);
}
static void sas_ata_flush_pm_eh(struct asd_sas_port *port, const char *func)
@@ -729,7 +719,7 @@ int sas_discover_sata(struct domain_device *dev)
if (res)
return res;
- sas_discover_event(dev->port, DISCE_PROBE);
+ sas_notify_device_event(dev, SAS_DEVICE_ADD);
return 0;
}
diff --git a/drivers/scsi/libsas/sas_discover.c
b/drivers/scsi/libsas/sas_discover.c
index 60de662..ea57c66 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -34,6 +34,12 @@
#include
#include "../scsi_sas_internal.h"
+
+static void sas_unregister_common_dev(struct asd_sas_port *port,
+ struct domain_device *dev);
+static void sas_unregister_fail_dev(struct asd_sas_port *port,
+ struct domain_device *dev);
+
/* -- Basic task processing for discovery purposes -- */
void sas_init_dev(struct domain_device *dev)
@@ -158,11 +164,8 @@ static int sas_get_port_device(struct asd_sas_port *port)
if (dev_is_sata(dev) || dev->dev_type == SAS_END_DEVICE)
list_add_tail(&dev->disco_list_node, &port->disco_list);
- else {
- spin_lock_irq(&port->dev_list_lock);
- list_add_tail(&dev->dev_list_node, &port->dev_list);
- spin_unlock_irq(&port->dev_list_lock);
- }
+ else
+ list_add_tail(&dev->dev_list_node, &port->expander_list);
spin_lock_irq(&port->phy_list_lock);
list_for_each_entry
[RFC][PATCH v1 1/2] libsas: Alloc dynamic work to avoid missing sas events
Now libsas hotplug work is static, LLDD driver queue
the hotplug work into shost->work_q. If LLDD driver
burst post lots hotplug event to libsas, the hotplug
events may pending in the workqueue like
shost->workq
tail | PHYE_LOSS_OF_SIGNAL | PORTE_BYTES_DMAED | head
In this case, if a new PORTE_BYTES_DMAED event coming,
it would be lost, because we can not queue a work which
is already pending in the workqueue, also libsas has a
pending bit to avoid queue the same event.
The lost hotplug event make something confusing, e.g.
we have sas disks connected hardware, but we can not
found them in kernel.
This patch remove the static defined hotplug work,
and use dynamic work to avoid missing hotplug events.
Signed-off-by: Yijing Wang
Signed-off-by: Yousong He
Signed-off-by: Qilin Chen
---
drivers/scsi/libsas/sas_event.c| 61 ---
drivers/scsi/libsas/sas_init.c |5 +--
drivers/scsi/libsas/sas_internal.h |3 ++
drivers/scsi/libsas/sas_phy.c | 50 -
drivers/scsi/libsas/sas_port.c | 23 --
include/scsi/libsas.h |8 -
6 files changed, 66 insertions(+), 84 deletions(-)
diff --git a/drivers/scsi/libsas/sas_event.c b/drivers/scsi/libsas/sas_event.c
index aadbd53..993156c 100644
--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c
@@ -27,6 +27,10 @@
#include "sas_internal.h"
#include "sas_dump.h"
+static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
+ [HAE_RESET] = sas_hae_reset,
+};
+
void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw)
{
if (!test_bit(SAS_HA_REGISTERED, &ha->state))
@@ -40,17 +44,14 @@ void sas_queue_work(struct sas_ha_struct *ha, struct
sas_work *sw)
scsi_queue_work(ha->core.shost, &sw->work);
}
-static void sas_queue_event(int event, unsigned long *pending,
- struct sas_work *work,
+static void sas_queue_event(int event, struct sas_work *work,
struct sas_ha_struct *ha)
{
- if (!test_and_set_bit(event, pending)) {
- unsigned long flags;
+ unsigned long flags;
- spin_lock_irqsave(&ha->lock, flags);
- sas_queue_work(ha, work);
- spin_unlock_irqrestore(&ha->lock, flags);
- }
+ spin_lock_irqsave(&ha->lock, flags);
+ sas_queue_work(ha, work);
+ spin_unlock_irqrestore(&ha->lock, flags);
}
@@ -111,52 +112,60 @@ void sas_enable_revalidation(struct sas_ha_struct *ha)
if (!test_and_clear_bit(ev, &d->pending))
continue;
- sas_queue_event(ev, &d->pending, &d->disc_work[ev].work, ha);
+ sas_queue_event(ev, &d->disc_work[ev].work, ha);
}
mutex_unlock(&ha->disco_mutex);
}
static void notify_ha_event(struct sas_ha_struct *sas_ha, enum ha_event event)
{
+ struct sas_ha_event *ev;
+
BUG_ON(event >= HA_NUM_EVENTS);
- sas_queue_event(event, &sas_ha->pending,
- &sas_ha->ha_events[event].work, sas_ha);
+ ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_ha_event_fns[event]);
+ ev->ha = sas_ha;
+ sas_queue_event(event, &ev->work, sas_ha);
}
static void notify_port_event(struct asd_sas_phy *phy, enum port_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PORT_NUM_EVENTS);
- sas_queue_event(event, &phy->port_events_pending,
- &phy->port_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_port_event_fns[event]);
+ ev->phy = phy;
+ sas_queue_event(event, &ev->work, ha);
}
void sas_notify_phy_event(struct asd_sas_phy *phy, enum phy_event event)
{
+ struct asd_sas_event *ev;
struct sas_ha_struct *ha = phy->ha;
BUG_ON(event >= PHY_NUM_EVENTS);
- sas_queue_event(event, &phy->phy_events_pending,
- &phy->phy_events[event].work, ha);
+ ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+ if (!ev)
+ return;
+
+ INIT_SAS_WORK(&ev->work, sas_phy_event_fns[event]);
+ ev->phy = phy;
+ sas_queue_event(event, &ev->work, ha);
}
int sas_init_events(struct sas_ha_struct *sas_ha)
{
- static const work_func_t sas_ha_event_fns[HA_NUM_EVENTS] = {
- [HAE_RESET] = sas_hae_reset,
- };
-
- int i;
-
- for (i = 0; i < HA_NUM_EVENTS; i++) {
- INIT_SAS_WORK(&sas_ha->ha_events[i].work
Re: [PATCH v2 1/6] scsi/bfa: use pcie_set/get_readrq to simplify code
On 2013/9/7 6:14, Bjorn Helgaas wrote:
> On Thu, Sep 05, 2013 at 03:55:25PM +0800, Yijing Wang wrote:
>> v1->v2: use pcie_get/set_readrq to simplify code
>> a lot suggestd by Bjorn.
>>
>> Use pcie_get_readrq()/pcie_set_readrq() to simplify
>> code.
>>
>> Signed-off-by: Yijing Wang
>> Cc: Jiang Liu
>> Cc: Anil Gurumurthy
>> Cc: Vijaya Mohan Guvva
>> Cc: "James E.J. Bottomley"
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>> drivers/scsi/bfa/bfad.c | 48
>> +-
>> 1 files changed, 6 insertions(+), 42 deletions(-)
Hi Bjorn,
Thanks for your patience guidance! Now I know more about how to write
a patch or patchset. I will check my patch follow the advices below before
I send out every time. Make it easier for you and other maintainers to
review and apply.
>
> I applied all these with some tweaks to my pci/yijing-pci_is_pcie-v2
> branch [1]. This will be rebased after v3.12-rc1, and may be amended
> if any patches are picked up by others.
>
> Hints (not just for you; I hope other people pay attention, too,
> because I'm obsessive and I pay attention to these details):
>
> - Include a "[PATCH v2 0/6]" email. That's a good place for you to
> put an overall description of the series, and a good place for
> responses like this one that apply to the whole series.
>
>> -
>> -pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> -if (mask != 0x && pcie_cap_reg) {
>> -pcie_cap_reg += 0x08;
>> -pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
>> -if ((pcie_dev_ctl & 0x7000) != mask) {
>> -printk(KERN_WARNING "BFA[%s]: "
>> +if (pcie_max_read_reqsz > 0 && pci_is_pcie(pdev)) {
>> +int max_rq = pcie_get_readrq(pdev);
>> +if (max_rq > 128 && max_rq < 4096 && is_power_of_2(max_rq))
>
> I think you meant to validate pcie_max_read_reqsz (the module parameter),
> not max_rq. I made this change on my branch.
Yes, thanks for your fix.
Thanks!
Yijing.
>
>> +printk(KERN_WARNING "BFA[%s]: "
>> "pcie_max_read_request_size is %d, "
>> -"reset to %d\n", bfad->pci_name,
>> -(1 << ((pcie_dev_ctl & 0x7000) >> 12)) << 7,
>> +"reset to %d\n", bfad->pci_name, max_rq,
>> pcie_max_read_reqsz);
>> -
>> -pcie_dev_ctl &= ~0x7000;
>> -pci_write_config_word(pdev, pcie_cap_reg,
>> -pcie_dev_ctl | mask);
>> -}
>> -}
>> +pcie_set_readrq(pdev, pcie_max_read_reqsz);
>> }
>>
>> pci_save_state(pdev);
>> --
>> 1.7.1
>>
>>
>
> .
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/6] scsi/qla2xxx: use pcie_is_pcie() to simplify code
Use pci_is_pcie() instead of pci_find_capability
to simplify code.
Acked-by: Chad Dupuis
Signed-off-by: Yijing Wang
Cc: Andrew Vasquez
Cc: linux-dri...@qlogic.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/qla2xxx/qla_mr.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index 2482975..5494d27 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -507,7 +507,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
pci_write_config_word(ha->pdev, PCI_COMMAND, w);
/* PCIe -- adjust Maximum Read Request Size (2048). */
- if (pci_find_capability(ha->pdev, PCI_CAP_ID_EXP))
+ if (pci_is_pcie(ha->pdev))
pcie_set_readrq(ha->pdev, 2048);
ha->chip_revision = ha->pdev->revision;
@@ -660,10 +660,8 @@ char *
qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
{
struct qla_hw_data *ha = vha->hw;
- int pcie_reg;
- pcie_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
- if (pcie_reg) {
+ if (pci_is_pcie(ha->pdev)) {
strcpy(str, "PCIe iSA");
return str;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/6] scsi/csiostor: use pcie_capability_xxx to simplify code
v1->v2: add #define for Completion Timeout Value, and use
pcie_capability_clear_and_set_word() instead suggested by Bjorn.
Pcie_capability_xxx() interfaces were introduced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.
Signed-off-by: Yijing Wang
Cc: Jiang Liu
Cc: "James E.J. Bottomley"
Cc: Naresh Kumar Inna
Cc: "David S. Miller"
Cc: Jesper Juhl
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/csiostor/csio_hw.c | 14 +++---
include/uapi/linux/pci_regs.h |1 +
2 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 0eb35b9..07f493a 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -855,17 +855,9 @@ csio_hw_get_flash_params(struct csio_hw *hw)
static void
csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
{
- uint16_t val;
- int pcie_cap;
-
- if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
- pci_read_config_word(hw->pdev,
-pcie_cap + PCI_EXP_DEVCTL2, &val);
- val &= 0xfff0;
- val |= range ;
- pci_write_config_word(hw->pdev,
- pcie_cap + PCI_EXP_DEVCTL2, val);
- }
+ if (pci_is_pcie(hw->pdev))
+ pcie_capability_clear_and_set_word(hw->pdev, PCI_EXP_DEVCTL2,
+ PCI_EXP_DEVCTL2_COMP_TIME, range);
}
/*/
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index baa7852..cd74182 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -558,6 +558,7 @@
#define PCI_EXP_DEVCAP2_OBFF_MSG 0x0004 /* New message signaling */
#define PCI_EXP_DEVCAP2_OBFF_WAKE 0x0008 /* Re-use WAKE# for OBFF */
#define PCI_EXP_DEVCTL240 /* Device Control 2 */
+#define PCI_EXP_DEVCTL2_COMP_TIME 0x0f/* Completion Timeout Value */
#define PCI_EXP_DEVCTL2_ARI 0x20/* Alternative Routing-ID */
#define PCI_EXP_DEVCTL2_IDO_REQ_EN0x0100 /* Allow IDO for requests */
#define PCI_EXP_DEVCTL2_IDO_CMP_EN0x0200 /* Allow IDO for completions */
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/6] scsi/bfa: use pcie_set/get_readrq to simplify code
v1->v2: use pcie_get/set_readrq to simplify code
a lot suggestd by Bjorn.
Use pcie_get_readrq()/pcie_set_readrq() to simplify
code.
Signed-off-by: Yijing Wang
Cc: Jiang Liu
Cc: Anil Gurumurthy
Cc: Vijaya Mohan Guvva
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/bfa/bfad.c | 48 +-
1 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index f8ca7be..0a458db 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -766,50 +766,14 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
bfad->pcidev = pdev;
/* Adjust PCIe Maximum Read Request Size */
- if (pcie_max_read_reqsz > 0) {
- int pcie_cap_reg;
- u16 pcie_dev_ctl;
- u16 mask = 0x;
-
- switch (pcie_max_read_reqsz) {
- case 128:
- mask = 0x0;
- break;
- case 256:
- mask = 0x1000;
- break;
- case 512:
- mask = 0x2000;
- break;
- case 1024:
- mask = 0x3000;
- break;
- case 2048:
- mask = 0x4000;
- break;
- case 4096:
- mask = 0x5000;
- break;
- default:
- break;
- }
-
- pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
- if (mask != 0x && pcie_cap_reg) {
- pcie_cap_reg += 0x08;
- pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
- if ((pcie_dev_ctl & 0x7000) != mask) {
- printk(KERN_WARNING "BFA[%s]: "
+ if (pcie_max_read_reqsz > 0 && pci_is_pcie(pdev)) {
+ int max_rq = pcie_get_readrq(pdev);
+ if (max_rq > 128 && max_rq < 4096 && is_power_of_2(max_rq))
+ printk(KERN_WARNING "BFA[%s]: "
"pcie_max_read_request_size is %d, "
- "reset to %d\n", bfad->pci_name,
- (1 << ((pcie_dev_ctl & 0x7000) >> 12)) << 7,
+ "reset to %d\n", bfad->pci_name, max_rq,
pcie_max_read_reqsz);
-
- pcie_dev_ctl &= ~0x7000;
- pci_write_config_word(pdev, pcie_cap_reg,
- pcie_dev_ctl | mask);
- }
- }
+ pcie_set_readrq(pdev, pcie_max_read_reqsz);
}
pci_save_state(pdev);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code
On 2013/9/4 7:43, Bjorn Helgaas wrote:
> On Tue, Sep 03, 2013 at 03:35:10PM +0800, Yijing Wang wrote:
>> Pcie_capability_xxx() interfaces were introudced to
>
> s/introudced/introduced/
Will update it.
>
>> simplify code to access PCIe Cap config space. And
>> because PCI core saves the PCIe Cap offset in
>> set_pcie_port_type() when device is enumerated.
>> So we can use pci_is_pcie() instead.
>>
>> Signed-off-by: Yijing Wang
>> Cc: Jiang Liu
>> Cc: "James E.J. Bottomley"
>> Cc: Naresh Kumar Inna
>> Cc: "David S. Miller"
>> Cc: Jesper Juhl
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>> drivers/scsi/csiostor/csio_hw.c |9 +++--
>> 1 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/csiostor/csio_hw.c
>> b/drivers/scsi/csiostor/csio_hw.c
>> index 0eb35b9..be9a6ef 100644
>> --- a/drivers/scsi/csiostor/csio_hw.c
>> +++ b/drivers/scsi/csiostor/csio_hw.c
>> @@ -856,15 +856,12 @@ static void
>> csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
>> {
>> uint16_t val;
>> -int pcie_cap;
>>
>> -if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
>> -pci_read_config_word(hw->pdev,
>> - pcie_cap + PCI_EXP_DEVCTL2, &val);
>> +if (pci_is_pcie(hw->pdev)) {
>> +pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
>> val &= 0xfff0;
>> val |= range ;
>> -pci_write_config_word(hw->pdev,
>> - pcie_cap + PCI_EXP_DEVCTL2, val);
>> +pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
>
> Please add a #define for the Completion Timeout Value field and use
> pcie_capability_clear_and_set_word() instead of writing it out the
> long way here.
Will update it, thanks!
>
>> }
>> }
>>
>> --
>> 1.7.1
>>
>>
>
> .
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code
>> @@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>> break;
>> }
>>
>> -pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
>> -if (mask != 0x && pcie_cap_reg) {
>> -pcie_cap_reg += 0x08;
>> -pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
>> +if (mask != 0x && pci_is_pcie(pdev)) {
>
> Please move the pci_is_pcie() test up to the
> "if (pcie_mas_read_reqsz ..." statement. There's no point in doing
> the switch statement if this isn't a PCIe device.
Right, will update.
>
>> +pcie_capability_read_word(pdev, PCI_EXP_DEVCTL,
>> &pcie_dev_ctl);
>> if ((pcie_dev_ctl & 0x7000) != mask) {
>> printk(KERN_WARNING "BFA[%s]: "
>> "pcie_max_read_request_size is %d, "
>> @@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
>> pcie_max_read_reqsz);
>>
>> pcie_dev_ctl &= ~0x7000;
>> -pci_write_config_word(pdev, pcie_cap_reg,
>> +pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
>> pcie_dev_ctl | mask);
>
> Please rework this to use pcie_set_readrq() instead of writing
> the capability directly. If we write the capability directly, we
> risk writing a value that is incompatible with the MPS
> configuration done by the PCI core.
Ah, this code is Long-winded, use pcie_set_readrq()/pcie_get_readrq() can
simplify
this code much.
Thanks!
Yijing.
>
>> }
>> }
>> --
>> 1.7.1
>>
>>
>
> .
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] scsi/bfa: use pcie_capability_xxx to simplify code
Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.
Signed-off-by: Yijing Wang
Cc: Jiang Liu
Cc: Anil Gurumurthy
Cc: Vijaya Mohan Guvva
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/bfa/bfad.c |9 +++--
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 9611195..d726b81 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -767,7 +767,6 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
/* Adjust PCIe Maximum Read Request Size */
if (pcie_max_read_reqsz > 0) {
- int pcie_cap_reg;
u16 pcie_dev_ctl;
u16 mask = 0x;
@@ -794,10 +793,8 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
break;
}
- pcie_cap_reg = pci_find_capability(pdev, PCI_CAP_ID_EXP);
- if (mask != 0x && pcie_cap_reg) {
- pcie_cap_reg += 0x08;
- pci_read_config_word(pdev, pcie_cap_reg, &pcie_dev_ctl);
+ if (mask != 0x && pci_is_pcie(pdev)) {
+ pcie_capability_read_word(pdev, PCI_EXP_DEVCTL,
&pcie_dev_ctl);
if ((pcie_dev_ctl & 0x7000) != mask) {
printk(KERN_WARNING "BFA[%s]: "
"pcie_max_read_request_size is %d, "
@@ -806,7 +803,7 @@ bfad_pci_init(struct pci_dev *pdev, struct bfad_s *bfad)
pcie_max_read_reqsz);
pcie_dev_ctl &= ~0x7000;
- pci_write_config_word(pdev, pcie_cap_reg,
+ pcie_capability_write_word(pdev, PCI_EXP_DEVCTL,
pcie_dev_ctl | mask);
}
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] scsi/qla2xxx: use pcie_is_pcie() to simplify code
Use pci_is_pcie() instead of pci_find_capability
to simplify code.
Signed-off-by: Yijing Wang
Cc: Andrew Vasquez
Cc: linux-dri...@qlogic.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/qla2xxx/qla_mr.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index d799379..5f74271 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -507,7 +507,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
pci_write_config_word(ha->pdev, PCI_COMMAND, w);
/* PCIe -- adjust Maximum Read Request Size (2048). */
- if (pci_find_capability(ha->pdev, PCI_CAP_ID_EXP))
+ if (pci_is_pcie(ha->pdev))
pcie_set_readrq(ha->pdev, 2048);
ha->chip_revision = ha->pdev->revision;
@@ -660,10 +660,8 @@ char *
qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str)
{
struct qla_hw_data *ha = vha->hw;
- int pcie_reg;
- pcie_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
- if (pcie_reg) {
+ if (pci_is_pcie(ha->pdev)) {
strcpy(str, "PCIe iSA");
return str;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] scsi/csiostor: use pcie_capability_xxx to simplify code
Pcie_capability_xxx() interfaces were introudced to
simplify code to access PCIe Cap config space. And
because PCI core saves the PCIe Cap offset in
set_pcie_port_type() when device is enumerated.
So we can use pci_is_pcie() instead.
Signed-off-by: Yijing Wang
Cc: Jiang Liu
Cc: "James E.J. Bottomley"
Cc: Naresh Kumar Inna
Cc: "David S. Miller"
Cc: Jesper Juhl
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/csiostor/csio_hw.c |9 +++--
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 0eb35b9..be9a6ef 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -856,15 +856,12 @@ static void
csio_set_pcie_completion_timeout(struct csio_hw *hw, u8 range)
{
uint16_t val;
- int pcie_cap;
- if (!csio_pci_capability(hw->pdev, PCI_CAP_ID_EXP, &pcie_cap)) {
- pci_read_config_word(hw->pdev,
-pcie_cap + PCI_EXP_DEVCTL2, &val);
+ if (pci_is_pcie(hw->pdev)) {
+ pcie_capability_read_word(hw->pdev, PCI_EXP_DEVCTL2, &val);
val &= 0xfff0;
val |= range ;
- pci_write_config_word(hw->pdev,
- pcie_cap + PCI_EXP_DEVCTL2, val);
+ pcie_capability_write_word(hw->pdev, PCI_EXP_DEVCTL2, val);
}
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/16] megaraid: clean up unnecessary MSI/MSI-X capability find
>> PCI_MSIX_FLAGS, >>control & >>~PCI_MSIX_FLAGS_ENABLE); >> } >> -- >> 1.7.1 >> >> > Acked-by: Sumit Saxena Hi James, Can you merge this patch? Thanks! Yijing. > > -- Thanks! Yijing -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/16] pm8001: clean up unnecessary MSI/MSI-X capability find
On 2013/8/9 10:57, lindar_liu wrote: > On 08/08/2013 9:10 PM, Yijing Wang wrote: >> PCI core will initialize device MSI/MSI-X capability in >> pci_msi_init_pci_dev(). So device driver should use pci_dev- >>> msi_cap/msix_cap to determine whether the device support MSI/MSI-X >> instead of using pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX). >> Access to PCIe device config space again will consume more time. >> > It's fine. > You can add my Acked-by. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/16] pmcraid: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: Anil Ravindranath
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/pmcraid.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 1eb7b02..d5b3315 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -4678,8 +4678,7 @@ pmcraid_register_interrupt_handler(struct
pmcraid_instance *pinstance)
int rc;
struct pci_dev *pdev = pinstance->pdev;
- if ((pmcraid_enable_msix) &&
- (pci_find_capability(pdev, PCI_CAP_ID_MSIX))) {
+ if (pmcraid_enable_msix && pdev->msix_cap) {
int num_hrrq = PMCRAID_NUM_MSIX_VECTORS;
struct msix_entry entries[PMCRAID_NUM_MSIX_VECTORS];
int i;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/16] hpsa: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: "Stephen M. Cameron"
Cc: "James E.J. Bottomley"
Cc: iss_storage...@hp.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/hpsa.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 7f4f790..05b31f1 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4090,7 +4090,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h)
if ((h->board_id == 0x40700E11) || (h->board_id == 0x40800E11) ||
(h->board_id == 0x40820E11) || (h->board_id == 0x40830E11))
goto default_int_mode;
- if (pci_find_capability(h->pdev, PCI_CAP_ID_MSIX)) {
+ if (h->pdev->msix_cap) {
dev_info(&h->pdev->dev, "MSIX\n");
err = pci_enable_msix(h->pdev, hpsa_msix_entries,
MAX_REPLY_QUEUES);
@@ -4110,7 +4110,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h)
goto default_int_mode;
}
}
- if (pci_find_capability(h->pdev, PCI_CAP_ID_MSI)) {
+ if (h->pdev->msi_cap) {
dev_info(&h->pdev->dev, "MSI\n");
if (!pci_enable_msi(h->pdev))
h->msi_vector = 1;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/16] pm8001: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: xjtu...@gmail.com
Cc: lindar_...@usish.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/pm8001/pm8001_init.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c
b/drivers/scsi/pm8001/pm8001_init.c
index 3861aa1..1ee61e8 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -734,7 +734,7 @@ static u32 pm8001_request_irq(struct pm8001_hba_info
*pm8001_ha)
pdev = pm8001_ha->pdev;
#ifdef PM8001_USE_MSIX
- if (pci_find_capability(pdev, PCI_CAP_ID_MSIX))
+ if (pdev->msix_cap)
return pm8001_setup_msix(pm8001_ha);
else {
PM8001_INIT_DBG(pm8001_ha,
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/16] mpt3sas: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: Nagalakshmi Nandigama
Cc: Sreekanth Reddy
Cc: supp...@lsi.com
Cc: "James E.J. Bottomley"
Cc: dl-mptfusionli...@lsi.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/mpt3sas/mpt3sas_base.c |7 +++
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 5dc280c..e756191 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1520,11 +1520,9 @@ _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc,
struct pci_dev *pdev)
static int
_base_check_enable_msix(struct MPT3SAS_ADAPTER *ioc)
{
- int base;
u16 message_control;
- base = pci_find_capability(ioc->pdev, PCI_CAP_ID_MSIX);
- if (!base) {
+ if (!ioc->pdev->msix_cap) {
dfailprintk(ioc, pr_info(MPT3SAS_FMT "msix not supported\n",
ioc->name));
return -EINVAL;
@@ -1532,7 +1530,8 @@ _base_check_enable_msix(struct MPT3SAS_ADAPTER *ioc)
/* get msix vector count */
- pci_read_config_word(ioc->pdev, base + 2, &message_control);
+ pci_read_config_word(ioc->pdev, ioc->pdev->msix_cap + 2,
+ &message_control);
ioc->msix_vector_count = (message_control & 0x3FF) + 1;
if (ioc->msix_vector_count > 8)
ioc->msix_vector_count = 8;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/16] mpt2sas: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: Nagalakshmi Nandigama
Cc: Sreekanth Reddy
Cc: supp...@lsi.com
Cc: "James E.J. Bottomley"
Cc: dl-mptfusionli...@lsi.com
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/mpt2sas/mpt2sas_base.c |7 +++
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index ccd6d5a..35c3676 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1217,7 +1217,6 @@ _base_config_dma_addressing(struct MPT2SAS_ADAPTER *ioc,
struct pci_dev *pdev)
static int
_base_check_enable_msix(struct MPT2SAS_ADAPTER *ioc)
{
- int base;
u16 message_control;
@@ -1228,8 +1227,7 @@ _base_check_enable_msix(struct MPT2SAS_ADAPTER *ioc)
return -EINVAL;
}
- base = pci_find_capability(ioc->pdev, PCI_CAP_ID_MSIX);
- if (!base) {
+ if (!ioc->pdev->msix_cap) {
dfailprintk(ioc, printk(MPT2SAS_INFO_FMT "msix not "
"supported\n", ioc->name));
return -EINVAL;
@@ -1246,7 +1244,8 @@ _base_check_enable_msix(struct MPT2SAS_ADAPTER *ioc)
ioc->pdev->device == MPI2_MFGPAGE_DEVID_SAS2116_2)
ioc->msix_vector_count = 1;
else {
- pci_read_config_word(ioc->pdev, base + 2, &message_control);
+ pci_read_config_word(ioc->pdev, ioc->pdev->msix_cap + 2,
+ &message_control);
ioc->msix_vector_count = (message_control & 0x3FF) + 1;
}
dinitprintk(ioc, printk(MPT2SAS_INFO_FMT "msix is supported, "
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/16] megaraid: clean up unnecessary MSI/MSI-X capability find
PCI core will initialize device MSI/MSI-X capability in
pci_msi_init_pci_dev(). So device driver should use
pci_dev->msi_cap/msix_cap to determine whether the device
support MSI/MSI-X instead of using
pci_find_capability(pci_dev, PCI_CAP_ID_MSI/MSIX).
Access to PCIe device config space again will consume more time.
Signed-off-by: Yijing Wang
Cc: Neela Syam Kolli
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/megaraid/megaraid_sas_base.c |9 -
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 0177295..ad8fc44 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4072,21 +4072,20 @@ fail_set_dma_mask:
static int megasas_probe_one(struct pci_dev *pdev,
const struct pci_device_id *id)
{
- int rval, pos, i, j;
+ int rval, i, j;
struct Scsi_Host *host;
struct megasas_instance *instance;
u16 control = 0;
/* Reset MSI-X in the kdump kernel */
if (reset_devices) {
- pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
- if (pos) {
- pci_read_config_word(pdev, pos + PCI_MSIX_FLAGS,
+ if (pdev->msix_cap) {
+ pci_read_config_word(pdev, pdev->msix_cap +
PCI_MSIX_FLAGS,
&control);
if (control & PCI_MSIX_FLAGS_ENABLE) {
dev_info(&pdev->dev, "resetting MSI-X\n");
pci_write_config_word(pdev,
- pos + PCI_MSIX_FLAGS,
+ pdev->msix_cap +
PCI_MSIX_FLAGS,
control &
~PCI_MSIX_FLAGS_ENABLE);
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Resend with Ack][PATCH 8/9] scsi/pm8001: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
in init path. So we can use pdev->pm_cap instead of using
pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified
code.
Acked-by: Lindar Liu
Tested-by: Lindar Liu
Signed-off-by: Yijing Wang
Cc: xjtu...@gmail.com
Cc: lindar_...@usish.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/pm8001/pm8001_init.c |7 +++
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c
b/drivers/scsi/pm8001/pm8001_init.c
index e4b9bc7..3861aa1 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -912,14 +912,13 @@ static int pm8001_pci_suspend(struct pci_dev *pdev,
pm_message_t state)
{
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha;
- int i , pos;
+ int i;
u32 device_state;
pm8001_ha = sha->lldd_ha;
flush_workqueue(pm8001_wq);
scsi_block_requests(pm8001_ha->shost);
- pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
- if (pos == 0) {
- printk(KERN_ERR " PCI PM not supported\n");
+ if (!pdev->pm_cap) {
+ dev_err(&pdev->dev, " PCI PM not supported\n");
return -ENODEV;
}
PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] scsi/pm8001: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
On 2013/6/26 17:33, Jack Wang wrote:
> On 06/18/2013 10:23 AM, Yijing Wang wrote:
>> Pci core has been saved pm cap register offset by pdev->pm_cap in
>> pci_pm_init()
>> in init path. So we can use pdev->pm_cap instead of using
>> pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and
>> simplified code.
>>
> I think Lindar already replied to you, and tested it on hardware.
I didn't receive the reply, I will check my email client, maybe I lose some
emails.
Jack, Lindar thank you very much.
>
> So you can add her:
> Acked-by or Tested-by
ok, I will add them.
>
>
> Regards,
> Jack
>> Signed-off-by: Yijing Wang
>> Cc: xjtu...@gmail.com
>> Cc: lindar_...@usish.com
>> Cc: "James E.J. Bottomley"
>> Cc: linux-scsi@vger.kernel.org
>> Cc: linux-ker...@vger.kernel.org
>> ---
>> drivers/scsi/pm8001/pm8001_init.c |7 +++
>> 1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/pm8001/pm8001_init.c
>> b/drivers/scsi/pm8001/pm8001_init.c
>> index e4b9bc7..3861aa1 100644
>> --- a/drivers/scsi/pm8001/pm8001_init.c
>> +++ b/drivers/scsi/pm8001/pm8001_init.c
>> @@ -912,14 +912,13 @@ static int pm8001_pci_suspend(struct pci_dev *pdev,
>> pm_message_t state)
>> {
>> struct sas_ha_struct *sha = pci_get_drvdata(pdev);
>> struct pm8001_hba_info *pm8001_ha;
>> -int i , pos;
>> +int i;
>> u32 device_state;
>> pm8001_ha = sha->lldd_ha;
>> flush_workqueue(pm8001_wq);
>> scsi_block_requests(pm8001_ha->shost);
>> -pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
>> -if (pos == 0) {
>> -printk(KERN_ERR " PCI PM not supported\n");
>> +if (!pdev->pm_cap) {
>> +dev_err(&pdev->dev, " PCI PM not supported\n");
>> return -ENODEV;
>> }
>> PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
>>
>
>
> .
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] scsi/pm8001: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
in init path. So we can use pdev->pm_cap instead of using
pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified
code.
Signed-off-by: Yijing Wang
Cc: xjtu...@gmail.com
Cc: lindar_...@usish.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/pm8001/pm8001_init.c |7 +++
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c
b/drivers/scsi/pm8001/pm8001_init.c
index e4b9bc7..3861aa1 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -912,14 +912,13 @@ static int pm8001_pci_suspend(struct pci_dev *pdev,
pm_message_t state)
{
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha;
- int i , pos;
+ int i;
u32 device_state;
pm8001_ha = sha->lldd_ha;
flush_workqueue(pm8001_wq);
scsi_block_requests(pm8001_ha->shost);
- pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
- if (pos == 0) {
- printk(KERN_ERR " PCI PM not supported\n");
+ if (!pdev->pm_cap) {
+ dev_err(&pdev->dev, " PCI PM not supported\n");
return -ENODEV;
}
PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/9] scsi/pm8001: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
Hi,
Any comments?
On 2013/6/18 16:23, Yijing Wang wrote:
> Pci core has been saved pm cap register offset by pdev->pm_cap in
> pci_pm_init()
> in init path. So we can use pdev->pm_cap instead of using
> pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and
> simplified code.
>
> Signed-off-by: Yijing Wang
> Cc: xjtu...@gmail.com
> Cc: lindar_...@usish.com
> Cc: "James E.J. Bottomley"
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> drivers/scsi/pm8001/pm8001_init.c |7 +++
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c
> b/drivers/scsi/pm8001/pm8001_init.c
> index e4b9bc7..3861aa1 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -912,14 +912,13 @@ static int pm8001_pci_suspend(struct pci_dev *pdev,
> pm_message_t state)
> {
> struct sas_ha_struct *sha = pci_get_drvdata(pdev);
> struct pm8001_hba_info *pm8001_ha;
> - int i , pos;
> + int i;
> u32 device_state;
> pm8001_ha = sha->lldd_ha;
> flush_workqueue(pm8001_wq);
> scsi_block_requests(pm8001_ha->shost);
> - pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
> - if (pos == 0) {
> - printk(KERN_ERR " PCI PM not supported\n");
> + if (!pdev->pm_cap) {
> + dev_err(&pdev->dev, " PCI PM not supported\n");
> return -ENODEV;
> }
> PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
>
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] scsi/pm8001: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
in init path. So we can use pdev->pm_cap instead of using
pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified
code.
Signed-off-by: Yijing Wang
Cc: xjtu...@gmail.com
Cc: lindar_...@usish.com
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/pm8001/pm8001_init.c |7 +++
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c
b/drivers/scsi/pm8001/pm8001_init.c
index e4b9bc7..3861aa1 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -912,14 +912,13 @@ static int pm8001_pci_suspend(struct pci_dev *pdev,
pm_message_t state)
{
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct pm8001_hba_info *pm8001_ha;
- int i , pos;
+ int i;
u32 device_state;
pm8001_ha = sha->lldd_ha;
flush_workqueue(pm8001_wq);
scsi_block_requests(pm8001_ha->shost);
- pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
- if (pos == 0) {
- printk(KERN_ERR " PCI PM not supported\n");
+ if (!pdev->pm_cap) {
+ dev_err(&pdev->dev, " PCI PM not supported\n");
return -ENODEV;
}
PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] hpsa: rework pci pm related code for simplification
Use pci core pm interface to simplify code.
Signed-off-by: Yijing Wang
Cc: "Stephen M. Cameron"
Cc: "James E.J. Bottomley"
Cc: iss_storage...@hp.com
Cc: linux-scsi@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
drivers/scsi/hpsa.c | 16 +++-
1 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 7f4f790..f5ba6d4 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3753,9 +3753,6 @@ static int hpsa_message(struct pci_dev *pdev, unsigned
char opcode,
static int hpsa_controller_hard_reset(struct pci_dev *pdev,
void * __iomem vaddr, u32 use_doorbell)
{
- u16 pmcsr;
- int pos;
-
if (use_doorbell) {
/* For everything after the P600, the PCI power state method
* of resetting the controller doesn't work, so we have this
@@ -3773,8 +3770,7 @@ static int hpsa_controller_hard_reset(struct pci_dev
*pdev,
* this causes a secondary PCI reset which will reset the
* controller." */
- pos = pci_find_capability(pdev, PCI_CAP_ID_PM);
- if (pos == 0) {
+ if (!pdev->pm_cap) {
dev_err(&pdev->dev,
"hpsa_reset_controller: "
"PCI PM not supported\n");
@@ -3782,18 +3778,12 @@ static int hpsa_controller_hard_reset(struct pci_dev
*pdev,
}
dev_info(&pdev->dev, "using PCI PM to reset controller\n");
/* enter the D3hot power management state */
- pci_read_config_word(pdev, pos + PCI_PM_CTRL, &pmcsr);
- pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
- pmcsr |= PCI_D3hot;
- pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr);
+ pci_set_power_state(pdev, PCI_D3hot);
msleep(500);
/* enter the D0 power management state */
- pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
- pmcsr |= PCI_D0;
- pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr);
-
+ pci_set_power_state(pdev, PCI_D0);
/*
* The P600 requires a small delay when changing states.
* Otherwise we may think the board did not reset and we bail.
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/20 9:47, Yijing Wang wrote: >>> We compared the two situations after BIOS initialization, and found Max >>> Payload Size in DEVCTRL is 256B >>> if FC card had been installed, if the slot is empty, Max Payload Size is >>> 128B. We force it to be 128B when >>> FC card installed when system boot up. Finally pci hotplug becomes ok. So I >>> suspect maybe our PCIe hardware >>> has problem supporting 256B. >> >> Ah, this sounds like something I've been worried about for a while, >> i.e., do we handle MPS correctly when we hot-add devices? >> >> Yijing, I'm not quite clear on what you're observing. I guess you're >> saying that if an FC card is installed at boot, the BIOS sets MPS to >> 256, and that if no FC card is installed, the BIOS sets MPS to 128? > > Yes. > >> You haven't mentioned any Linux boot options, so I assume you haven't >> tried any. Does "pci=pcie_bus_safe" make any difference? > > Yes, I don't add any Linux boot options before. I will try this boot option > "pci=pcie_bus_safe". > >> Jon, here's a pointer to the beginning of the thread: >> http://marc.info/?l=linux-pci&m=134770460302298&w=2 (full dmesg log at >> http://marc.info/?l=linux-scsi&m=134788365823217&w=2). I'm not sure >> we have enough in the dmesg log to diagnose an issue like this. I >> wonder if it would be useful to log the current setting, so we could >> notice BIOS default differences like this one. >> Hi Bjorn, After add "pci=pcie_bus_safe" boot options, Hot-plug for FC card is OK. It's great! Thanks for your help! Giri, it's not the fc card problem, we fix this problem by add boot option "pci=pcie_bus_safe" which force the MPS of the entire system to the smallest possible. Thanks Yijing >> Bjorn >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-pci" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> . >> > > -- Thanks! Yijing -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/19 21:39, Bjorn Helgaas wrote: > On Tue, Sep 18, 2012 at 7:50 PM, Yijing Wang wrote: >> On 2012/9/19 1:54, Bjorn Helgaas wrote: >>> On Mon, Sep 17, 2012 at 6:06 AM, Yijing Wang wrote: >>>> On 2012/9/16 11:30, Bjorn Helgaas wrote: >>>>> On Sat, Sep 15, 2012 at 4:22 AM, Yijing Wang >>>>> wrote: >>>>>> Hi all, >>>>>>I encountered a very strange problem when I hot plug a fiber channel >>>>>> card(using qla2xxx driver). >>>>>> I did the hotplug in arch x86 machine, using pciehp driver for hotplug, >>>>>> this platform supports pci hot-plug triggering from both >>>>>> sysfs and attention button. If a hot-plug slot is empty when system >>>>>> boot-up, then hotplug FC card in this slot is ok. >>>>>> If a hot-plug slot has been embeded a FC card when system boot-up, >>>>>> hot-remove this card is ok, but hot-add this card will fail. >>>>>> I used >>>>>> #modprobe qla2xxx ql2xextended_error_logging=0x7fff >>>>>> to get all probe info. As bellow: >>>>>> >>>>>> Can anyone give me any suggestion for this problem? >>>>> >>>>> It sounds like you did this: >>>>> >>>>> 1) Power down system >>>>> 2) Remove FC card from slot >>>>> 3) Boot system >>>>> 4) Hot-add FC card >>>>> 5) Load qla2xxx driver >>>>> 6) qla2xxx driver claims FC card >>>>> 7) FC card works correctly >>>>> >>>>> 8) Power down system >>>>> 9) Install FC card in slot >>>>> 10) Boot system >>>>> 11) Load qla2xxx driver >>>>> 12) qla2xxx driver claims FC card >>>>> 13) FC card works correctly >>>> I rmmod qla2xxx driver here and modprobe qla2xxx >>>> ql2xextended_error_logging=0x1e40 again for get errors info >>>> Also I modprobe pciehp pciehp_debug=1 for getting debug info >>>>> 14) Hot-remove card >>>>> 15) Hot-add card >>>>> 16) qla2xxx driver claims FC card >>>>> 17) FC card does not work >>>>> >>>>> and I assume the dmesg log you included is just from steps 15 and 16 >>>>> (correct me if I'm wrong). >>>>> >>>>> It would be useful to see the entire log showing all these events so >>>>> we can compare the working cases with the non-working one. If you use >>>>> the pciehp_debug module parameter, we should also see some pciehp >>>>> events that would help me understand that driver. >>>>> >>>> >>>> Hi Bjorn, >>>>Thanks for your comments very much! >>>> >>>> My steps: >>>> 1) power down system >>>> 2) Install FC card in slot >>>> 3) Boot system >>>> 4) Load qla2xxx driver >>>> 5) qla2xxx driver claims FC card >>>> 6) FC card works correctly(at least probe return ok, I don't know qla2xxx >>>> driver much..) >>>> 7) rmmod qla2xxx >>>> 8) modprobe qla2xxx ql2xextended_error_logging=0x1e40(for get errors >>>> info) >>>> 9) modprobe pciehp pciehp_debug=1 >>>> 10) Hot-remove card >>>> 11) Hot-add card >>>> 12) qla2xxx driver claims FC card fail(probe return fail, setup chip fail) >>>> --so this is failed situation-- >>>> >>>> --continue to hot-add fc card into >>>> empty slot(also support pci hp) >>>> 13) Install FC card in empty slot >>>> 14) Hot-add card >>>> 15) qla2xxx driver claims FC card ok (probe return ok) >>>> >>>> btw: >>>> If fc card firmware version 4.03, everything is ok (hot-plug in any >>>> slots(empty or not)) >>>> fc card firmware version is 4.04 or 5.04 , situation as same as 1)--->12) >>> >>> Thanks. The FW change is a good clue. If everything works with >>> version 4.03, but it doesn't work with version 4.04, it's likely to be >>> a FW problem, not a Linux PCI core problem. >>> >>> Here's what I see from your logs. In slot 4 (bus 08), the card was >>> present before boot, you removed it, re-added it, and it failed after >>> being re-added. Slot 3
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/19 23:31, Jiang Liu wrote: > On 09/19/2012 09:39 PM, Bjorn Helgaas wrote: >> On Tue, Sep 18, 2012 at 7:50 PM, Yijing Wang wrote: >>> On 2012/9/19 1:54, Bjorn Helgaas wrote: >>>> On Mon, Sep 17, 2012 at 6:06 AM, Yijing Wang wrote: >>>>> On 2012/9/16 11:30, Bjorn Helgaas wrote: >>>>>> On Sat, Sep 15, 2012 at 4:22 AM, Yijing Wang >>>>>> wrote: >>>>>>> Hi all, >>>>>>>I encountered a very strange problem when I hot plug a fiber channel >>>>>>> card(using qla2xxx driver). >>>>>>> I did the hotplug in arch x86 machine, using pciehp driver for hotplug, >>>>>>> this platform supports pci hot-plug triggering from both >>>>>>> sysfs and attention button. If a hot-plug slot is empty when system >>>>>>> boot-up, then hotplug FC card in this slot is ok. >>>>>>> If a hot-plug slot has been embeded a FC card when system boot-up, >>>>>>> hot-remove this card is ok, but hot-add this card will fail. >>>>>>> I used >>>>>>> #modprobe qla2xxx ql2xextended_error_logging=0x7fff >>>>>>> to get all probe info. As bellow: >>>>>>> >>>>>>> Can anyone give me any suggestion for this problem? >>>>>> >>>>>> It sounds like you did this: >>>>>> >>>>>> 1) Power down system >>>>>> 2) Remove FC card from slot >>>>>> 3) Boot system >>>>>> 4) Hot-add FC card >>>>>> 5) Load qla2xxx driver >>>>>> 6) qla2xxx driver claims FC card >>>>>> 7) FC card works correctly >>>>>> >>>>>> 8) Power down system >>>>>> 9) Install FC card in slot >>>>>> 10) Boot system >>>>>> 11) Load qla2xxx driver >>>>>> 12) qla2xxx driver claims FC card >>>>>> 13) FC card works correctly >>>>> I rmmod qla2xxx driver here and modprobe qla2xxx >>>>> ql2xextended_error_logging=0x1e40 again for get errors info >>>>> Also I modprobe pciehp pciehp_debug=1 for getting debug info >>>>>> 14) Hot-remove card >>>>>> 15) Hot-add card >>>>>> 16) qla2xxx driver claims FC card >>>>>> 17) FC card does not work >>>>>> >>>>>> and I assume the dmesg log you included is just from steps 15 and 16 >>>>>> (correct me if I'm wrong). >>>>>> >>>>>> It would be useful to see the entire log showing all these events so >>>>>> we can compare the working cases with the non-working one. If you use >>>>>> the pciehp_debug module parameter, we should also see some pciehp >>>>>> events that would help me understand that driver. >>>>>> >>>>> >>>>> Hi Bjorn, >>>>>Thanks for your comments very much! >>>>> >>>>> My steps: >>>>> 1) power down system >>>>> 2) Install FC card in slot >>>>> 3) Boot system >>>>> 4) Load qla2xxx driver >>>>> 5) qla2xxx driver claims FC card >>>>> 6) FC card works correctly(at least probe return ok, I don't know qla2xxx >>>>> driver much..) >>>>> 7) rmmod qla2xxx >>>>> 8) modprobe qla2xxx ql2xextended_error_logging=0x1e40(for get errors >>>>> info) >>>>> 9) modprobe pciehp pciehp_debug=1 >>>>> 10) Hot-remove card >>>>> 11) Hot-add card >>>>> 12) qla2xxx driver claims FC card fail(probe return fail, setup chip fail) >>>>> --so this is failed >>>>> situation-- >>>>> >>>>> --continue to hot-add fc card into >>>>> empty slot(also support pci hp) >>>>> 13) Install FC card in empty slot >>>>> 14) Hot-add card >>>>> 15) qla2xxx driver claims FC card ok (probe return ok) >>>>> >>>>> btw: >>>>> If fc card firmware version 4.03, everything is ok (hot-plug in any >>>>> slots(empty or not)) >>>>> fc card firmware version is 4.04 or 5.04 , situation as same as 1)--->12) >>>> >>>> Thanks. The FW chan
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/19 7:49, Giridhar Malavali wrote: > > > On 9/18/12 10:54 AM, "Bjorn Helgaas" wrote: > >> On Mon, Sep 17, 2012 at 6:06 AM, Yijing Wang >> wrote: >>> On 2012/9/16 11:30, Bjorn Helgaas wrote: >>>> On Sat, Sep 15, 2012 at 4:22 AM, Yijing Wang >>>> wrote: >>>>> Hi all, >>>>>I encountered a very strange problem when I hot plug a fiber >>>>> channel card(using qla2xxx driver). >>>>> I did the hotplug in arch x86 machine, using pciehp driver for >>>>> hotplug, this platform supports pci hot-plug triggering from both >>>>> sysfs and attention button. If a hot-plug slot is empty when system >>>>> boot-up, then hotplug FC card in this slot is ok. >>>>> If a hot-plug slot has been embeded a FC card when system boot-up, >>>>> hot-remove this card is ok, but hot-add this card will fail. >>>>> I used >>>>> #modprobe qla2xxx ql2xextended_error_logging=0x7fff >>>>> to get all probe info. As bellow: >>>>> >>>>> Can anyone give me any suggestion for this problem? >>>> >>>> It sounds like you did this: >>>> >>>> 1) Power down system >>>> 2) Remove FC card from slot >>>> 3) Boot system >>>> 4) Hot-add FC card >>>> 5) Load qla2xxx driver >>>> 6) qla2xxx driver claims FC card >>>> 7) FC card works correctly >>>> >>>> 8) Power down system >>>> 9) Install FC card in slot >>>> 10) Boot system >>>> 11) Load qla2xxx driver >>>> 12) qla2xxx driver claims FC card >>>> 13) FC card works correctly >>> I rmmod qla2xxx driver here and modprobe qla2xxx >>> ql2xextended_error_logging=0x1e40 again for get errors info >>> Also I modprobe pciehp pciehp_debug=1 for getting debug info >>>> 14) Hot-remove card >>>> 15) Hot-add card >>>> 16) qla2xxx driver claims FC card >>>> 17) FC card does not work >>>> >>>> and I assume the dmesg log you included is just from steps 15 and 16 >>>> (correct me if I'm wrong). >>>> >>>> It would be useful to see the entire log showing all these events so >>>> we can compare the working cases with the non-working one. If you use >>>> the pciehp_debug module parameter, we should also see some pciehp >>>> events that would help me understand that driver. >>>> >>> >>> Hi Bjorn, >>>Thanks for your comments very much! >>> >>> My steps: >>> 1) power down system >>> 2) Install FC card in slot >>> 3) Boot system >>> 4) Load qla2xxx driver >>> 5) qla2xxx driver claims FC card >>> 6) FC card works correctly(at least probe return ok, I don't know >>> qla2xxx driver much..) >>> 7) rmmod qla2xxx >>> 8) modprobe qla2xxx ql2xextended_error_logging=0x1e40(for get >>> errors info) >>> 9) modprobe pciehp pciehp_debug=1 >>> 10) Hot-remove card >>> 11) Hot-add card >>> 12) qla2xxx driver claims FC card fail(probe return fail, setup chip >>> fail) >>> --so this is failed >>> situation-- >>> >>> --continue to hot-add fc card into >>> empty slot(also support pci hp) >>> 13) Install FC card in empty slot >>> 14) Hot-add card >>> 15) qla2xxx driver claims FC card ok (probe return ok) >>> >>> btw: >>> If fc card firmware version 4.03, everything is ok (hot-plug in any >>> slots(empty or not)) >>> fc card firmware version is 4.04 or 5.04 , situation as same as >>> 1)--->12) > > That's good data pointer. Let me follow up with firmware team and get back > to you. > Hi Giri, We Found that this problem(hot-plug fail for 4.04 and 4.05 fw fc card) is generated by Max Payload Size in DEVCTRL. The MPS was 256B when hot plug fail situation occurs. If manually force it set to 128B, everything will be ok. So maybe our hardware have some problems. Thanks! Yijing > -- Giri >> >> Thanks. The FW change is a good clue. If everything works with >> version 4.03, but it doesn't work with version 4.04, it's likely to be >> a FW problem, not a Linux PCI core problem. >> >> Here's what I see from your logs. In slot 4 (bus 08), the card was >> present before boot,
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/19 1:54, Bjorn Helgaas wrote: > On Mon, Sep 17, 2012 at 6:06 AM, Yijing Wang wrote: >> On 2012/9/16 11:30, Bjorn Helgaas wrote: >>> On Sat, Sep 15, 2012 at 4:22 AM, Yijing Wang wrote: >>>> Hi all, >>>>I encountered a very strange problem when I hot plug a fiber channel >>>> card(using qla2xxx driver). >>>> I did the hotplug in arch x86 machine, using pciehp driver for hotplug, >>>> this platform supports pci hot-plug triggering from both >>>> sysfs and attention button. If a hot-plug slot is empty when system >>>> boot-up, then hotplug FC card in this slot is ok. >>>> If a hot-plug slot has been embeded a FC card when system boot-up, >>>> hot-remove this card is ok, but hot-add this card will fail. >>>> I used >>>> #modprobe qla2xxx ql2xextended_error_logging=0x7fff >>>> to get all probe info. As bellow: >>>> >>>> Can anyone give me any suggestion for this problem? >>> >>> It sounds like you did this: >>> >>> 1) Power down system >>> 2) Remove FC card from slot >>> 3) Boot system >>> 4) Hot-add FC card >>> 5) Load qla2xxx driver >>> 6) qla2xxx driver claims FC card >>> 7) FC card works correctly >>> >>> 8) Power down system >>> 9) Install FC card in slot >>> 10) Boot system >>> 11) Load qla2xxx driver >>> 12) qla2xxx driver claims FC card >>> 13) FC card works correctly >> I rmmod qla2xxx driver here and modprobe qla2xxx >> ql2xextended_error_logging=0x1e40 again for get errors info >> Also I modprobe pciehp pciehp_debug=1 for getting debug info >>> 14) Hot-remove card >>> 15) Hot-add card >>> 16) qla2xxx driver claims FC card >>> 17) FC card does not work >>> >>> and I assume the dmesg log you included is just from steps 15 and 16 >>> (correct me if I'm wrong). >>> >>> It would be useful to see the entire log showing all these events so >>> we can compare the working cases with the non-working one. If you use >>> the pciehp_debug module parameter, we should also see some pciehp >>> events that would help me understand that driver. >>> >> >> Hi Bjorn, >>Thanks for your comments very much! >> >> My steps: >> 1) power down system >> 2) Install FC card in slot >> 3) Boot system >> 4) Load qla2xxx driver >> 5) qla2xxx driver claims FC card >> 6) FC card works correctly(at least probe return ok, I don't know qla2xxx >> driver much..) >> 7) rmmod qla2xxx >> 8) modprobe qla2xxx ql2xextended_error_logging=0x1e40(for get errors >> info) >> 9) modprobe pciehp pciehp_debug=1 >> 10) Hot-remove card >> 11) Hot-add card >> 12) qla2xxx driver claims FC card fail(probe return fail, setup chip fail) >> --so this is failed situation-- >> >> --continue to hot-add fc card into empty >> slot(also support pci hp) >> 13) Install FC card in empty slot >> 14) Hot-add card >> 15) qla2xxx driver claims FC card ok (probe return ok) >> >> btw: >> If fc card firmware version 4.03, everything is ok (hot-plug in any >> slots(empty or not)) >> fc card firmware version is 4.04 or 5.04 , situation as same as 1)--->12) > > Thanks. The FW change is a good clue. If everything works with > version 4.03, but it doesn't work with version 4.04, it's likely to be > a FW problem, not a Linux PCI core problem. > > Here's what I see from your logs. In slot 4 (bus 08), the card was > present before boot, you removed it, re-added it, and it failed after > being re-added. Slot 3 (bus 06) was empty at boot, you hot-added a > card, and it worked. Here are the resources available on those two > buses and the boot-time config of the first device in slot 4: > > pci :00:07.0: PCI bridge to [bus 06-07] > pci :00:07.0: bridge window [io 0xc000-0xcfff] > pci :00:07.0: bridge window [mem 0xf900-0xf9ff] > pci :00:07.0: bridge window [mem 0xf100-0xf1ff 64bit pref] > pci :00:09.0: PCI bridge to [bus 08-09] > pci :00:09.0: bridge window [io 0xb000-0xbfff] > pci :00:09.0: bridge window [mem 0xf800-0xf8ff] > pci :00:09.0: bridge window [mem 0xf000-0xf0ff 64bit pref] > pci :08:00.0: [1077:2532] type 00 class 0x0c0400 > pci :08:00.0: reg
Re: Fail to probe qla2xxx fiber channel card while doing pci hotplug
On 2012/9/18 2:15, Giridhar Malavali wrote: > > > On 9/17/12 5:06 AM, "Yijing Wang" wrote: > >> [ 769.531289] qla2xxx [:08:00.0]-001d: : Found an ISP2532 irq 32 >> iobase 0xc90016bcc000. >> [ 769.531748] qla2xxx :08:00.0: irq 91 for MSI/MSI-X >> [ 769.531768] qla2xxx :08:00.0: irq 92 for MSI/MSI-X >> [ 799.503673] qla2xxx [:08:00.0]-d008:10: No buffer available for >> dump. >> [ 799.503682] qla2xxx [:08:00.0]-101c:10: Mailbox cmd timeout >> occurred, cmd=0xb, mb[0]=0xb, eeh_busy=0x0. Scheduling ISP abort. >> [ 799.503689] qla2xxx [:08:00.0]-0098:10: Failed to load segment 0 >> of firmware. >> [ 829.853242] qla2xxx [:08:00.0]-d008:10: No buffer available for >> dump. >> [ 829.853249] qla2xxx [:08:00.0]-008f:10: Failed to load segment 0 >> of firmware. >> [ 829.853253] qla2xxx [:08:00.0]-00cf:10: Setup chip FAILED. >> [ 829.853258] qla2xxx [:08:00.0]-00d6:10: Failed to initialize >> adapter - Adapter flags 2. >> [ 829.853893] qla2xxx :08:00.1: enabling device (0104 -> 0106) >> [ 829.855251] qla2xxx [:08:00.1]-001d: : Found an ISP2532 irq 42 >> iobase 0xc90016b88000. >> [ 829.855828] qla2xxx :08:00.1: irq 91 for MSI/MSI-X >> [ 829.855849] qla2xxx :08:00.1: irq 92 for MSI/MSI-X >> [ 859.827388] qla2xxx [:08:00.1]-d008:11: No buffer available for >> dump. >> [ 859.827395] qla2xxx [:08:00.1]-101c:11: Mailbox cmd timeout >> occurred, cmd=0xb, mb[0]=0xb, eeh_busy=0x0. Scheduling ISP abort. >> [ 859.827400] qla2xxx [:08:00.1]-0098:11: Failed to load segment 0 >> of firmware. >> [ 890.176829] qla2xxx [:08:00.1]-d008:11: No buffer available for >> dump. >> [ 890.176837] qla2xxx [:08:00.1]-008f:11: Failed to load segment 0 >> of firmware. >> [ 890.176841] qla2xxx [:08:00.1]-00cf:11: Setup chip FAILED. >> [ 890.176845] qla2xxx [:08:00.1]-00d6:11: Failed to initialize >> adapter - Adapter flags 2. > > I see that the one of the initial mailbox which loads data from system > memory to RISC RAM failing causing the probe to fail. > > -- Giri > Hi Giri, Thanks for your comments very much! Because I hot-add FC card in empty slot is ok, so I think maybe FC card and driver have no problem(I'm not sure). So I want to know what hardware(or other) problem may result mailbox loads data from system memory to RISC RAM fail? Thanks Yijing > > > > . > -- Thanks! Yijing -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fail to probe qla2xxx fiber channel card while doing pci hotplug
Hi all, I encountered a very strange problem when I hot plug a fiber channel card(using qla2xxx driver). I did the hotplug in arch x86 machine, using pciehp driver for hotplug, this platform supports pci hot-plug triggering from both sysfs and attention button. If a hot-plug slot is empty when system boot-up, then hotplug FC card in this slot is ok. If a hot-plug slot has been embeded a FC card when system boot-up, hot-remove this card is ok, but hot-add this card will fail. I used #modprobe qla2xxx ql2xextended_error_logging=0x7fff to get all probe info. As bellow: Can anyone give me any suggestion for this problem? Thanks very much! [ 511.011780] pci :06:00.0: [1077:2532] type 0 class 0x000c04 [ 511.011803] pci :06:00.0: reg 10: [io 0x-0x00ff] [ 511.011821] pci :06:00.0: reg 14: [mem 0x-0x3fff 64bit] [ 511.011856] pci :06:00.0: reg 30: [mem 0x-0x0003 pref] [ 511.011922] pci :06:00.1: [1077:2532] type 0 class 0x000c04 [ 511.011937] pci :06:00.1: reg 10: [io 0x-0x00ff] [ 511.011953] pci :06:00.1: reg 14: [mem 0x-0x3fff 64bit] [ 511.011989] pci :06:00.1: reg 30: [mem 0x-0x0003 pref] [ 511.019751] pci :06:00.0: BAR 6: assigned [mem 0xf100-0xf103 pref] [ 511.019759] pci :06:00.1: BAR 6: assigned [mem 0xf104-0xf107 pref] [ 511.019765] pci :06:00.0: BAR 1: assigned [mem 0xf800-0xf8003fff 64bit] [ 511.019775] pci :06:00.0: BAR 1: set to [mem 0xf800-0xf8003fff 64bit] (PCI address [0xf800-0xf8003fff]) [ 511.019781] pci :06:00.1: BAR 1: assigned [mem 0xf8004000-0xf8007fff 64bit] [ 511.019791] pci :06:00.1: BAR 1: set to [mem 0xf8004000-0xf8007fff 64bit] (PCI address [0xf8004000-0xf8007fff]) [ 511.019797] pci :06:00.0: BAR 0: assigned [io 0xc000-0xc0ff] [ 511.019804] pci :06:00.0: BAR 0: set to [io 0xc000-0xc0ff] (PCI address [0xc000-0xc0ff]) [ 511.019809] pci :06:00.1: BAR 0: assigned [io 0xc400-0xc4ff] [ 511.019816] pci :06:00.1: BAR 0: set to [io 0xc400-0xc4ff] (PCI address [0xc400-0xc4ff]) [ 511.019822] pcieport :00:07.0: PCI bridge to [bus 06-07] [ 511.019827] pcieport :00:07.0: bridge window [io 0xc000-0xcfff] [ 511.019834] pcieport :00:07.0: bridge window [mem 0xf800-0xf8ff] [ 511.019840] pcieport :00:07.0: bridge window [mem 0xf100-0xf1ff 64bit pref] [ 511.019856] pcieport :00:07.0: setting latency timer to 64 [ 511.020226] qla2xxx [:06:00.0]-0807: : Mem only adapter. [ 511.020232] qla2xxx [:06:00.0]-0808: : Bars=66. [ 511.020243] qla2xxx :06:00.0: enabling device (0100 -> 0102) [ 511.020256] qla2xxx :06:00.0: PCI INT A -> GSI 30 (level, low) -> IRQ 30 [ 511.020270] qla2xxx [:06:00.0]-080a: : Memory allocated for ha=88084c6ae000. [ 511.020281] qla2xxx [:06:00.0]-080b: : device_type=0x9c000800 port=1 fw_srisc_address=0010. [ 511.021139] qla2xxx [:06:00.0]-081c: : MSIX Count:2. [ 511.021142] qla2xxx [:06:00.0]-001d: : Found an ISP2532 irq 30 iobase 0xc900060c2000. [ 511.021147] qla2xxx [:06:00.0]-081e: : mbx_count=32, req_length=2048, rsp_length=512, max_loop_id=2047, init_cb_size=5252, gid_list_info_size=8, optrom_size=2097152, nvram_npiv_size=256, . [ 511.021152] qla2xxx [:06:00.0]-081f: : isp_ops=a04745e0, flash_conf_off=2147287040, flash_data_off=2146435072, nvram_conf_off=2147418112, nvram_data_off=2147352576. [ 511.021164] qla2xxx [:06:00.0]-0820: : 64 Bit addressing is enable. [ 511.021207] qla2xxx [:06:00.0]-0822: : init_cb=88084e48a000 gid_list=88084ebd5000, srb_mempool=88084b0b5440 s_dma_pool=88084d9b5840. [ 511.021223] qla2xxx [:06:00.0]-0827: : ms_iocb=88084e2a ct_sns=88084f132000. [ 511.021332] qla2xxx [:06:00.0]-082c: : req=88084e72 req->length=2048 req->ring=88084b14 rsp=88084d9b5ac0 rsp->length=512 rsp->ring=88084f30. [ 511.021582] qla2xxx [:06:00.0]-0841:11: Allocated the host=88084b3f4000 hw=88084c6ae000 vha=88084b3f4670 dev_name=:06:00.0 [ 511.021594] qla2xxx [:06:00.0]-0832:11: can_queue=2176, req=88084e72, mgmt_svr_loop_id=10, sg_tablesize=1024. [ 511.021608] qla2xxx [:06:00.0]-0833:11: max_id=512 this_id=255 cmd_per_len=3 unique_id=11 max_cmd_len=16 max_channel=0 max_lun=65535 transportt=88084d7ad000, vendor_id=4215. [ 511.021721] qla2xxx :06:00.0: irq 83 for MSI/MSI-X [ 511.021749] qla2xxx :06:00.0: irq 84 for MSI/MSI-X [ 511.021814] qla2xxx [:06:00.0]-c805:11: mqiobase= (null), max_rsp_queues=1, max_req_queues=1. [ 511.021823] qla2xxx [:06:00.0]-0855:11: mqiobase= (null), max_rsp_queues=1, max_req_queues=1. [ 511.021831] qla2xxx [:06:00.0]-0836:11: MSI-X: Enabled (0x0, 0x0). [ 511.021886] qla2xxx [:06:00.0]-c809:11: rsp_q_map=88084d0176a0 req_q_map=88084d017720 rsp->req=ff
