* [PATCH 01/11] libfcoe: Remove mutex_trylock/restart_syscall checks
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
@ 2011-04-01 23:05 ` Robert Love
2011-04-01 23:05 ` [PATCH 02/11] fcoe: " Robert Love
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:05 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain
This code was incorrectly ported from fcoe.c when the
fcoe transport infrastructure was put into place. It
was originally needed in fcoe.c when dealing with
the rtnl mutex. In that code it was only needed to
avoid a lockdep false positive. In libfcoe we don't
deal with the rtnl mutex, we don't get the lockdep
false positive and therefore we don't need these
checks.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
---
drivers/scsi/fcoe/fcoe_transport.c | 36 +++++++++++-------------------------
1 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index 2586841..7b61d00 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -530,9 +530,6 @@ static int fcoe_transport_create(const char *buffer, struct kernel_param *kp)
struct fcoe_transport *ft = NULL;
enum fip_state fip_mode = (enum fip_state)(long)kp->arg;
- if (!mutex_trylock(&ft_mutex))
- return restart_syscall();
-
#ifdef CONFIG_LIBFCOE_MODULE
/*
* Make sure the module has been initialized, and is not about to be
@@ -543,6 +540,8 @@ static int fcoe_transport_create(const char *buffer, struct kernel_param *kp)
goto out_nodev;
#endif
+ mutex_lock(&ft_mutex);
+
netdev = fcoe_if_to_netdev(buffer);
if (!netdev) {
LIBFCOE_TRANSPORT_DBG("Invalid device %s.\n", buffer);
@@ -586,10 +585,7 @@ out_putdev:
dev_put(netdev);
out_nodev:
mutex_unlock(&ft_mutex);
- if (rc == -ERESTARTSYS)
- return restart_syscall();
- else
- return rc;
+ return rc;
}
/**
@@ -608,9 +604,6 @@ static int fcoe_transport_destroy(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
- if (!mutex_trylock(&ft_mutex))
- return restart_syscall();
-
#ifdef CONFIG_LIBFCOE_MODULE
/*
* Make sure the module has been initialized, and is not about to be
@@ -621,6 +614,8 @@ static int fcoe_transport_destroy(const char *buffer, struct kernel_param *kp)
goto out_nodev;
#endif
+ mutex_lock(&ft_mutex);
+
netdev = fcoe_if_to_netdev(buffer);
if (!netdev) {
LIBFCOE_TRANSPORT_DBG("invalid device %s.\n", buffer);
@@ -645,11 +640,7 @@ out_putdev:
dev_put(netdev);
out_nodev:
mutex_unlock(&ft_mutex);
-
- if (rc == -ERESTARTSYS)
- return restart_syscall();
- else
- return rc;
+ return rc;
}
/**
@@ -667,9 +658,6 @@ static int fcoe_transport_disable(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
- if (!mutex_trylock(&ft_mutex))
- return restart_syscall();
-
#ifdef CONFIG_LIBFCOE_MODULE
/*
* Make sure the module has been initialized, and is not about to be
@@ -680,6 +668,8 @@ static int fcoe_transport_disable(const char *buffer, struct kernel_param *kp)
goto out_nodev;
#endif
+ mutex_lock(&ft_mutex);
+
netdev = fcoe_if_to_netdev(buffer);
if (!netdev)
goto out_nodev;
@@ -716,9 +706,6 @@ static int fcoe_transport_enable(const char *buffer, struct kernel_param *kp)
struct net_device *netdev = NULL;
struct fcoe_transport *ft = NULL;
- if (!mutex_trylock(&ft_mutex))
- return restart_syscall();
-
#ifdef CONFIG_LIBFCOE_MODULE
/*
* Make sure the module has been initialized, and is not about to be
@@ -729,6 +716,8 @@ static int fcoe_transport_enable(const char *buffer, struct kernel_param *kp)
goto out_nodev;
#endif
+ mutex_lock(&ft_mutex);
+
netdev = fcoe_if_to_netdev(buffer);
if (!netdev)
goto out_nodev;
@@ -743,10 +732,7 @@ out_putdev:
dev_put(netdev);
out_nodev:
mutex_unlock(&ft_mutex);
- if (rc == -ERESTARTSYS)
- return restart_syscall();
- else
- return rc;
+ return rc;
}
/**
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 02/11] fcoe: Remove mutex_trylock/restart_syscall checks
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
2011-04-01 23:05 ` [PATCH 01/11] libfcoe: Remove mutex_trylock/restart_syscall checks Robert Love
@ 2011-04-01 23:05 ` Robert Love
2011-04-01 23:06 ` [PATCH 03/11] fcoe: remove unnecessary module state check Robert Love
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:05 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain
These checks were initially added to avoid a lockdep
false positive when dealing with the s_active, rtnl
and fcoe_config_mutex mutexes. Recently the create,
destroy, enable and disable sysfs entries were moved
from fcoe.ko to libfcoe.ko. With this change the mutex
usage was shuffled around and the lockdep false
positive stopped happening. We can now remove these
checks.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
---
drivers/scsi/fcoe/fcoe.c | 24 ++++--------------------
1 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index bde6ee5..9e7206e 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1795,11 +1795,7 @@ static int fcoe_disable(struct net_device *netdev)
}
#endif
- if (!rtnl_trylock()) {
- mutex_unlock(&fcoe_config_mutex);
- return -ERESTARTSYS;
- }
-
+ rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
rtnl_unlock();
@@ -1839,11 +1835,7 @@ static int fcoe_enable(struct net_device *netdev)
goto out_nodev;
}
#endif
- if (!rtnl_trylock()) {
- mutex_unlock(&fcoe_config_mutex);
- return -ERESTARTSYS;
- }
-
+ rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
rtnl_unlock();
@@ -1882,11 +1874,7 @@ static int fcoe_destroy(struct net_device *netdev)
goto out_nodev;
}
#endif
- if (!rtnl_trylock()) {
- mutex_unlock(&fcoe_config_mutex);
- return -ERESTARTSYS;
- }
-
+ rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
if (!fcoe) {
rtnl_unlock();
@@ -1948,11 +1936,7 @@ static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
struct fc_lport *lport;
mutex_lock(&fcoe_config_mutex);
-
- if (!rtnl_trylock()) {
- mutex_unlock(&fcoe_config_mutex);
- return -ERESTARTSYS;
- }
+ rtnl_lock();
#ifdef CONFIG_FCOE_MODULE
/*
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 03/11] fcoe: remove unnecessary module state check
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
2011-04-01 23:05 ` [PATCH 01/11] libfcoe: Remove mutex_trylock/restart_syscall checks Robert Love
2011-04-01 23:05 ` [PATCH 02/11] fcoe: " Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 04/11] scsi: use list_move() instead of list_del()/list_add() combination Robert Love
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain, Yi Zou
From: Yi Zou <yi.zou@intel.com>
The check of module state being MODULE_STATE_LIVE is no longer needed for the
individual fcoe transport driver, e.g., fcoe.ko, as sysfs entries now go to
libfcoe now, if it reaches fcoe.ko, it has to be already registered. The module
state check for libfcoe will guard the possible race condition of sysfs being
writable before module_init function is called and after module_exit.
Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe.c | 47 ----------------------------------------------
1 files changed, 0 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9e7206e..34408d9 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1783,17 +1783,6 @@ static int fcoe_disable(struct net_device *netdev)
int rc = 0;
mutex_lock(&fcoe_config_mutex);
-#ifdef CONFIG_FCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module paramter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE) {
- rc = -ENODEV;
- goto out_nodev;
- }
-#endif
rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
@@ -1805,7 +1794,6 @@ static int fcoe_disable(struct net_device *netdev)
} else
rc = -ENODEV;
-out_nodev:
mutex_unlock(&fcoe_config_mutex);
return rc;
}
@@ -1824,17 +1812,6 @@ static int fcoe_enable(struct net_device *netdev)
int rc = 0;
mutex_lock(&fcoe_config_mutex);
-#ifdef CONFIG_FCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module paramter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE) {
- rc = -ENODEV;
- goto out_nodev;
- }
-#endif
rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
rtnl_unlock();
@@ -1844,7 +1821,6 @@ static int fcoe_enable(struct net_device *netdev)
else if (!fcoe_link_ok(fcoe->ctlr.lp))
fcoe_ctlr_link_up(&fcoe->ctlr);
-out_nodev:
mutex_unlock(&fcoe_config_mutex);
return rc;
}
@@ -1863,17 +1839,6 @@ static int fcoe_destroy(struct net_device *netdev)
int rc = 0;
mutex_lock(&fcoe_config_mutex);
-#ifdef CONFIG_FCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module paramter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE) {
- rc = -ENODEV;
- goto out_nodev;
- }
-#endif
rtnl_lock();
fcoe = fcoe_hostlist_lookup_port(netdev);
if (!fcoe) {
@@ -1938,18 +1903,6 @@ static int fcoe_create(struct net_device *netdev, enum fip_state fip_mode)
mutex_lock(&fcoe_config_mutex);
rtnl_lock();
-#ifdef CONFIG_FCOE_MODULE
- /*
- * Make sure the module has been initialized, and is not about to be
- * removed. Module paramter sysfs files are writable before the
- * module_init function is called and after module_exit.
- */
- if (THIS_MODULE->state != MODULE_STATE_LIVE) {
- rc = -ENODEV;
- goto out_nodev;
- }
-#endif
-
/* look for existing lport */
if (fcoe_hostlist_lookup(netdev)) {
rc = -EEXIST;
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 04/11] scsi: use list_move() instead of list_del()/list_add() combination
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (2 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 03/11] fcoe: remove unnecessary module state check Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 05/11] libfc: Move host_lock usage into ramp_up/down routines Robert Love
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Kirill A. Shutemov
From: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/esp_scsi.c | 6 ++----
drivers/scsi/fcoe/fcoe_ctlr.c | 6 ++----
drivers/scsi/scsi_tgt_lib.c | 6 ++----
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
index 5755852..9a1af1d 100644
--- a/drivers/scsi/esp_scsi.c
+++ b/drivers/scsi/esp_scsi.c
@@ -708,8 +708,7 @@ static void esp_maybe_execute_command(struct esp *esp)
tp = &esp->target[tgt];
lp = dev->hostdata;
- list_del(&ent->list);
- list_add(&ent->list, &esp->active_cmds);
+ list_move(&ent->list, &esp->active_cmds);
esp->active_cmd = ent;
@@ -1244,8 +1243,7 @@ static int esp_finish_select(struct esp *esp)
/* Now that the state is unwound properly, put back onto
* the issue queue. This command is no longer active.
*/
- list_del(&ent->list);
- list_add(&ent->list, &esp->queued_cmds);
+ list_move(&ent->list, &esp->queued_cmds);
esp->active_cmd = NULL;
/* Return value ignored by caller, it directly invokes
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index c93f007..fb3a506 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -978,10 +978,8 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
* the FCF that answers multicast solicitations, not the others that
* are sending periodic multicast advertisements.
*/
- if (mtu_valid) {
- list_del(&fcf->list);
- list_add(&fcf->list, &fip->fcfs);
- }
+ if (mtu_valid)
+ list_move(&fcf->list, &fip->fcfs);
/*
* If this is the first validated FCF, note the time and
diff --git a/drivers/scsi/scsi_tgt_lib.c b/drivers/scsi/scsi_tgt_lib.c
index f672820..edb8bff 100644
--- a/drivers/scsi/scsi_tgt_lib.c
+++ b/drivers/scsi/scsi_tgt_lib.c
@@ -275,10 +275,8 @@ void scsi_tgt_free_queue(struct Scsi_Host *shost)
for (i = 0; i < ARRAY_SIZE(qdata->cmd_hash); i++) {
list_for_each_entry_safe(tcmd, n, &qdata->cmd_hash[i],
- hash_list) {
- list_del(&tcmd->hash_list);
- list_add(&tcmd->hash_list, &cmds);
- }
+ hash_list)
+ list_move(&tcmd->hash_list, &cmds);
}
spin_unlock_irqrestore(&qdata->cmd_hash_lock, flags);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 05/11] libfc: Move host_lock usage into ramp_up/down routines
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (3 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 04/11] scsi: use list_move() instead of list_del()/list_add() combination Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 06/11] libfcoe: clean up netdev mapping properly when the transport goes away Robert Love
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain
The host_lock is still used to protect the can_queue
value in the Scsi_Host, but it doesn't need to be held
and released by each caller. This patch moves the lock
usage into the fc_fcp_can_queue_ramp_up and
fc_fcp_can_queue_ramp_down routines.
Signed-off-by: Robert Love <robert.w.love@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
---
drivers/scsi/libfc/fc_fcp.c | 25 +++++++++++++++----------
1 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index b1b03af..92ba9f0 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -335,22 +335,23 @@ static void fc_fcp_ddp_done(struct fc_fcp_pkt *fsp)
/**
* fc_fcp_can_queue_ramp_up() - increases can_queue
* @lport: lport to ramp up can_queue
- *
- * Locking notes: Called with Scsi_Host lock held
*/
static void fc_fcp_can_queue_ramp_up(struct fc_lport *lport)
{
struct fc_fcp_internal *si = fc_get_scsi_internal(lport);
+ unsigned long flags;
int can_queue;
+ spin_lock_irqsave(lport->host->host_lock, flags);
+
if (si->last_can_queue_ramp_up_time &&
(time_before(jiffies, si->last_can_queue_ramp_up_time +
FC_CAN_QUEUE_PERIOD)))
- return;
+ goto unlock;
if (time_before(jiffies, si->last_can_queue_ramp_down_time +
FC_CAN_QUEUE_PERIOD))
- return;
+ goto unlock;
si->last_can_queue_ramp_up_time = jiffies;
@@ -362,6 +363,9 @@ static void fc_fcp_can_queue_ramp_up(struct fc_lport *lport)
lport->host->can_queue = can_queue;
shost_printk(KERN_ERR, lport->host, "libfc: increased "
"can_queue to %d.\n", can_queue);
+
+unlock:
+ spin_unlock_irqrestore(lport->host->host_lock, flags);
}
/**
@@ -373,18 +377,19 @@ static void fc_fcp_can_queue_ramp_up(struct fc_lport *lport)
* commands complete or timeout, then try again with a reduced
* can_queue. Eventually we will hit the point where we run
* on all reserved structs.
- *
- * Locking notes: Called with Scsi_Host lock held
*/
static void fc_fcp_can_queue_ramp_down(struct fc_lport *lport)
{
struct fc_fcp_internal *si = fc_get_scsi_internal(lport);
+ unsigned long flags;
int can_queue;
+ spin_lock_irqsave(lport->host->host_lock, flags);
+
if (si->last_can_queue_ramp_down_time &&
(time_before(jiffies, si->last_can_queue_ramp_down_time +
FC_CAN_QUEUE_PERIOD)))
- return;
+ goto unlock;
si->last_can_queue_ramp_down_time = jiffies;
@@ -395,6 +400,9 @@ static void fc_fcp_can_queue_ramp_down(struct fc_lport *lport)
lport->host->can_queue = can_queue;
shost_printk(KERN_ERR, lport->host, "libfc: Could not allocate frame.\n"
"Reducing can_queue to %d.\n", can_queue);
+
+unlock:
+ spin_unlock_irqrestore(lport->host->host_lock, flags);
}
/*
@@ -409,16 +417,13 @@ static inline struct fc_frame *fc_fcp_frame_alloc(struct fc_lport *lport,
size_t len)
{
struct fc_frame *fp;
- unsigned long flags;
fp = fc_frame_alloc(lport, len);
if (likely(fp))
return fp;
/* error case */
- spin_lock_irqsave(lport->host->host_lock, flags);
fc_fcp_can_queue_ramp_down(lport);
- spin_unlock_irqrestore(lport->host->host_lock, flags);
return NULL;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 06/11] libfcoe: clean up netdev mapping properly when the transport goes away
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (4 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 05/11] libfc: Move host_lock usage into ramp_up/down routines Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 07/11] libfcoe: fix possible buffer overflow in fcoe_transport_show Robert Love
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain, Yi Zou
From: Yi Zou <yi.zou@intel.com>
When rmmoving the underlying fcoe transport driver module by force when
it's attached and in use, the correspoding netdev mapping should be
cleaned up properly as well, otherwise the lookup for a given netdev
for the transport would still return non NULL pointer, causing "unable
to handle paging request" bug.
Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe_transport.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index 7b61d00..ec0f395 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -343,6 +343,7 @@ EXPORT_SYMBOL(fcoe_transport_attach);
int fcoe_transport_detach(struct fcoe_transport *ft)
{
int rc = 0;
+ struct fcoe_netdev_mapping *nm = NULL, *tmp;
mutex_lock(&ft_mutex);
if (!ft->attached) {
@@ -352,6 +353,19 @@ int fcoe_transport_detach(struct fcoe_transport *ft)
goto out_attach;
}
+ /* remove netdev mapping for this transport as it is going away */
+ mutex_lock(&fn_mutex);
+ list_for_each_entry_safe(nm, tmp, &fcoe_netdevs, list) {
+ if (nm->ft == ft) {
+ LIBFCOE_TRANSPORT_DBG("transport %s going away, "
+ "remove its netdev mapping for %s\n",
+ ft->name, nm->netdev->name);
+ list_del(&nm->list);
+ kfree(nm);
+ }
+ }
+ mutex_unlock(&fn_mutex);
+
list_del(&ft->list);
ft->attached = false;
LIBFCOE_TRANSPORT_DBG("detaching transport %s\n", ft->name);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 07/11] libfcoe: fix possible buffer overflow in fcoe_transport_show
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (5 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 06/11] libfcoe: clean up netdev mapping properly when the transport goes away Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 08/11] libfcoe: fix wrong comment in fcoe_transport_detach Robert Love
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain, Tomas Henzl, Yi Zou
From: Yi Zou <yi.zou@intel.com>
possible buffer overflow in fcoe_transport_show when reaching the end of
buffer and crossing PAGE_SIZE boundary.
Signed-off-by: Yi Zou <yi.zou@intel.com>
Signed-off-by: Tomas Henzl <thenzl@redhat.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe_transport.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index ec0f395..538f299 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -385,9 +385,9 @@ static int fcoe_transport_show(char *buffer, const struct kernel_param *kp)
i = j = sprintf(buffer, "Attached FCoE transports:");
mutex_lock(&ft_mutex);
list_for_each_entry(ft, &fcoe_transports, list) {
- i += snprintf(&buffer[i], IFNAMSIZ, "%s ", ft->name);
- if (i >= PAGE_SIZE)
+ if (i >= PAGE_SIZE - IFNAMSIZ)
break;
+ i += snprintf(&buffer[i], IFNAMSIZ, "%s ", ft->name);
}
mutex_unlock(&ft_mutex);
if (i == j)
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 08/11] libfcoe: fix wrong comment in fcoe_transport_detach
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (6 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 07/11] libfcoe: fix possible buffer overflow in fcoe_transport_show Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 09/11] libfc: remove duplicate ema_list init Robert Love
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain, Frank Zhang, Yi Zou
From: Yi Zou <yi.zou@intel.com>
fix typo of '_attach' -> '_detach' in the comment.
Reported-by: Frank Zhang <frank_1.zhang@intel.com>
Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe_transport.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c
index 538f299..f81f77c 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -335,7 +335,7 @@ out_attach:
EXPORT_SYMBOL(fcoe_transport_attach);
/**
- * fcoe_transport_attach - Detaches an FCoE transport
+ * fcoe_transport_detach - Detaches an FCoE transport
* @ft: The fcoe transport to be attached
*
* Returns : 0 for success
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 09/11] libfc: remove duplicate ema_list init
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (7 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 08/11] libfcoe: fix wrong comment in fcoe_transport_detach Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 10/11] libfc: rec tov value and REC_TOV_CONST units usages is incorrect Robert Love
2011-04-01 23:06 ` [PATCH 11/11] fcoe: have fcoe log off and lport destroy before ndo_fcoe_disable Robert Love
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Vasu Dev
From: Vasu Dev <vasu.dev@intel.com>
As ema_list is already initialized by libfc_host_alloc.
Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/libfc/fc_lport.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 8c08b21..735f1f8 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1590,7 +1590,6 @@ void fc_lport_enter_flogi(struct fc_lport *lport)
*/
int fc_lport_config(struct fc_lport *lport)
{
- INIT_LIST_HEAD(&lport->ema_list);
INIT_DELAYED_WORK(&lport->retry_work, fc_lport_timeout);
mutex_init(&lport->lp_mutex);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 10/11] libfc: rec tov value and REC_TOV_CONST units usages is incorrect
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (8 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 09/11] libfc: remove duplicate ema_list init Robert Love
@ 2011-04-01 23:06 ` Robert Love
2011-04-01 23:06 ` [PATCH 11/11] fcoe: have fcoe log off and lport destroy before ndo_fcoe_disable Robert Love
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Ross Brattain, Vasu Dev
From: Vasu Dev <vasu.dev@intel.com>
Added REC_TOV_CONST intent was to have rec tov as e_d_tov + 1s
but currently it is e_d_tov + 1ms since e_d_tov is stored in ms
unit.
Also returned rec tov by get_fsp_rec_tov is in ms and this ms tov
is used as-is with fc_fcp_timer_set expecting jiffies tov.
Fixed this by having get_fsp_rec_tov return rec tov in jiffies
as e_d_tov + 1s and then use jiffies tov w/ fc_fcp_timer_set.
Also some cleanup, no need to cache get_fsp_rec_tov return value
in local rec_tov at various places.
Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/libfc/fc_fcp.c | 32 +++++++++-----------------------
1 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 92ba9f0..a879c96 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -57,9 +57,6 @@ static struct kmem_cache *scsi_pkt_cachep;
#define FC_SRB_READ (1 << 1)
#define FC_SRB_WRITE (1 << 0)
-/* constant added to e_d_tov timeout to get rec_tov value */
-#define REC_TOV_CONST 1
-
/*
* The SCp.ptr should be tested and set under the scsi_pkt_queue lock
*/
@@ -248,7 +245,7 @@ static inline void fc_fcp_unlock_pkt(struct fc_fcp_pkt *fsp)
/**
* fc_fcp_timer_set() - Start a timer for a fcp_pkt
* @fsp: The FCP packet to start a timer for
- * @delay: The timeout period for the timer
+ * @delay: The timeout period in jiffies
*/
static void fc_fcp_timer_set(struct fc_fcp_pkt *fsp, unsigned long delay)
{
@@ -1098,16 +1095,14 @@ static int fc_fcp_pkt_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp)
/**
* get_fsp_rec_tov() - Helper function to get REC_TOV
* @fsp: the FCP packet
+ *
+ * Returns rec tov in jiffies as rpriv->e_d_tov + 1 second
*/
static inline unsigned int get_fsp_rec_tov(struct fc_fcp_pkt *fsp)
{
- struct fc_rport *rport;
- struct fc_rport_libfc_priv *rpriv;
+ struct fc_rport_libfc_priv *rpriv = fsp->rport->dd_data;
- rport = fsp->rport;
- rpriv = rport->dd_data;
-
- return rpriv->e_d_tov + REC_TOV_CONST;
+ return msecs_to_jiffies(rpriv->e_d_tov) + HZ;
}
/**
@@ -1127,7 +1122,6 @@ static int fc_fcp_cmd_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
struct fc_rport_libfc_priv *rpriv;
const size_t len = sizeof(fsp->cdb_cmd);
int rc = 0;
- unsigned int rec_tov;
if (fc_fcp_lock_pkt(fsp))
return 0;
@@ -1158,12 +1152,9 @@ static int fc_fcp_cmd_send(struct fc_lport *lport, struct fc_fcp_pkt *fsp,
fsp->seq_ptr = seq;
fc_fcp_pkt_hold(fsp); /* hold for fc_fcp_pkt_destroy */
- rec_tov = get_fsp_rec_tov(fsp);
-
setup_timer(&fsp->timer, fc_fcp_timeout, (unsigned long)fsp);
-
if (rpriv->flags & FC_RP_FLAGS_REC_SUPPORTED)
- fc_fcp_timer_set(fsp, rec_tov);
+ fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp));
unlock:
fc_fcp_unlock_pkt(fsp);
@@ -1240,16 +1231,14 @@ static void fc_lun_reset_send(unsigned long data)
{
struct fc_fcp_pkt *fsp = (struct fc_fcp_pkt *)data;
struct fc_lport *lport = fsp->lp;
- unsigned int rec_tov;
if (lport->tt.fcp_cmd_send(lport, fsp, fc_tm_done)) {
if (fsp->recov_retry++ >= FC_MAX_RECOV_RETRY)
return;
if (fc_fcp_lock_pkt(fsp))
return;
- rec_tov = get_fsp_rec_tov(fsp);
setup_timer(&fsp->timer, fc_lun_reset_send, (unsigned long)fsp);
- fc_fcp_timer_set(fsp, rec_tov);
+ fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp));
fc_fcp_unlock_pkt(fsp);
}
}
@@ -1541,12 +1530,11 @@ static void fc_fcp_rec_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
}
fc_fcp_srr(fsp, r_ctl, offset);
} else if (e_stat & ESB_ST_SEQ_INIT) {
- unsigned int rec_tov = get_fsp_rec_tov(fsp);
/*
* The remote port has the initiative, so just
* keep waiting for it to complete.
*/
- fc_fcp_timer_set(fsp, rec_tov);
+ fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp));
} else {
/*
@@ -1710,7 +1698,6 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
{
struct fc_fcp_pkt *fsp = arg;
struct fc_frame_header *fh;
- unsigned int rec_tov;
if (IS_ERR(fp)) {
fc_fcp_srr_error(fsp, fp);
@@ -1737,8 +1724,7 @@ static void fc_fcp_srr_resp(struct fc_seq *seq, struct fc_frame *fp, void *arg)
switch (fc_frame_payload_op(fp)) {
case ELS_LS_ACC:
fsp->recov_retry = 0;
- rec_tov = get_fsp_rec_tov(fsp);
- fc_fcp_timer_set(fsp, rec_tov);
+ fc_fcp_timer_set(fsp, get_fsp_rec_tov(fsp));
break;
case ELS_LS_RJT:
default:
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 11/11] fcoe: have fcoe log off and lport destroy before ndo_fcoe_disable
2011-04-01 23:05 [PATCH 00/11] libfc, libfcoe and fcoe updates for scsi-misc Robert Love
` (9 preceding siblings ...)
2011-04-01 23:06 ` [PATCH 10/11] libfc: rec tov value and REC_TOV_CONST units usages is incorrect Robert Love
@ 2011-04-01 23:06 ` Robert Love
10 siblings, 0 replies; 12+ messages in thread
From: Robert Love @ 2011-04-01 23:06 UTC (permalink / raw)
To: James.Bottomley, linux-scsi; +Cc: Vasu Dev
From: Vasu Dev <vasu.dev@intel.com>
Currently fcoe interface cleanup is done after ndo_fcoe_disable
and that prevents logoff going out to the peer, so this patch
moves all netdev cleanup and its releasing inside
fcoe_interface_cleanup to have log off before ndo_fcoe_disable
disables the fcoe.
This patch also fixes asymmetric rtnl locking around fcoe_if_destroy,
as currently this function requires rtnl held by its caller
and then have this func drops the lock, instead now don't have
any processing under rtnl inside fcoe_if_destroy, this required
moving few func to get build working again.
Signed-off-by: Vasu Dev <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
drivers/scsi/fcoe/fcoe.c | 131 +++++++++++++++++++++-------------------------
1 files changed, 60 insertions(+), 71 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 34408d9..5d3700d 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -381,6 +381,42 @@ out:
}
/**
+ * fcoe_interface_release() - fcoe_port kref release function
+ * @kref: Embedded reference count in an fcoe_interface struct
+ */
+static void fcoe_interface_release(struct kref *kref)
+{
+ struct fcoe_interface *fcoe;
+ struct net_device *netdev;
+
+ fcoe = container_of(kref, struct fcoe_interface, kref);
+ netdev = fcoe->netdev;
+ /* tear-down the FCoE controller */
+ fcoe_ctlr_destroy(&fcoe->ctlr);
+ kfree(fcoe);
+ dev_put(netdev);
+ module_put(THIS_MODULE);
+}
+
+/**
+ * fcoe_interface_get() - Get a reference to a FCoE interface
+ * @fcoe: The FCoE interface to be held
+ */
+static inline void fcoe_interface_get(struct fcoe_interface *fcoe)
+{
+ kref_get(&fcoe->kref);
+}
+
+/**
+ * fcoe_interface_put() - Put a reference to a FCoE interface
+ * @fcoe: The FCoE interface to be released
+ */
+static inline void fcoe_interface_put(struct fcoe_interface *fcoe)
+{
+ kref_put(&fcoe->kref, fcoe_interface_release);
+}
+
+/**
* fcoe_interface_cleanup() - Clean up a FCoE interface
* @fcoe: The FCoE interface to be cleaned up
*
@@ -392,6 +428,21 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
struct fcoe_ctlr *fip = &fcoe->ctlr;
u8 flogi_maddr[ETH_ALEN];
const struct net_device_ops *ops;
+ struct fcoe_port *port = lport_priv(fcoe->ctlr.lp);
+
+ FCOE_NETDEV_DBG(netdev, "Destroying interface\n");
+
+ /* Logout of the fabric */
+ fc_fabric_logoff(fcoe->ctlr.lp);
+
+ /* Cleanup the fc_lport */
+ fc_lport_destroy(fcoe->ctlr.lp);
+
+ /* Stop the transmit retry timer */
+ del_timer_sync(&port->timer);
+
+ /* Free existing transmit skbs */
+ fcoe_clean_pending_queue(fcoe->ctlr.lp);
/*
* Don't listen for Ethernet packets anymore.
@@ -414,6 +465,9 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
} else
dev_mc_del(netdev, FIP_ALL_ENODE_MACS);
+ if (!is_zero_ether_addr(port->data_src_addr))
+ dev_uc_del(netdev, port->data_src_addr);
+
/* Tell the LLD we are done w/ FCoE */
ops = netdev->netdev_ops;
if (ops->ndo_fcoe_disable) {
@@ -421,42 +475,7 @@ void fcoe_interface_cleanup(struct fcoe_interface *fcoe)
FCOE_NETDEV_DBG(netdev, "Failed to disable FCoE"
" specific feature for LLD.\n");
}
-}
-
-/**
- * fcoe_interface_release() - fcoe_port kref release function
- * @kref: Embedded reference count in an fcoe_interface struct
- */
-static void fcoe_interface_release(struct kref *kref)
-{
- struct fcoe_interface *fcoe;
- struct net_device *netdev;
-
- fcoe = container_of(kref, struct fcoe_interface, kref);
- netdev = fcoe->netdev;
- /* tear-down the FCoE controller */
- fcoe_ctlr_destroy(&fcoe->ctlr);
- kfree(fcoe);
- dev_put(netdev);
- module_put(THIS_MODULE);
-}
-
-/**
- * fcoe_interface_get() - Get a reference to a FCoE interface
- * @fcoe: The FCoE interface to be held
- */
-static inline void fcoe_interface_get(struct fcoe_interface *fcoe)
-{
- kref_get(&fcoe->kref);
-}
-
-/**
- * fcoe_interface_put() - Put a reference to a FCoE interface
- * @fcoe: The FCoE interface to be released
- */
-static inline void fcoe_interface_put(struct fcoe_interface *fcoe)
-{
- kref_put(&fcoe->kref, fcoe_interface_release);
+ fcoe_interface_put(fcoe);
}
/**
@@ -821,39 +840,9 @@ skip_oem:
* fcoe_if_destroy() - Tear down a SW FCoE instance
* @lport: The local port to be destroyed
*
- * Locking: must be called with the RTNL mutex held and RTNL mutex
- * needed to be dropped by this function since not dropping RTNL
- * would cause circular locking warning on synchronous fip worker
- * cancelling thru fcoe_interface_put invoked by this function.
- *
*/
static void fcoe_if_destroy(struct fc_lport *lport)
{
- struct fcoe_port *port = lport_priv(lport);
- struct fcoe_interface *fcoe = port->priv;
- struct net_device *netdev = fcoe->netdev;
-
- FCOE_NETDEV_DBG(netdev, "Destroying interface\n");
-
- /* Logout of the fabric */
- fc_fabric_logoff(lport);
-
- /* Cleanup the fc_lport */
- fc_lport_destroy(lport);
-
- /* Stop the transmit retry timer */
- del_timer_sync(&port->timer);
-
- /* Free existing transmit skbs */
- fcoe_clean_pending_queue(lport);
-
- if (!is_zero_ether_addr(port->data_src_addr))
- dev_uc_del(netdev, port->data_src_addr);
- rtnl_unlock();
-
- /* receives may not be stopped until after this */
- fcoe_interface_put(fcoe);
-
/* Free queued packets for the per-CPU receive threads */
fcoe_percpu_clean(lport);
@@ -1836,6 +1825,7 @@ static int fcoe_enable(struct net_device *netdev)
static int fcoe_destroy(struct net_device *netdev)
{
struct fcoe_interface *fcoe;
+ struct fc_lport *lport;
int rc = 0;
mutex_lock(&fcoe_config_mutex);
@@ -1846,10 +1836,11 @@ static int fcoe_destroy(struct net_device *netdev)
rc = -ENODEV;
goto out_nodev;
}
- fcoe_interface_cleanup(fcoe);
+ lport = fcoe->ctlr.lp;
list_del(&fcoe->list);
- /* RTNL mutex is dropped by fcoe_if_destroy */
- fcoe_if_destroy(fcoe->ctlr.lp);
+ fcoe_interface_cleanup(fcoe);
+ rtnl_unlock();
+ fcoe_if_destroy(lport);
out_nodev:
mutex_unlock(&fcoe_config_mutex);
return rc;
@@ -1865,8 +1856,6 @@ static void fcoe_destroy_work(struct work_struct *work)
port = container_of(work, struct fcoe_port, destroy_work);
mutex_lock(&fcoe_config_mutex);
- rtnl_lock();
- /* RTNL mutex is dropped by fcoe_if_destroy */
fcoe_if_destroy(port->lport);
mutex_unlock(&fcoe_config_mutex);
}
^ permalink raw reply related [flat|nested] 12+ messages in thread