Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Sebastian Herbszt]

Hannes Reinecke wrote:
> On 06/13/2017 01:08 AM, James Smart wrote:

[snip]

> > Questions:
> > a) How best to deal with overlapping pci id's ?  E.g. if we do (1)
> > and we have an initiator and target driver, there is a lot of
> > adapters that are fully functional for target operation, but were
> > sold as primarily an initiator adapter. How could we manage target
> > mode enablement without code mod or hard pci id partitioning ?   I
> > know individual pci unbind/bind could work, but its been frowned
> > upon as a long term option. Same thing goes for module parameters
> > to select which ports do what role. 
> That indeed is a problem.
> 
> Ideally we should be able to set the required mode on a per-port base;
> having it per PCI device might be too coarse. Unless you represent
> each port as a PCI function; not sure if that's the case, though.

It seems to be the case.
At least a dual port FC HBA (SLI-3) has two PCI functions.

> If we were to allow to set the mode on a per-port base we could easily
> implement kernel parameters like fctarget=WWPN and/or
> fcinitiator=WWPN; NVMe could be treated similarly.
> And have a config option specifying if the default FC mode should be
> initiator or target.

The old lpfc+lpfc_scst combination and also qla2xxx+tcm_qla2xxx allow
simultaneous initiator and target mode on the same port. I guess this
won't be possible with a split driver.

Sebastian

[PATCH] efct: fix compilation warning about atomic_t usage

[Sebastian Herbszt]

Use kref_read() instead of accessing the counter inside a kref.

Signed-off-by: Sebastian Herbszt <herb...@gmx.de>
---
 efct/efct_ddump.c | 5 ++---
 efct/efct_hw.c| 6 +++---
 efct/efct_io.c| 3 +--
 efct/efct_lio.c   | 2 +-
 efct/efct_scsi.c  | 4 ++--
 5 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/efct/efct_ddump.c b/efct/efct_ddump.c
index e91a838..faea9a0 100644
--- a/efct/efct_ddump.c
+++ b/efct/efct_ddump.c
@@ -340,8 +340,7 @@ efct_ddump_hw_io(struct efct_textbuf_s *textbuf, struct 
efct_hw_io_s *io)
efct_ddump_value(textbuf, "tag", "0x%x", io->reqtag);
efct_ddump_value(textbuf, "abort_reqtag",
"0x%x", io->abort_reqtag);
-   efct_ddump_value(textbuf, "ref_count", "%d",
-   atomic_read(>ref.refcount));
+   efct_ddump_value(textbuf, "ref_count", "%d", kref_read(>ref));
 
/* just to make it obvious, display abort bit from tag */
efct_ddump_value(textbuf, "abort", "0x%x", io->abort_in_progress);
@@ -606,7 +605,7 @@ efct_ddump_hw(struct efct_textbuf_s *textbuf, struct 
efct_hw_s *hw,
io = >io[i];
 
if (efct_hw_is_xri_port_owned(hw, io->indicator)) {
-   if (atomic_read(>ref.refcount)) {
+   if (kref_read(>ref)) {
/* only display free ios if they're active */
efct_ddump_hw_io(textbuf, io);
}
diff --git a/efct/efct_hw.c b/efct/efct_hw.c
index e52a505..cf37dc9 100644
--- a/efct/efct_hw.c
+++ b/efct/efct_hw.c
@@ -3647,7 +3647,7 @@ efct_hw_io_alloc(struct efct_hw_s *hw)
 struct efct_hw_io_s *
 efct_hw_io_activate_port_owned(struct efct_hw_s *hw, struct efct_hw_io_s *io)
 {
-   if (atomic_read(>ref.refcount) > 0) {
+   if (kref_read(>ref) > 0) {
efct_log_err(hw->os, "Bad parameter: refcount > 0\n");
return NULL;
}
@@ -3794,7 +3794,7 @@ int32_t
 efct_hw_io_free(struct efct_hw_s *hw, struct efct_hw_io_s *io)
 {
/* just put refcount */
-   if (atomic_read(>ref.refcount) <= 0) {
+   if (kref_read(>ref) <= 0) {
efct_log_err(hw->os,
"Bad parameter: refcount <= 0 xri=%x tag=%x\n",
io->indicator, io->reqtag);
@@ -3821,7 +3821,7 @@ efct_hw_io_free(struct efct_hw_s *hw, struct efct_hw_io_s 
*io)
 uint8_t
 efct_hw_io_inuse(struct efct_hw_s *hw, struct efct_hw_io_s *io)
 {
-   return (atomic_read(>ref.refcount) > 0);
+   return (kref_read(>ref) > 0);
 }
 
 /**
diff --git a/efct/efct_io.c b/efct/efct_io.c
index 988ec28..9ee9932 100644
--- a/efct/efct_io.c
+++ b/efct/efct_io.c
@@ -332,8 +332,7 @@ efct_ddump_io(struct efct_textbuf_s *textbuf, struct 
efct_io_s *io)
efct_ddump_value(textbuf, "display_name", "%s", io->display_name);
efct_ddump_value(textbuf, "node_name", "%s", io->node->display_name);
 
-   efct_ddump_value(textbuf, "ref_count", "%d",
-   atomic_read(>ref.refcount));
+   efct_ddump_value(textbuf, "ref_count", "%d", kref_read(>ref));
efct_ddump_value(textbuf, "io_type", "%d", io->io_type);
efct_ddump_value(textbuf, "hio_type", "%d", io->hio_type);
efct_ddump_value(textbuf, "cmd_tgt", "%d", io->cmd_tgt);
diff --git a/efct/efct_lio.c b/efct/efct_lio.c
index f5c7a70..74b48ad 100644
--- a/efct/efct_lio.c
+++ b/efct/efct_lio.c
@@ -2276,7 +2276,7 @@ efct_scsi_tgt_ddump(struct efct_textbuf_s *textbuf,
efct_ddump_value(textbuf, "se_cmd.state_active", "%d",
io->tgt_io.cmd.state_active);
efct_ddump_value(textbuf, "se_cmd.cmd_kref.refcount", "%d",
-   atomic_read(>tgt_io.cmd.cmd_kref.refcount));
+   kref_read(>tgt_io.cmd.cmd_kref));
efct_ddump_value(textbuf, "se_cmd.se_cmd_flags", "%#x",
io->tgt_io.cmd.se_cmd_flags);
efct_ddump_value(textbuf, "se_cmd.t_state", "%d",
diff --git a/efct/efct_scsi.c b/efct/efct_scsi.c
index c2f5a35..ad49aa6 100644
--- a/efct/efct_scsi.c
+++ b/efct/efct_scsi.c
@@ -301,7 +301,7 @@ void
 efct_scsi_io_free(struct efct_io_s *io)
 {
scsi_io_trace(io, "freeing io 0x%p %s\n", io, io->display_name);
-   efct_assert(atomic_read(>ref.refcount) > 0);
+   efct_assert(kref_read(>ref) > 0);
kref_put(>ref, io->release);
 }
 
@@ -2320,7 +2320,7 @@ efct_scsi_io_complete(struct efct_io_s *io)
}
 
scsi_io_trace(io, "freeing io 0x%p %s\n", io, io->display_name);
-   efct_assert(atomic_read(>ref.refcount) > 0);
+   efct_assert(kref_read(>ref) > 0);
kref_put(>ref, io->release);
 }
 
-- 
2.7.3

[PATCH] efct: add missing include file

[Sebastian Herbszt]

After the split of linux/sched.h, linux/sched/signal.h is required.

Signed-off-by: Sebastian Herbszt <herb...@gmx.de>
---
 efct/efct_os.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/efct/efct_os.h b/efct/efct_os.h
index 16f7dc3..5ae269e 100644
--- a/efct/efct_os.h
+++ b/efct/efct_os.h
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.7.3

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Sebastian Herbszt]

Nicholas A. Bellinger wrote:
> Hi James & Co,
> 
> Adding target-devel and Sebastian CC'
> 
> On Mon, 2017-02-27 at 15:28 -0800, James Smart wrote:
> > I'd like to announce the availability of the Broadcom (Emulex) FC
> > Target driver - efct.
> > This driver has been part of the Emulex OneCore Storage SDK tool
> > kit for Emulex
> > SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher
> > adapters. Although this
> > kit has supported FCoE in the past, it is currently limited to FC
> > support.
> > 
> > This driver provides the following:
> > - Target mode operation:
> >- Functional with LIO-based interfaces  
> 
> Glad to see the upstream push for this after all these years.  :)

How does this compare to ocs_fc_lio? Does efct replace it?
 
> >- Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp,
> > cmd cpu spreading
> >- High login mode - thousands of logins
> >- T-10 DIF/PI support  (inline and separate)
> >- NPIV support
> > - Concurrent Initiator support if needed
> > - Discovery engine has F_Port and fabric services emulation.
> > - Extended mgmt interfaces:
> >- firmware dump api, including dump to host memory for faster
> > dumps
> >- Healthcheck operations and watchdogs
> > - Extended driver behaviors such as:
> >- polled mode operation
> >- multi-queue: cpu, roundrobin, or priority  (but not tied  to
> > scsi-mq)
> >- long chained sgl's
> >- extensive internal logging and statistics
> >- Tuning parameters on modes and resource allocation to
> > different features
> > 
> > Broadcom is looking to upstream this driver and would like review
> > and feedback.
> > The driver may be found at the following git repository:
> >  g...@gitlab.com:jsmart/efct-Emulex_FC_Target.git
> >   
> 
> Can we get the patch series posted to linux-scsi and target-devel to
> start giving some initial feedback..?
> 
> > 
> > Some of the key questions we have are with lpfc :
> > 1) Coexistence vs integration
> > Currently, the efct driver maps to a different set of PCI ids than
> > lpfc. It's very clear there's an overlap with lpfc, both on SLI-4
> > hw as well as initiator support.
> > Although target mode support can be simplistically added to lpfc,
> > what we've found is
> > that doing so means a lot of tradeoffs. Some of the target mode 
> > features, when enabled,
> > impact the initiator support and how it would operate.
> >   
> 
> I don't really have much preference either way.  

Just like Hannes I do favour integration. I guess it could be
comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
lpfc + tcm_lpfc. That approach might even help Bart with his
target driver unification if he didn't give up on that topic.

> > 2) SLI-3 support
> > lpfc provides SLI-3 support so that all FC adapters are supported, 
> > including the older ones.
> > The form of the driver, based on its history, is SLI-3 with SLI-3 
> > adapted to SLI-4 at the point
> > it hits the hardware. efct does not support SLI-3.  
> 
> AFAIK I think Sebastian was using SLI-3, so he might have some
> comments here.

I do care for SLI-3. Currently I still only got access to 8GFC HBAs and
even our new cards are SLI-3 and not Advanced-8 with SLI-4. Seems like
SLI-3 is not dead yet.

> Since he's been the main person using the original tcm_lpfc code from
> way back when, maybe it would be a good idea to send him a couple of
> SLI-4 HBAs to help with the upstreaming of efct..?

The code I got here is actually a direct port from lpfc + lpfc_scst to
TCM. As of now it kind of works on kernel version 4.10.

> > 
> > 3) complexity of configuration knobs caused by the kitchen-sink of 
> > features in lpfc ?
> > we are pushing the limit on needing per-instance attributes rather
> > than global module
> > parameters.  
> 
> This is exactly what
> /sys/kernel/config/target/efct/$WWPN/tpgt_1/attribute/ is intended
> for.
> 

Sebastian

Re: [PATCH v2 03/11] lpfc: Deprecate lpfc_soft_wwn parameter

[Sebastian Herbszt]

James,

thank you for taking the time to answer me.

James Smart wrote:
> Sebastian,
> 
> "not portable" isn't the right way to describe it. It's not a 
> chip-architecture issue, but rather that some oem platforms have 
> side-band management that overrides anything that could have been
> done in the os, and in ways that may not be easy to communicate back
> to the driver.

"not portable" might not be the term I actually meant. Let's call it
server vendor dependent.
Can you please elaborate on the oem platforms and side-band management
you mean? I know of a solution called "ServerView Virtual-IO Manager"
by Fujitsu which might fit. AFAIK it is a piece of software which
communicates with the BMC and allows to change the MAC address and WWN.
Of course it only works with Fujitsu servers and supported cards.
I think I heard that for NICs it just changes the MAC address in the
eeprom. This is not done "online" but when the server is powered on.
Even though this might be a comparable case (external MAC address
configuration) I think the ability to change the MAC address by the
means of "ip link set dev ethX address" was not removed.

> FAWN - true Brocade only - but that is the 70+%
> market share of the FC switches. The current implementation already
> doesn't work many situations - so there's no clear winner for having
> it work everywhere. Given the inability for it always to work, the
> decision was to deprecate the attribute.
> 
> -- james

How do you choose the desired WWN between the possible options
(factory set, oem platform set, FA-WWN)?
And wouldn't soft wwn always win because it is applied at last?
However it might compete with FA-WWN. I wasn't yet able to
find details on how it works.

Sebastian
--
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 v2 03/11] lpfc: Deprecate lpfc_soft_wwn parameter

[Sebastian Herbszt]

James Smart wrote:
> I competes, without reasonable solutions to resolve which should be
> what at what time, with things like fabric-assigned wwn's as well as 
> platform-oem name assignments via platform mechanisms.  Too many
> admin planes competing.
> 
> -- james

FA-WWN seems to be fabric (and Brocade?) only and "platform mechanisms"
are likely not portable. Soft wwn is a simple and flexible solution
which works in fabric and loop configurations and is platform and
server vendor agnostic. It already works with existing HBAs on x86 and
should also work on PowerPC or SPARC systems.
It is useful for initiators (hardware independence/mobility) and
targets (transparent failover/floating wwn).
Therefore I like to ask you to revert this patch and not introduce this
functional regression.

Sebastian
--
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 v2 03/11] lpfc: Deprecate lpfc_soft_wwn parameter

[Sebastian Herbszt]

James Smart wrote:
> Deprecate lpfc_soft_wwn parameter.
> No longer allow override of hw-assigned wwns
> 
> Signed-off-by: Dick Kennedy 
> Signed-off-by: James Smart 

Any reason to remove this functionality?
I was actually using it.

Sebastian
--
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] lpfc: remove incorrect lockdep assertion

[Sebastian Herbszt]

Remove incorrect lockdep assertion from lpfc_sli_hbqbuf_find() which acquires
the hbalock itself. Fix the comment which resulted in this mistake.

Fixes: 1c2ba475eb0e ("lpfc: Add lockdep assertions")
Signed-off-by: Sebastian Herbszt <herb...@gmx.de>

diff -uNr -U 5 a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
--- a/drivers/scsi/lpfc/lpfc_sli.c  2016-03-15 03:25:44.324554218 +0100
+++ b/drivers/scsi/lpfc/lpfc_sli.c  2016-04-17 13:01:59.700182669 +0200
@@ -1998,24 +1998,21 @@
 /**
  * lpfc_sli_hbqbuf_find - Find the hbq buffer associated with a tag
  * @phba: Pointer to HBA context object.
  * @tag: Tag of the hbq buffer.
  *
- * This function is called with hbalock held. This function searches
- * for the hbq buffer associated with the given tag in the hbq buffer
- * list. If it finds the hbq buffer, it returns the hbq_buffer other wise
- * it returns NULL.
+ * This function searches for the hbq buffer associated with the given tag in
+ * the hbq buffer list. If it finds the hbq buffer, it returns the hbq_buffer
+ * otherwise it returns NULL.
  **/
 static struct hbq_dmabuf *
 lpfc_sli_hbqbuf_find(struct lpfc_hba *phba, uint32_t tag)
 {
struct lpfc_dmabuf *d_buf;
struct hbq_dmabuf *hbq_buf;
uint32_t hbqno;
 
-   lockdep_assert_held(>hbalock);
-
hbqno = tag >> 16;
if (hbqno >= LPFC_MAX_HBQS)
return NULL;
 
spin_lock_irq(>hbalock);
--
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: [RESEND PATCH v2 2/2] scsi: Add intermediate STARGET_REMOVE state to scsi_target_state

[Sebastian Herbszt]

Johannes Thumshirn wrote:
> Add intermediate STARGET_REMOVE state to scsi_target_state to avoid running
> into the BUG_ON() in scsi_target_reap().
> 
> This intermediate state is only valid in the path from scsi_remove_target() to
> scsi_target_destroy() indicating this target is going to be removed.
> 
> Signed-off-by: Johannes Thumshirn 
> Fixes: 40998193560dab6c3ce8d25f4fa58a23e252ef38
> Cc: sta...@vger.kernel.org
> Reviewed-by: Hannes Reinecke 
> Reviewed-by: Ewan D. Milne 
> ---
> 
> Changes from v1:
> * The state transition from STARGET_CREATED to STARGET_DEL is legitimate,
>   so don't BUG() on it. Found by the 0-Day Bot.

This is yet another attempt to fix 40998193560d. Can you please explain how
it is "superior" to the one proposed by Bart before [1] ?

[1] http://marc.info/?l=linux-scsi=145227191917602=2

Sebastian
--
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/3] lpfc: fix misleading indentation

[Sebastian Herbszt]

Arnd Bergmann wrote:
> gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
> call in lpfc_online(), which clearly doesn't look right:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
> drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it 
> were guarded by... [-Wmisleading-indentation]
>lpfc_destroy_vport_work_array(phba, vports);
>^
> drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
>   if (vports != NULL)
>   ^~
> 
> Looking at the patch that introduced this code, it's clear that the
> behavior is correct and the indentation is wrong.
> 
> This fixes the indentation and adds curly braces around the previous
> if() block for clarity, as that is most likely what caused the code
> to be misindented in the first place.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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] Separate target visibility from reaped state information

[Sebastian Herbszt]

Martin K. Petersen wrote:
> >>>>> "Sebastian" == Sebastian Herbszt <herb...@gmx.de> writes:
> 
> >> The only change from the current workflow is that the DEL transition
> >> (now the reaped flag) is done before the spin lock is dropped which
> >> would fix a tiny window for two threads both trying to remove the
> >> same target, but there's nothing that could possibly fix an iterative
> >> soft lockup caused by restarting the loop, which is what the
> >> changelog says.
> 
> Sebastian> James, Martin, what's the status of this patch?  I still hit
> Sebastian> the reported soft lockup on 4.5-rc1.
> 
> And you have verified that Bart's patch applied on top of 4.5-rc1 still
> fixes the lockup? (I know you tested a previous version)

I now have verified that Bart's patch does fix my soft lockup on 4.5-rc2.

Sebastian
--
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] Separate target visibility from reaped state information

[Sebastian Herbszt]

James Bottomley wrote:
> On Wed, 2016-02-03 at 23:38 +0100, Sebastian Herbszt wrote:
> > James Bottomley wrote:
> > > On Mon, 2016-02-01 at 19:43 -0800, Bart Van Assche wrote:
> > > > On 01/19/16 17:03, James Bottomley wrote:
> > > > > On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
> > > > > > > > > > > "Bart" == Bart Van Assche <
> > > > > > > > > > > bart.vanass...@sandisk.com>
> > > > > > > > > > > writes:
> > > > > > 
> > > > > > Bart> Instead of representing the states "visible in sysfs"
> > > > > > and
> > > > > > "has
> > > > > > Bart> been removed from the target list" by a single state
> > > > > > variable,
> > > > > > use
> > > > > > Bart> two variables to represent this information.
> > > > > > 
> > > > > > James: Are you happy with the latest iteration of this?
> > > > > > Should I
> > > > > > queue
> > > > > > it?
> > > > > 
> > > > > Well, I'm OK with the patch: it's a simple transformation of
> > > > > the
> > > > > enumerated state to a two bit state.  What I can't see is how
> > > > > it
> > > > > fixes
> > > > > any soft lockup.
> > > > > 
> > > > > The only change from the current workflow is that the DEL
> > > > > transition
> > > > > (now the reaped flag) is done before the spin lock is dropped
> > > > > which
> > > > > would fix a tiny window for two threads both trying to remove
> > > > > the
> > > > > same
> > > > > target, but there's nothing that could possibly fix an
> > > > > iterative
> > > > > soft
> > > > > lockup caused by restarting the loop, which is what the
> > > > > changelog
> > > > > says.
> > > > 
> > > > Hello James,
> > > > 
> > > > scsi_remove_target() doesn't lock the scan_mutex which means that
> > > > concurrent SCSI scanning activity is not prohibited. Such
> > > > scanning 
> > > > activity can postpone the transition of the state of a SCSI
> > > > target 
> > > > into STARGET_DEL. I think if the scheduler decides to run the
> > > > thread 
> > > > that executes scsi_remove_target() on the same CPU as the
> > > > scanning 
> > > > code after the scanning code has obtained a reap ref and before
> > > > the 
> > > > scanning code has released the reap ref again that the soft
> > > > lockup 
> > > > can be triggered that has been reported by Sebastian Herbszt.
> > > 
> > > OK, I finally understand the scenario;  I'm not sure I understand
> > > how
> > > we're getting concurrent scanning and removal from a simple rmmod
> > > ... I
> > > take it this is insmod rmmod in a tight loop?
> > 
> > I am able to trigger the soft lockup with this test case run once:
> > 
> > modprobe lpfc
> > run fio for 10 seconds
> > rmmod lpfc
> > 
> > My test setup involves running qla2xxx in target mode (SCST) and
> > lpfc as initiator on the same system with one exported volume.
> 
> Hm, that doesn't seem to involve scanning colliding with removal. 
>  Probably something else is pinning the target for some reason ...
> unless there's some instability or constant change on the FC fabric
> itself?  FC is hotplug, so if devices continually appear and disappear,
> they'd effectively cause a scan of the added device to take place even
> during the rmmod.
> 
> Anyway, does the last_scan patch fix the issue?
> 
> James

That patch does also fix my soft lockup.

Sebastian
--
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] Separate target visibility from reaped state information

[Sebastian Herbszt]

James Bottomley wrote:
> On Mon, 2016-02-01 at 19:43 -0800, Bart Van Assche wrote:
> > On 01/19/16 17:03, James Bottomley wrote:
> > > On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
> > > > > > > > > "Bart" == Bart Van Assche <bart.vanass...@sandisk.com>
> > > > > > > > > writes:
> > > > 
> > > > Bart> Instead of representing the states "visible in sysfs" and
> > > > "has
> > > > Bart> been removed from the target list" by a single state
> > > > variable,
> > > > use
> > > > Bart> two variables to represent this information.
> > > > 
> > > > James: Are you happy with the latest iteration of this? Should I
> > > > queue
> > > > it?
> > > 
> > > Well, I'm OK with the patch: it's a simple transformation of the
> > > enumerated state to a two bit state.  What I can't see is how it
> > > fixes
> > > any soft lockup.
> > > 
> > > The only change from the current workflow is that the DEL
> > > transition
> > > (now the reaped flag) is done before the spin lock is dropped which
> > > would fix a tiny window for two threads both trying to remove the
> > > same
> > > target, but there's nothing that could possibly fix an iterative
> > > soft
> > > lockup caused by restarting the loop, which is what the changelog
> > > says.
> > 
> > Hello James,
> > 
> > scsi_remove_target() doesn't lock the scan_mutex which means that 
> > concurrent SCSI scanning activity is not prohibited. Such scanning 
> > activity can postpone the transition of the state of a SCSI target 
> > into STARGET_DEL. I think if the scheduler decides to run the thread 
> > that executes scsi_remove_target() on the same CPU as the scanning 
> > code after the scanning code has obtained a reap ref and before the 
> > scanning code has released the reap ref again that the soft lockup 
> > can be triggered that has been reported by Sebastian Herbszt.
> 
> OK, I finally understand the scenario;  I'm not sure I understand how
> we're getting concurrent scanning and removal from a simple rmmod ... I
> take it this is insmod rmmod in a tight loop?

I am able to trigger the soft lockup with this test case run once:

modprobe lpfc
run fio for 10 seconds
rmmod lpfc

My test setup involves running qla2xxx in target mode (SCST) and
lpfc as initiator on the same system with one exported volume.

Dick, how did you trigger the lockup?

Sebastian
--
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] Separate target visibility from reaped state information

[Sebastian Herbszt]

James Bottomley wrote:
> On Wed, 2016-02-03 at 23:38 +0100, Sebastian Herbszt wrote:
> > James Bottomley wrote:
> > > On Mon, 2016-02-01 at 19:43 -0800, Bart Van Assche wrote:
> > > > On 01/19/16 17:03, James Bottomley wrote:
> > > > > On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
> > > > > > > > > > > "Bart" == Bart Van Assche <
> > > > > > > > > > > bart.vanass...@sandisk.com>
> > > > > > > > > > > writes:
> > > > > > 
> > > > > > Bart> Instead of representing the states "visible in sysfs"
> > > > > > and
> > > > > > "has
> > > > > > Bart> been removed from the target list" by a single state
> > > > > > variable,
> > > > > > use
> > > > > > Bart> two variables to represent this information.
> > > > > > 
> > > > > > James: Are you happy with the latest iteration of this?
> > > > > > Should I
> > > > > > queue
> > > > > > it?
> > > > > 
> > > > > Well, I'm OK with the patch: it's a simple transformation of
> > > > > the
> > > > > enumerated state to a two bit state.  What I can't see is how
> > > > > it
> > > > > fixes
> > > > > any soft lockup.
> > > > > 
> > > > > The only change from the current workflow is that the DEL
> > > > > transition
> > > > > (now the reaped flag) is done before the spin lock is dropped
> > > > > which
> > > > > would fix a tiny window for two threads both trying to remove
> > > > > the
> > > > > same
> > > > > target, but there's nothing that could possibly fix an
> > > > > iterative
> > > > > soft
> > > > > lockup caused by restarting the loop, which is what the
> > > > > changelog
> > > > > says.
> > > > 
> > > > Hello James,
> > > > 
> > > > scsi_remove_target() doesn't lock the scan_mutex which means that
> > > > concurrent SCSI scanning activity is not prohibited. Such
> > > > scanning 
> > > > activity can postpone the transition of the state of a SCSI
> > > > target 
> > > > into STARGET_DEL. I think if the scheduler decides to run the
> > > > thread 
> > > > that executes scsi_remove_target() on the same CPU as the
> > > > scanning 
> > > > code after the scanning code has obtained a reap ref and before
> > > > the 
> > > > scanning code has released the reap ref again that the soft
> > > > lockup 
> > > > can be triggered that has been reported by Sebastian Herbszt.
> > > 
> > > OK, I finally understand the scenario;  I'm not sure I understand
> > > how
> > > we're getting concurrent scanning and removal from a simple rmmod
> > > ... I
> > > take it this is insmod rmmod in a tight loop?
> > 
> > I am able to trigger the soft lockup with this test case run once:
> > 
> > modprobe lpfc
> > run fio for 10 seconds
> > rmmod lpfc
> > 
> > My test setup involves running qla2xxx in target mode (SCST) and
> > lpfc as initiator on the same system with one exported volume.
> 
> Hm, that doesn't seem to involve scanning colliding with removal. 
>  Probably something else is pinning the target for some reason ...
> unless there's some instability or constant change on the FC fabric
> itself?  FC is hotplug, so if devices continually appear and disappear,
> they'd effectively cause a scan of the added device to take place even
> during the rmmod.

No FC fabric involved; there is a direct connection between both HBAs.

> Anyway, does the last_scan patch fix the issue?

Will try to test it soon.

> James

Sebastian
--
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] Separate target visibility from reaped state information

[Sebastian Herbszt]

James Bottomley wrote:
> On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
> > > > > > > "Bart" == Bart Van Assche 
> > > > > > > writes:
> > 
> > Bart> Instead of representing the states "visible in sysfs" and "has
> > Bart> been removed from the target list" by a single state variable,
> > use
> > Bart> two variables to represent this information.
> > 
> > James: Are you happy with the latest iteration of this? Should I
> > queue
> > it?
> 
> Well, I'm OK with the patch: it's a simple transformation of the
> enumerated state to a two bit state.  What I can't see is how it fixes
> any soft lockup.
> 
> The only change from the current workflow is that the DEL transition
> (now the reaped flag) is done before the spin lock is dropped which
> would fix a tiny window for two threads both trying to remove the same
> target, but there's nothing that could possibly fix an iterative soft
> lockup caused by restarting the loop, which is what the changelog says.
> 
> James

James, Martin, what's the status of this patch?
I still hit the reported soft lockup on 4.5-rc1.

Sebastian
--
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] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl

[Sebastian Herbszt]

Johannes Thumshirn wrote:
> This removes a redundant code block that will either be executed if the
> ENABLE_FCP_RING_POLLING flag is set in phba->cfg_poll or not. The code is just
> duplicated in both cases, hence we unify it again.
> 
> This probably is a left over from some sort of refactoring.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 v2, RESEND] Separate target visibility from reaped state information

[Sebastian Herbszt]

James Bottomley wrote:
> On Wed, 2016-01-06 at 09:24 +0100, Bart Van Assche wrote:
> > Instead of representing the states "visible in sysfs" and
> > "has been removed from the target list" by a single state
> > variable, use two variables to represent this information.
> > 
> > This patch avoids that SCSI device removal can trigger a
> > soft lockup.
> 
> It does?  When I asked you this the last time, you said the soft lockup
> was fixed by a prior patch:
> 
> http://thread.gmane.org/gmane.linux.scsi/107248
> 
> If you've actually caught a problem, can we have details because the
> distro people will want to know what gets fixed by this.
> 
> Thanks,
> 
> James

Which details do you need?
 
> > See also:
> > * "scsi: restart list search after unlock in scsi_remove_target"
> >   (commit 40998193560d).
> > * "scsi_remove_target: fix softlockup regression on hot remove"
> >   (commit bc3f02a795d3).
> > 
> > Reported-by: Sebastian Herbszt <herb...@gmx.de>
> > Tested-by: Sebastian Herbszt <herb...@gmx.de>
> > Signed-off-by: Bart Van Assche <bart.vanass...@sandisk.com>
> > Cc: Christoph Hellwig <h...@lst.de>
> > Cc: Johannes Thumshirn <jthumsh...@suse.de>
> > Cc: Dan Williams <dan.j.willi...@intel.com>
> > Cc: stable <sta...@vger.kernel.org>
> > ---
> >  drivers/scsi/scsi_scan.c   | 31 +++
> >  drivers/scsi/scsi_sysfs.c  |  7 ---
> >  include/scsi/scsi_device.h |  9 ++---
> >  3 files changed, 9 insertions(+), 38 deletions(-)
> > 
> > See also:
> > - http://thread.gmane.org/gmane.linux.scsi/107245.

I was able to hit a soft lockup and reported it here:

> > - http://thread.gmane.org/gmane.linux.scsi/108614.

Sebastian
--
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: NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [kworker/1:1:29]

[Sebastian Herbszt]

I wrote:
> Christoph Hellwig wrote:
> > On Thu, Dec 31, 2015 at 06:04:15PM +0100, Sebastian Herbszt wrote:
> > > I still get this on 4.4.0-rc7-1.g276c9f4-default. Since this did not
> > > happen on 4.3 I checked the scsi changes and found the following commit:
> > > 
> > > scsi: restart list search after unlock in scsi_remove_target
> > > 
> > > Christoph, can it cause this issue?
> > 
> > Apparently yes.  Bad hard a patch to avoid this when he resubmitted
> > my patch, which for some reason didn't get apply.  Can you grab it
> > from the list archives and give it a try?
> 
> Do you mean the following patch?
> 
> "[PATCH v2] Separate target visibility from reaped state information" [1]
> 
> I can give it a try but it will likely take a few days.
> 
> [1] http://marc.info/?l=linux-scsi=144771953020415=2

Bart's patch mentioned above seems to fix my issue.
The soft lockup is gone and now I am getting the following but that
already happened on 4.3 too:

[  531.269588] [ cut here ]
[  531.269609] WARNING: CPU: 0 PID: 3482 at fs/sysfs/group.c:237 
sysfs_remove_group+0x8f/0xa0()
[  531.269616] sysfs group c0ad62c8 not found for kobject '4:0:0:0'
[  531.269620] Modules linked in: lpfc(-) qla2x00tgt(O) qla2xxx_scst(O) 
scst_vdisk(O) scst(O) dlm configfs libcrc32c scsi_transport_fc edd nfsd lockd 
grace nfs_acl snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device auth_rpcgss 
sunrpc dm_mod snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel 
snd_hda_codec snd_hda_core iTCO_wdt snd_hwdep ppdev parport_pc tg3 snd_pcm 
libphy gpio_ich acpi_cpufreq iTCO_vendor_support snd_timer parport fjes snd 
sr_mod ehci_pci ptp lpc_ich tpm_tis floppy 8250_fintek cdrom pps_core soundcore 
i2c_i801 tpm hwmon sg pcspkr i915 drm_kms_helper drm fb_sys_fops sysimgblt 
sysfillrect syscopyarea i2c_algo_bit uhci_hcd ehci_hcd usbcore button video 
usb_common fan ata_generic ata_piix ahci libahci libata thermal
[  531.269778] CPU: 0 PID: 3482 Comm: rmmod Tainted: G   O4.4.0-rc8 
#2
[  531.269785] Hardware name: FUJITSU SIEMENS ESPRIMO E   /D2164-A1, 
BIOS 5.00 R1.10.2164.A1   05/08/2006
[  531.269791]    f47cbd14 c051cfe0 00ed c09d07e8 f47cbd44 
c025aefe
[  531.269808]  c09c6620 f47cbd70 0d9a c09d07e8 00ed c041fbdf c041fbdf 
c0ad62c8
[  531.269824]   f4644408 f47cbd5c c025afe3 0009 f47cbd54 c09c6620 
f47cbd70
[  531.269841] Call Trace:
[  531.269852]  [] dump_stack+0x44/0x64
[  531.269859]  [] warn_slowpath_common+0x8e/0xd0
[  531.269865]  [] ? sysfs_remove_group+0x8f/0xa0
[  531.269869]  [] ? sysfs_remove_group+0x8f/0xa0
[  531.269874]  [] warn_slowpath_fmt+0x33/0x40
[  531.269879]  [] sysfs_remove_group+0x8f/0xa0
[  531.269885]  [] dpm_sysfs_remove+0x49/0x60
[  531.269891]  [] device_del+0x3f/0x1d0
[  531.269897]  [] device_unregister+0x1e/0x60
[  531.269903]  [] bsg_unregister_queue+0x54/0x90
[  531.269909]  [] __scsi_remove_device+0x96/0xc0
[  531.269913]  [] scsi_forget_host+0x57/0x60
[  531.269919]  [] scsi_remove_host+0x68/0x100
[  531.269940]  [] lpfc_pci_remove_one_s3+0xca/0x2b0 [lpfc]
[  531.269957]  [] lpfc_pci_remove_one+0x65/0x80 [lpfc]
[  531.269963]  [] ? __pm_runtime_resume+0x46/0x60
[  531.269970]  [] pci_device_remove+0x38/0xc0
[  531.269975]  [] __device_release_driver+0x72/0xf0
[  531.269979]  [] driver_detach+0x8f/0xa0
[  531.269984]  [] bus_remove_driver+0x4c/0xc0
[  531.269989]  [] driver_unregister+0x28/0x60
[  531.269994]  [] ? device_destroy+0x32/0x40
[  531.26]  [] ? class_dir_child_ns_type+0x10/0x10
[  531.270004]  [] pci_unregister_driver+0x18/0x70
[  531.270010]  [] ? misc_deregister+0x67/0x90
[  531.270024]  [] lpfc_exit+0x1a/0x92b [lpfc]
[  531.270031]  [] ? find_module+0x1b/0x20
[  531.270035]  [] SyS_delete_module+0x176/0x1f0
[  531.270041]  [] ? do_munmap+0x22b/0x2c0
[  531.270046]  [] ? vm_munmap+0x46/0x60
[  531.270052]  [] do_fast_syscall_32+0x91/0x140
[  531.270059]  [] sysenter_past_esp+0x3d/0x69
[  531.270064] ---[ end trace 6d7ac64edaa712d5 ]---
[  531.270160] [ cut here ]


Sebastian
--
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: NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [kworker/1:1:29]

[Sebastian Herbszt]

Christoph Hellwig wrote:
> On Thu, Dec 31, 2015 at 06:04:15PM +0100, Sebastian Herbszt wrote:
> > I still get this on 4.4.0-rc7-1.g276c9f4-default. Since this did not
> > happen on 4.3 I checked the scsi changes and found the following commit:
> > 
> > scsi: restart list search after unlock in scsi_remove_target
> > 
> > Christoph, can it cause this issue?
> 
> Apparently yes.  Bad hard a patch to avoid this when he resubmitted
> my patch, which for some reason didn't get apply.  Can you grab it
> from the list archives and give it a try?

Do you mean the following patch?

"[PATCH v2] Separate target visibility from reaped state information" [1]

I can give it a try but it will likely take a few days.

[1] http://marc.info/?l=linux-scsi=144771953020415=2

Sebastian
--
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: NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [kworker/1:1:29]

[Sebastian Herbszt]

I wrote:
> I got the following lockup on openSUSEs 4.4.0-rc5-2.g1e923a3-default kernel
> while unloading the lpfc module:
> 
> [  444.088002] NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! 
> [kworker/1:1:29]
> [  444.088002] Modules linked in: lpfc(-) qla2x00tgt(O) qla2xxx_scst(O) 
> scst_vdisk(O) scsi_transport_fc libcrc32c scst(O) dlm configfs nfsd lockd 
> grace nfs_acl auth_rpcgss sunrpc ed
> d snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device dm_mod iTCO_wdt 
> snd_hda_codec_realtek snd_hda_codec_generic gpio_ich iTCO_vendor_support 
> ppdev snd_hda_intel snd_hda_codec snd_hda
> _core snd_hwdep tg3 snd_pcm snd_timer libphy lpc_ich parport_pc ptp 
> acpi_cpufreq snd pps_core fjes parport i2c_i801 ehci_pci tpm_tis tpm sr_mod 
> cdrom soundcore floppy hwmon sg 8250_
> fintek pcspkr i915 drm_kms_helper uhci_hcd ehci_hcd drm fb_sys_fops sysimgblt 
> sysfillrect syscopyarea i2c_algo_bit usbcore button video usb_common fan 
> ata_generic ata_piix libata th
> ermal
> [  444.088002] CPU: 1 PID: 29 Comm: kworker/1:1 Tainted: G   O
> 4.4.0-rc5-2.g1e923a3-default #1
> [  444.088002] Hardware name: FUJITSU SIEMENS ESPRIMO E   /D2164-A1, 
> BIOS 5.00 R1.10.2164.A1   05/08/2006
> [  444.088002] Workqueue: fc_wq_4 fc_rport_final_delete [scsi_transport_fc]
> [  444.088002] task: f6266ec0 ti: f6268000 task.ti: f6268000
> [  444.088002] EIP: 0060:[] EFLAGS: 0286 CPU: 1
> [  444.088002] EIP is at _raw_spin_unlock_irqrestore+0x14/0x20
> [  444.088002] EAX: 0286 EBX: f20d3800 ECX: 0002 EDX: 0286
> [  444.088002] ESI: f50ba800 EDI: f2146848 EBP: f6269ec8 ESP: f6269ec8
> [  444.088002]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [  444.088002] CR0: 8005003b CR2: 08f96600 CR3: 363ae000 CR4: 06d0
> [  444.088002] Stack:
> [  444.088002]  f6269eec c066b0f7 0286 f2146848 f50ba808 f50ba800 
> f50ba800 f2146a90
> [  444.088002]  f2146848 f6269f08 f8f0a4ed f3141000 f2146800 f2146a90 
> f619fa00 0040
> [  444.088002]  f6269f40 c026cb25 0001 166c6392 0061 f6757140 
> f6136340 0004
> [  444.088002] Call Trace:
> [  444.088002]  [] scsi_remove_target+0x167/0x1c0
> [  444.088002]  [] fc_rport_final_delete+0x9d/0x1e0 
> [scsi_transport_fc]
> [  444.088002]  [] process_one_work+0x155/0x3e0
> [  444.088002]  [] worker_thread+0x37/0x490
> [  444.088002]  [] kthread+0x9b/0xb0
> [  444.088002]  [] ret_from_kernel_thread+0x21/0x40
> [  444.088002] DWARF2 unwinder stuck at ret_from_kernel_thread+0x21/0x40
> [  444.088002]
> [  444.088002] Leftover inexact backtrace:
> [  444.088002]  [] ? kthread_park+0x50/0x50
> [  444.088002] Code: c8 e8 41 7f ab ff 89 d8 5b 5d c3 8d b6 00 00 00 00 8d bf 
> 00 00 00 00 55 89 e5 3e 8d 74 26 00 0f ae f0 89 f6 c6 00 00 89 d0 50 9d <8d> 
> 74 26 00 5d c3 8d b6 00 00 00 00 55 89 e5 3e 8d 74 26 00 0f
> [  444.088002] Kernel panic - not syncing: softlockup: hung tasks
> [  444.088002] CPU: 1 PID: 29 Comm: kworker/1:1 Tainted: G   O L  
> 4.4.0-rc5-2.g1e923a3-default #1
> [  444.088002] Hardware name: FUJITSU SIEMENS ESPRIMO E   /D2164-A1, 
> BIOS 5.00 R1.10.2164.A1   05/08/2006
> [  444.088002] Workqueue: fc_wq_4 fc_rport_final_delete [scsi_transport_fc]
> [  444.088002]    f6269da4 c0518e08 0016 f6269dbc 
> c034faa0 f6266ec0
> [  444.088002]  0016 f6266ec0  f6269df0 c0303f54 c09a6589 
> 0001 0016
> [  444.088002]  f62672ac 001d f6269e8c  0187 f67506c0 
> f6750700 f6750a60
> [  444.088002] Call Trace:
> [  444.088002]  [] try_stack_unwind+0x132/0x140
> [  444.088002]  [] dump_trace+0x54/0x200
> [  444.088002]  [] show_trace_log_lvl+0x46/0x50
> [  444.088002]  [] show_stack_log_lvl+0xcb/0x110
> [  444.088002]  [] show_stack+0x26/0x50
> [  444.088002]  [] dump_stack+0x41/0x59
> [  444.088002]  [] panic+0x81/0x1b4
> [  444.088002]  [] watchdog_timer_fn+0x1e4/0x1f0
> [  444.088002]  [] __hrtimer_run_queues+0xc0/0x240
> [  444.088002]  [] hrtimer_interrupt+0x8a/0x190
> [  444.088002]  [] local_apic_timer_interrupt+0x32/0x60
> [  444.088002]  [] smp_apic_timer_interrupt+0x33/0x50
> [  444.088002]  [] apic_timer_interrupt+0x34/0x40
> [  444.088002] DWARF2 unwinder stuck at apic_timer_interrupt+0x34/0x40
> [  444.088002]
> [  444.088002] Leftover inexact backtrace:
> [  444.088002]  [] ? mipi_dsi_host_register+0x30/0x1d0
> [  444.088002]  [] ? _raw_spin_unlock_irqrestore+0x14/0x20
> [  444.088002]  [] scsi_remove_target+0x167/0x1c0
> [  444.088002]  [] fc_rport_final_delete+0x9d/0x1e0 
> [scsi_transport_fc]
> [  444.088002]  [] process_one_work+0x155/0x3e0
> [  444.088002]  [] worker_thread+0x37/0x490
> [  444.088002]  [] ? process_one_work+0x3e0/0x3e0
> [  444.088002]  [] kthread+0x9b/0xb0
> [  444.088002]  [] ret_from_kernel_thread+0x21/0x40
> [  444.088002]  [] ? kthread_park+0x50/0x50

I still get this on 4.4.0-rc7-1.g276c9f4-default. Since this did not
happen on 4.3 I checked the scsi changes and found the following commit:

scsi: restart list search 

Re: [PATCH 15/17] lpfc: Delete unnecessary checks before the function call "mempool_destroy"

[Sebastian Herbszt]

James Smart wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> 
> The mempool_destroy() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> Signed-off-by: James Smart <james.sm...@avagotech.com>

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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

NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [kworker/1:1:29]

[Sebastian Herbszt]

I got the following lockup on openSUSEs 4.4.0-rc5-2.g1e923a3-default kernel
while unloading the lpfc module:

[  444.088002] NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! 
[kworker/1:1:29]
[  444.088002] Modules linked in: lpfc(-) qla2x00tgt(O) qla2xxx_scst(O) 
scst_vdisk(O) scsi_transport_fc libcrc32c scst(O) dlm configfs nfsd lockd grace 
nfs_acl auth_rpcgss sunrpc ed
d snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device dm_mod iTCO_wdt 
snd_hda_codec_realtek snd_hda_codec_generic gpio_ich iTCO_vendor_support ppdev 
snd_hda_intel snd_hda_codec snd_hda
_core snd_hwdep tg3 snd_pcm snd_timer libphy lpc_ich parport_pc ptp 
acpi_cpufreq snd pps_core fjes parport i2c_i801 ehci_pci tpm_tis tpm sr_mod 
cdrom soundcore floppy hwmon sg 8250_
fintek pcspkr i915 drm_kms_helper uhci_hcd ehci_hcd drm fb_sys_fops sysimgblt 
sysfillrect syscopyarea i2c_algo_bit usbcore button video usb_common fan 
ata_generic ata_piix libata th
ermal
[  444.088002] CPU: 1 PID: 29 Comm: kworker/1:1 Tainted: G   O
4.4.0-rc5-2.g1e923a3-default #1
[  444.088002] Hardware name: FUJITSU SIEMENS ESPRIMO E   /D2164-A1, 
BIOS 5.00 R1.10.2164.A1   05/08/2006
[  444.088002] Workqueue: fc_wq_4 fc_rport_final_delete [scsi_transport_fc]
[  444.088002] task: f6266ec0 ti: f6268000 task.ti: f6268000
[  444.088002] EIP: 0060:[] EFLAGS: 0286 CPU: 1
[  444.088002] EIP is at _raw_spin_unlock_irqrestore+0x14/0x20
[  444.088002] EAX: 0286 EBX: f20d3800 ECX: 0002 EDX: 0286
[  444.088002] ESI: f50ba800 EDI: f2146848 EBP: f6269ec8 ESP: f6269ec8
[  444.088002]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  444.088002] CR0: 8005003b CR2: 08f96600 CR3: 363ae000 CR4: 06d0
[  444.088002] Stack:
[  444.088002]  f6269eec c066b0f7 0286 f2146848 f50ba808 f50ba800 f50ba800 
f2146a90
[  444.088002]  f2146848 f6269f08 f8f0a4ed f3141000 f2146800 f2146a90 f619fa00 
0040
[  444.088002]  f6269f40 c026cb25 0001 166c6392 0061 f6757140 f6136340 
0004
[  444.088002] Call Trace:
[  444.088002]  [] scsi_remove_target+0x167/0x1c0
[  444.088002]  [] fc_rport_final_delete+0x9d/0x1e0 
[scsi_transport_fc]
[  444.088002]  [] process_one_work+0x155/0x3e0
[  444.088002]  [] worker_thread+0x37/0x490
[  444.088002]  [] kthread+0x9b/0xb0
[  444.088002]  [] ret_from_kernel_thread+0x21/0x40
[  444.088002] DWARF2 unwinder stuck at ret_from_kernel_thread+0x21/0x40
[  444.088002]
[  444.088002] Leftover inexact backtrace:
[  444.088002]  [] ? kthread_park+0x50/0x50
[  444.088002] Code: c8 e8 41 7f ab ff 89 d8 5b 5d c3 8d b6 00 00 00 00 8d bf 
00 00 00 00 55 89 e5 3e 8d 74 26 00 0f ae f0 89 f6 c6 00 00 89 d0 50 9d <8d> 74 
26 00 5d c3 8d b6 00 00 00 00 55 89 e5 3e 8d 74 26 00 0f
[  444.088002] Kernel panic - not syncing: softlockup: hung tasks
[  444.088002] CPU: 1 PID: 29 Comm: kworker/1:1 Tainted: G   O L  
4.4.0-rc5-2.g1e923a3-default #1
[  444.088002] Hardware name: FUJITSU SIEMENS ESPRIMO E   /D2164-A1, 
BIOS 5.00 R1.10.2164.A1   05/08/2006
[  444.088002] Workqueue: fc_wq_4 fc_rport_final_delete [scsi_transport_fc]
[  444.088002]    f6269da4 c0518e08 0016 f6269dbc c034faa0 
f6266ec0
[  444.088002]  0016 f6266ec0  f6269df0 c0303f54 c09a6589 0001 
0016
[  444.088002]  f62672ac 001d f6269e8c  0187 f67506c0 f6750700 
f6750a60
[  444.088002] Call Trace:
[  444.088002]  [] try_stack_unwind+0x132/0x140
[  444.088002]  [] dump_trace+0x54/0x200
[  444.088002]  [] show_trace_log_lvl+0x46/0x50
[  444.088002]  [] show_stack_log_lvl+0xcb/0x110
[  444.088002]  [] show_stack+0x26/0x50
[  444.088002]  [] dump_stack+0x41/0x59
[  444.088002]  [] panic+0x81/0x1b4
[  444.088002]  [] watchdog_timer_fn+0x1e4/0x1f0
[  444.088002]  [] __hrtimer_run_queues+0xc0/0x240
[  444.088002]  [] hrtimer_interrupt+0x8a/0x190
[  444.088002]  [] local_apic_timer_interrupt+0x32/0x60
[  444.088002]  [] smp_apic_timer_interrupt+0x33/0x50
[  444.088002]  [] apic_timer_interrupt+0x34/0x40
[  444.088002] DWARF2 unwinder stuck at apic_timer_interrupt+0x34/0x40
[  444.088002]
[  444.088002] Leftover inexact backtrace:
[  444.088002]  [] ? mipi_dsi_host_register+0x30/0x1d0
[  444.088002]  [] ? _raw_spin_unlock_irqrestore+0x14/0x20
[  444.088002]  [] scsi_remove_target+0x167/0x1c0
[  444.088002]  [] fc_rport_final_delete+0x9d/0x1e0 
[scsi_transport_fc]
[  444.088002]  [] process_one_work+0x155/0x3e0
[  444.088002]  [] worker_thread+0x37/0x490
[  444.088002]  [] ? process_one_work+0x3e0/0x3e0
[  444.088002]  [] kthread+0x9b/0xb0
[  444.088002]  [] ret_from_kernel_thread+0x21/0x40
[  444.088002]  [] ? kthread_park+0x50/0x50

Sebastian
--
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 16/17] lpfc: Use kzalloc instead of kmalloc

[Sebastian Herbszt]

James Smart wrote:
> From: Punit Vara <punitv...@gmail.com>
> 
> This patch is to the lpfc_els.c which resolves following warning
> reported by coccicheck:
> 
> WARNING: kzalloc should be used for rdp_context, instead of
> kmalloc/memset
> 
> Signed-off-by: Punit Vara <punitv...@gmail.com>
> Signed-off-by: James Smart <james.sm...@avagotech.com>

You missed the two already available positive reviews [1][2].

Anyway,

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>

[1] http://marc.info/?l=linux-scsi=144574360607327=2
[2] http://marc.info/?l=linux-scsi=144823385015994=2
--
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 v2 1/2] scsi: lpfc: Use kzalloc instead of kmalloc

[Sebastian Herbszt]

Punit Vara wrote:
> This patch is to the lpfc_els.c which resolves following warning
> reported by coccicheck:
> 
> WARNING: kzalloc should be used for rdp_context, instead of
> kmalloc/memset
> 
> Signed-off-by: Punit Vara <punitv...@gmail.com>
> ---
>  drivers/scsi/lpfc/lpfc_els.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index 36bf58b..9729ab1 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -4990,13 +4990,12 @@ lpfc_els_rcv_rdp(struct lpfc_vport *vport, struct 
> lpfc_iocbq *cmdiocb,
>   if (RDP_NPORT_ID_SIZE !=
>   be32_to_cpu(rdp_req->nport_id_desc.length))
>   goto rjt_logerr;
> - rdp_context = kmalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
> + rdp_context = kzalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
>   if (!rdp_context) {
>   rjt_err = LSRJT_UNABLE_TPC;
>   goto error;
>   }
>  
> - memset(rdp_context, 0, sizeof(struct lpfc_rdp_context));
>   cmd = >iocb;
>   rdp_context->ndlp = lpfc_nlp_get(ndlp);
>   rdp_context->ox_id = cmd->unsli3.rcvsli3.ox_id;

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 v2] lpfc: replaced kmalloc + memset with kzalloc

[Sebastian Herbszt]

Saurabh Sengar wrote:
> replacing kmalloc and memset by a single call of kzalloc
> 
> Signed-off-by: Saurabh Sengar 
> ---
> v2 : I didn't got any response for my initial patch,
> I am sending it again on top of latest kernel(today's)
> 
>  drivers/scsi/lpfc/lpfc_els.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index b6fa257..92dd204 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -4956,13 +4956,12 @@ lpfc_els_rcv_rdp(struct lpfc_vport *vport, struct 
> lpfc_iocbq *cmdiocb,
>   if (RDP_NPORT_ID_SIZE !=
>   be32_to_cpu(rdp_req->nport_id_desc.length))
>   goto rjt_logerr;
> - rdp_context = kmalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
> + rdp_context = kzalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
>   if (!rdp_context) {
>   rjt_err = LSRJT_UNABLE_TPC;
>   goto error;
>   }
>  
> - memset(rdp_context, 0, sizeof(struct lpfc_rdp_context));
>   cmd = >iocb;
>   rdp_context->ndlp = lpfc_nlp_get(ndlp);
>   rdp_context->ox_id = cmd->unsli3.rcvsli3.ox_id;

Same patch was already sent by Punit Vara on 2015-10-24.

Sebastian
--
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] libfc: correct function name in comment

[Sebastian Herbszt]

Use the correct function name in kernel-doc comment.

Signed-off-by: Sebastian Herbszt <herb...@gmx.de>
Acked-by: Vasu Dev <vasu@intel.com>
---

diff -U 5 -rp a/drivers/scsi/libfc/fc_npiv.c b/drivers/scsi/libfc/fc_npiv.c
--- a/drivers/scsi/libfc/fc_npiv.c  2014-09-03 19:30:11.760273300 +0200
+++ b/drivers/scsi/libfc/fc_npiv.c  2015-08-05 00:06:17.927753719 +0200
@@ -23,11 +23,11 @@
 
 #include 
 #include 
 
 /**
- * fc_vport_create() - Create a new NPIV vport instance
+ * libfc_vport_create() - Create a new NPIV vport instance
  * @vport: fc_vport structure from scsi_transport_fc
  * @privsize: driver private data size to allocate along with the Scsi_Host
  */
 
 struct fc_lport *libfc_vport_create(struct fc_vport *vport, int privsize)
--
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 06/14] lpfc:Make the function lpfc_sli4_mbox_completions_pending static in order to comply with function prototype

[Sebastian Herbszt]

James Smart wrote:
> 
> From: Nicholas Krause <xerofo...@gmail.com>
> 
> This makes the function lpfc_sli4_mbox_completion's definition
> static now in order to comply with its prototype being also
> declared as static too.
> 
> Signed-off-by: Nicholas Krause <xerofo...@gmail.com>
> Signed-off-by: James Smart <james.sm...@avagotech.com>
> ---
>  drivers/scsi/lpfc/lpfc_sli.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
> index 4feb931..95d53c7 100644
> --- a/drivers/scsi/lpfc/lpfc_sli.c
> +++ b/drivers/scsi/lpfc/lpfc_sli.c
> @@ -6696,7 +6696,7 @@ lpfc_mbox_timeout(unsigned long ptr)
>   * This function checks if any mailbox completions are present on the mailbox
>   * completion queue.
>   **/
> -bool
> +static bool
>  lpfc_sli4_mbox_completions_pending(struct lpfc_hba *phba)
>  {
>  

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 09/14] fix: lpfc_send_rscn_event sends bigger buffer size

[Sebastian Herbszt]

James Smart wrote:
> 
> From: Ales Novak <alno...@suse.cz>
> 
> lpfc_send_rscn_event() allocates data for sizeof(struct
> lpfc_rscn_event_header) + payload_len, but claims that the data has size
> of sizeof(struct lpfc_els_event_header) + payload_len. That leads to
> buffer overruns.
> 
> Signed-off-by: Ales Novak <alno...@suse.cz>
> Signed-off-by: James Smart <james.sm...@avagotech.com>
> Reviewed-by: Hannes Reinecke <h...@suse.de>
> ---
>  drivers/scsi/lpfc/lpfc_els.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
> index c859aa3..f9c957d 100644
> --- a/drivers/scsi/lpfc/lpfc_els.c
> +++ b/drivers/scsi/lpfc/lpfc_els.c
> @@ -5401,7 +5401,7 @@ lpfc_send_rscn_event(struct lpfc_vport *vport,
>  
>   fc_host_post_vendor_event(shost,
>   fc_get_event_number(),
> - sizeof(struct lpfc_els_event_header) + payload_len,
> + sizeof(struct lpfc_rscn_event_header) + payload_len,
>       (char *)rscn_event_data,
>   LPFC_NL_VENDOR_ID);
>  

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 02/14] lpfc: Remove unnessary cast

[Sebastian Herbszt]

James Smart wrote:
> 
> From: Firo Yang <fir...@gmail.com>
> 
> kzalloc() returns a void pointer - no need to cast it in
> drivers/scsi/lpfc/lpfc_init.c::lpfc_sli_driver_resource_setup()
> 
> Signed-off-by: Firo Yang <fir...@gmail.com>
> Signed-off-by: James Smart <james.sm...@avagotech.com>
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 1992e74..da9b6fc 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -4982,8 +4982,7 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
>   }
>  
>   if (!phba->sli.ring)
> - phba->sli.ring = (struct lpfc_sli_ring *)
> - kzalloc(LPFC_SLI3_MAX_RING *
> + phba->sli.ring = kzalloc(LPFC_SLI3_MAX_RING *
>   sizeof(struct lpfc_sli_ring), GFP_KERNEL);
>   if (!phba->sli.ring)
>   return -ENOMEM;

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 03/14] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize

[Sebastian Herbszt]

James Smart wrote:
> 
> From: Bodo Stroesser <bstroes...@ts.fujitsu.com>
> 
> Hi James,
> 
> We had some performance problems with RAID systems connected to LPe12k.
> AFAICS, the reason is a small bug in lpfc.ko, causing the IO-size to
> be smaller than expected.
> 
> The patch below fixes it for us.
> 
> Please CC me, I'm not on the list.
> 
> Best regards
> Bodo
> 
> --
> 
> Currently the module parameter lpfc_sg_seg_count does not have effect
> for sli3 devices.
> 
> In lpfc_sli_driver_resource_setup(), which is used for sli3, the code
> writes the configured sg_seg_cnt into lpfc_template.sg_tablesize.
> But lpfc_template is the template used for sli4 only. Thus the value should
> correctly be written to lpfc_template_s3->sg_tablesize.
> 
> This patch is for kernel 4.1-rc5, but is tested with lpfc 10.2.405.26 only.
> 
> Signed-off-by: Bodo Stroesser <bstroes...@ts.fujitsu.com>
> Signed-off-by: James Smart <james.sm...@avagotech.com>
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index da9b6fc..81bfb2d 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -4994,7 +4994,7 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
>  
>   /* Initialize the host templates the configured values. */
>   lpfc_vport_template.sg_tablesize = phba->cfg_sg_seg_cnt;
> - lpfc_template.sg_tablesize = phba->cfg_sg_seg_cnt;
> + lpfc_template_s3.sg_tablesize = phba->cfg_sg_seg_cnt;
>  
>   /* There are going to be 2 reserved BDEs: 1 FCP cmnd + 1 FCP rsp */
>   if (phba->cfg_enable_bg) {

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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 08/14] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()

[Sebastian Herbszt]

James Smart wrote:
> 
> From: Johannes Thumshirn <jthumsh...@suse.de>
> 
> If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution
> continues normally and mp gets kfree()d.
> 
> If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the
> error label where lpfc_mbuf_free() is called with mp->virt and mp->phys as
> function arguments. This is the use after free. Following the use after free 
> mp
> gets kfree()d again which is a double free.
> 
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Signed-off-by: James Smart <james.sm...@avagotech.com>
> ---
>  drivers/scsi/lpfc/lpfc_mbox.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c
> index 723e110..18838ea 100644
> --- a/drivers/scsi/lpfc/lpfc_mbox.c
> +++ b/drivers/scsi/lpfc/lpfc_mbox.c
> @@ -2276,7 +2276,7 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, 
> LPFC_MBOXQ_t *mbox)
>   (struct lpfc_rdp_context *)(mbox->context2);
>  
>   if (bf_get(lpfc_mqe_status, >u.mqe))
> - goto error;
> + goto error_mbuf_free;
>  
>   lpfc_sli_bemem_bcopy(mp->virt, _context->page_a2,
>   DMP_SFF_PAGE_A2_SIZE);
> @@ -2291,13 +2291,14 @@ lpfc_mbx_cmpl_rdp_page_a2(struct lpfc_hba *phba, 
> LPFC_MBOXQ_t *mbox)
>   mbox->mbox_cmpl = lpfc_mbx_cmpl_rdp_link_stat;
>   mbox->context2 = (struct lpfc_rdp_context *) rdp_context;
>   if (lpfc_sli_issue_mbox(phba, mbox, MBX_NOWAIT) == MBX_NOT_FINISHED)
> - goto error;
> + goto error_cmd_free;
>  
>   return;
>  
> -error:
> +error_mbuf_free:
>   lpfc_mbuf_free(phba, mp->virt, mp->phys);
>   kfree(mp);
> +error_cmd_free:
>   lpfc_sli4_mbox_cmd_free(phba, mbox);
>   rdp_context->cmpl(phba, rdp_context, FAILURE);
>  }

Reviewed-by: Sebastian Herbszt <herb...@gmx.de>
--
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] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()

[Sebastian Herbszt]

Johannes Thumshirn wrote:
 Sebastian Herbszt herb...@gmx.de writes:
 
  Johannes Thumshirn wrote:
  If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, 
  execution
  continues normally and mp gets kfree()d.
  
  If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to 
  the
  error label where lpfc_mbuf_free() is called with mp-virt and mp-phys as
  function arguments. This is the use after free. Following the use after 
  free mp
  gets kfree()d again which is a double free.
 
  A similar patch was posted by Colin Ian King on 2015-07-31 [1].
 
  [1] http://marc.info/?l=linux-scsim=143835937206204w=2
 
 OK,
 
 Is it already in James' tree (haven't checked)? The problematic code was
 merged for 4.2-rc1 so if the fix (Collin's or mine I don't care) could go
 in while we're still in the rc phase, we could avoid all that stable
 circus.
 
 Thanks for digging this out.
 
 Byte,
 Johannes

It is not yet in scsi.git.

James S., Dick, which patch do you prefer?

Sebastian
--
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] lpfc: Fix possible use-after-free and double free in lpfc_mbx_cmpl_rdp_page_a2()

[Sebastian Herbszt]

Johannes Thumshirn wrote:
 If the bf_get() call in lpfc_mbx_cmpl_rdp_page_a2() does succeeds, execution
 continues normally and mp gets kfree()d.
 
 If the subsequent call to lpfc_sli_issue_mbox() fails execution jumps to the
 error label where lpfc_mbuf_free() is called with mp-virt and mp-phys as
 function arguments. This is the use after free. Following the use after free 
 mp
 gets kfree()d again which is a double free.

A similar patch was posted by Colin Ian King on 2015-07-31 [1].

[1] http://marc.info/?l=linux-scsim=143835937206204w=2

Sebastian
--
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] lpfc: remove set but not used variables

[Sebastian Herbszt]

Remove set but not used variables.

Signed-off-by: Sebastian Herbszt herb...@gmx.de
---

diff -urp a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
--- a/drivers/scsi/lpfc/lpfc_attr.c 2015-04-15 06:18:24.669045137 +0200
+++ b/drivers/scsi/lpfc/lpfc_attr.c 2015-07-25 01:22:42.072765205 +0200
@@ -1642,8 +1642,6 @@ lpfc_##attr##_show(struct device *dev, s
struct Scsi_Host  *shost = class_to_shost(dev);\
struct lpfc_vport *vport = (struct lpfc_vport *) shost-hostdata;\
struct lpfc_hba   *phba = vport-phba;\
-   uint val = 0;\
-   val = phba-cfg_##attr;\
return snprintf(buf, PAGE_SIZE, %d\n,\
phba-cfg_##attr);\
 }
@@ -1808,8 +1806,6 @@ lpfc_##attr##_show(struct device *dev, s
 { \
struct Scsi_Host  *shost = class_to_shost(dev);\
struct lpfc_vport *vport = (struct lpfc_vport *) shost-hostdata;\
-   uint val = 0;\
-   val = vport-cfg_##attr;\
return snprintf(buf, PAGE_SIZE, %d\n, vport-cfg_##attr);\
 }
 
@@ -1835,8 +1831,6 @@ lpfc_##attr##_show(struct device *dev, s
 { \
struct Scsi_Host  *shost = class_to_shost(dev);\
struct lpfc_vport *vport = (struct lpfc_vport *) shost-hostdata;\
-   uint val = 0;\
-   val = vport-cfg_##attr;\
return snprintf(buf, PAGE_SIZE, %#x\n, vport-cfg_##attr);\
 }
 
diff -urp a/drivers/scsi/lpfc/lpfc_bsg.c b/drivers/scsi/lpfc/lpfc_bsg.c
--- a/drivers/scsi/lpfc/lpfc_bsg.c  2015-04-15 06:18:24.669045137 +0200
+++ b/drivers/scsi/lpfc/lpfc_bsg.c  2015-07-25 01:15:43.796746130 +0200
@@ -904,7 +904,6 @@ lpfc_bsg_ct_unsol_event(struct lpfc_hba
 {
uint32_t evt_req_id = 0;
uint32_t cmd;
-   uint32_t len;
struct lpfc_dmabuf *dmabuf = NULL;
struct lpfc_bsg_event *evt;
struct event_data *evt_dat = NULL;
@@ -946,7 +945,6 @@ lpfc_bsg_ct_unsol_event(struct lpfc_hba
ct_req = (struct lpfc_sli_ct_request *)dmabuf-virt;
evt_req_id = ct_req-FsType;
cmd = ct_req-CommandResponse.bits.CmdRsp;
-   len = ct_req-CommandResponse.bits.Size;
if (!(phba-sli3_options  LPFC_SLI3_HBQ_ENABLED))
lpfc_sli_ringpostbuf_put(phba, pring, dmabuf);
 
@@ -2988,7 +2986,6 @@ lpfc_bsg_diag_loopback_run(struct fc_bsg
 {
struct lpfc_vport *vport = (struct lpfc_vport *)job-shost-hostdata;
struct lpfc_hba *phba = vport-phba;
-   struct diag_mode_test *diag_mode;
struct lpfc_bsg_event *evt;
struct event_data *evdat;
struct lpfc_sli *psli = phba-sli;
@@ -3031,8 +3028,6 @@ lpfc_bsg_diag_loopback_run(struct fc_bsg
rc = -EINVAL;
goto loopback_test_exit;
}
-   diag_mode = (struct diag_mode_test *)
-   job-request-rqst_data.h_vendor.vendor_cmd;
 
if ((phba-link_state == LPFC_HBA_ERROR) ||
(psli-sli_flag  LPFC_BLOCK_MGMT_IO) ||
@@ -3293,7 +3288,6 @@ lpfc_bsg_get_dfc_rev(struct fc_bsg_job *
 {
struct lpfc_vport *vport = (struct lpfc_vport *)job-shost-hostdata;
struct lpfc_hba *phba = vport-phba;
-   struct get_mgmt_rev *event_req;
struct get_mgmt_rev_reply *event_reply;
int rc = 0;
 
@@ -3306,9 +3300,6 @@ lpfc_bsg_get_dfc_rev(struct fc_bsg_job *
goto job_error;
}
 
-   event_req = (struct get_mgmt_rev *)
-   job-request-rqst_data.h_vendor.vendor_cmd;
-
event_reply = (struct get_mgmt_rev_reply *)
job-reply-reply_data.vendor_reply.vendor_rsp;
 
@@ -4348,7 +4339,6 @@ static int
 lpfc_bsg_write_ebuf_set(struct lpfc_hba *phba, struct fc_bsg_job *job,
struct lpfc_dmabuf *dmabuf)
 {
-   struct lpfc_sli_config_mbox *sli_cfg_mbx;
struct bsg_job_data *dd_data = NULL;
LPFC_MBOXQ_t *pmboxq = NULL;
MAILBOX_t *pmb;
@@ -4362,9 +4352,6 @@ lpfc_bsg_write_ebuf_set(struct lpfc_hba
phba-mbox_ext_buf_ctx.seqNum++;
nemb_tp = phba-mbox_ext_buf_ctx.nembType;
 
-   sli_cfg_mbx = (struct lpfc_sli_config_mbox *)
-   phba-mbox_ext_buf_ctx.mbx_dmabuf-virt;
-
dd_data = kmalloc(sizeof(struct bsg_job_data), GFP_KERNEL);
if (!dd_data) {
rc = -ENOMEM;
@@ -4606,7 +4593,6 @@ lpfc_bsg_issue_mbox(struct lpfc_hba *phb
uint32_t transmit_length, receive_length, mode;
struct lpfc_mbx_sli4_config *sli4_config;
struct lpfc_mbx_nembed_cmd *nembed_sge;
-   struct mbox_header *header;
struct ulp_bde64 *bde;
uint8_t *ext = NULL;
int rc = 0;
@@ -4804,8 +4790,6 @@ lpfc_bsg_issue_mbox(struct lpfc_hba *phb
/* rebuild the command for sli4 using our
 * own buffers like we do for biu diags
 */
-   header = (struct mbox_header *)
-   pmb-un.varWords[0

[PATCH v2] qla2xxx: use t10_pi_tuple

[Sebastian Herbszt]

Drop unused scsi_dif_tuple and use t10_pi_tuple instead of own sd_dif_tuple 
copy.
Also use sg_virt() instead of open coding it.

Signed-off-by: Sebastian Herbszt herb...@gmx.de

diff -urp linux/drivers/scsi/qla2xxx-orig/qla_def.h 
linux/drivers/scsi/qla2xxx/qla_def.h
--- linux/drivers/scsi/qla2xxx-orig/qla_def.h   2015-04-15 06:18:24.681045138 
+0200
+++ linux/drivers/scsi/qla2xxx/qla_def.h2015-07-09 09:45:44.285703687 
+0200
@@ -25,6 +25,7 @@
 #include linux/firmware.h
 #include linux/aer.h
 #include linux/mutex.h
+#include linux/t10-pi.h
 
 #include scsi/scsi.h
 #include scsi/scsi_host.h
@@ -276,16 +277,6 @@
 struct req_que;
 
 /*
- * (sd.h is not exported, hence local inclusion)
- * Data Integrity Field tuple.
- */
-struct sd_dif_tuple {
-   __be16 guard_tag;   /* Checksum */
-   __be16 app_tag; /* Opaque storage */
-   __be32 ref_tag; /* Target LBA or indirect LBA */
-};
-
-/*
  * SCSI Request Block
  */
 struct srb_cmd {
diff -urp linux/drivers/scsi/qla2xxx-orig/qla_isr.c 
linux/drivers/scsi/qla2xxx/qla_isr.c
--- linux/drivers/scsi/qla2xxx-orig/qla_isr.c   2015-07-05 22:18:44.157739160 
+0200
+++ linux/drivers/scsi/qla2xxx/qla_isr.c2015-07-12 18:46:24.092536496 
+0200
@@ -1711,12 +1711,6 @@ qla2x00_handle_sense(srb_t *sp, uint8_t
}
 }
 
-struct scsi_dif_tuple {
-   __be16 guard;   /* Checksum */
-   __be16 app_tag; /* APPL identifier */
-   __be32 ref_tag; /* Target LBA or indirect LBA */
-};
-
 /*
  * Checks the guard or meta-data for the type of error
  * detected by the HBA. In case of errors, we set the
@@ -1779,7 +1773,7 @@ qla2x00_handle_dif_error(srb_t *sp, stru
if (scsi_prot_sg_count(cmd)) {
uint32_t i, j = 0, k = 0, num_ent;
struct scatterlist *sg;
-   struct sd_dif_tuple *spt;
+   struct t10_pi_tuple *pi;
 
/* Patch the corresponding protection tags */
scsi_for_each_prot_sg(cmd, sg,
@@ -1801,12 +1795,12 @@ qla2x00_handle_dif_error(srb_t *sp, stru
return 1;
}
 
-   spt = page_address(sg_page(sg)) + sg-offset;
-   spt += j;
+   pi = sg_virt(sg);
+   pi += j;
 
-   spt-app_tag = 0x;
+   pi-app_tag = 0x;
if (scsi_get_prot_type(cmd) == SCSI_PROT_DIF_TYPE3)
-   spt-ref_tag = 0x;
+   pi-ref_tag = 0x;
}
 
return 0;
diff -urp linux/drivers/scsi/qla2xxx-orig/qla_target.c 
linux/drivers/scsi/qla2xxx/qla_target.c
--- linux/drivers/scsi/qla2xxx-orig/qla_target.c2015-07-05 
22:18:44.177739161 +0200
+++ linux/drivers/scsi/qla2xxx/qla_target.c 2015-07-12 18:46:46.080537498 
+0200
@@ -2589,17 +2589,17 @@ qlt_handle_dif_error(struct scsi_qla_hos
}
 
 #if 0
-   struct sd_dif_tuple *spt;
+   struct t10_pi_tuple *pi;
/* TODO:
 * This section came from initiator. Is it valid here?
 * should ulp be override with actual val???
 */
-   spt = page_address(sg_page(sg)) + sg-offset;
-   spt += j;
+   pi = sg_virt(sg);
+   pi += j;
 
-   spt-app_tag = 0x;
+   pi-app_tag = 0x;
if (cmd-se_cmd.prot_type == SCSI_PROT_DIF_TYPE3)
-   spt-ref_tag = 0x;
+   pi-ref_tag = 0x;
 #endif
}
 
--
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][PATCH] qla2xxx: use t10_pi_tuple

[Sebastian Herbszt]

Drop unused scsi_dif_tuple and use t10_pi_tuple instead of own sd_dif_tuple 
copy.

Signed-off-by: Sebastian Herbszt herb...@gmx.de

diff -urp linux/drivers/scsi/qla2xxx-orig/qla_def.h 
linux/drivers/scsi/qla2xxx/qla_def.h
--- linux/drivers/scsi/qla2xxx-orig/qla_def.h   2015-04-15 06:18:24.681045138 
+0200
+++ linux/drivers/scsi/qla2xxx/qla_def.h2015-07-09 09:45:44.285703687 
+0200
@@ -25,6 +25,7 @@
 #include linux/firmware.h
 #include linux/aer.h
 #include linux/mutex.h
+#include linux/t10-pi.h
 
 #include scsi/scsi.h
 #include scsi/scsi_host.h
@@ -276,16 +277,6 @@
 struct req_que;
 
 /*
- * (sd.h is not exported, hence local inclusion)
- * Data Integrity Field tuple.
- */
-struct sd_dif_tuple {
-   __be16 guard_tag;   /* Checksum */
-   __be16 app_tag; /* Opaque storage */
-   __be32 ref_tag; /* Target LBA or indirect LBA */
-};
-
-/*
  * SCSI Request Block
  */
 struct srb_cmd {
diff -urp linux/drivers/scsi/qla2xxx-orig/qla_isr.c 
linux/drivers/scsi/qla2xxx/qla_isr.c
--- linux/drivers/scsi/qla2xxx-orig/qla_isr.c   2015-07-05 22:18:44.157739160 
+0200
+++ linux/drivers/scsi/qla2xxx/qla_isr.c2015-07-09 09:45:44.289703687 
+0200
@@ -1711,12 +1711,6 @@ qla2x00_handle_sense(srb_t *sp, uint8_t
}
 }
 
-struct scsi_dif_tuple {
-   __be16 guard;   /* Checksum */
-   __be16 app_tag; /* APPL identifier */
-   __be32 ref_tag; /* Target LBA or indirect LBA */
-};
-
 /*
  * Checks the guard or meta-data for the type of error
  * detected by the HBA. In case of errors, we set the
@@ -1779,7 +1773,7 @@ qla2x00_handle_dif_error(srb_t *sp, stru
if (scsi_prot_sg_count(cmd)) {
uint32_t i, j = 0, k = 0, num_ent;
struct scatterlist *sg;
-   struct sd_dif_tuple *spt;
+   struct t10_pi_tuple *pi;
 
/* Patch the corresponding protection tags */
scsi_for_each_prot_sg(cmd, sg,
@@ -1801,12 +1795,12 @@ qla2x00_handle_dif_error(srb_t *sp, stru
return 1;
}
 
-   spt = page_address(sg_page(sg)) + sg-offset;
-   spt += j;
+   pi = page_address(sg_page(sg)) + sg-offset;
+   pi += j;
 
-   spt-app_tag = 0x;
+   pi-app_tag = 0x;
if (scsi_get_prot_type(cmd) == SCSI_PROT_DIF_TYPE3)
-   spt-ref_tag = 0x;
+   pi-ref_tag = 0x;
}
 
return 0;
diff -urp linux/drivers/scsi/qla2xxx-orig/qla_target.c 
linux/drivers/scsi/qla2xxx/qla_target.c
--- linux/drivers/scsi/qla2xxx-orig/qla_target.c2015-07-05 
22:18:44.177739161 +0200
+++ linux/drivers/scsi/qla2xxx/qla_target.c 2015-07-09 09:45:44.293703687 
+0200
@@ -2589,17 +2589,17 @@ qlt_handle_dif_error(struct scsi_qla_hos
}
 
 #if 0
-   struct sd_dif_tuple *spt;
+   struct t10_pi_tuple *pi;
/* TODO:
 * This section came from initiator. Is it valid here?
 * should ulp be override with actual val???
 */
-   spt = page_address(sg_page(sg)) + sg-offset;
-   spt += j;
+   pi = page_address(sg_page(sg)) + sg-offset;
+   pi += j;
 
-   spt-app_tag = 0x;
+   pi-app_tag = 0x;
if (cmd-se_cmd.prot_type == SCSI_PROT_DIF_TYPE3)
-   spt-ref_tag = 0x;
+   pi-ref_tag = 0x;
 #endif
}
 
--
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][PATCH] lpfc: fix model description

[Sebastian Herbszt]

Remove trailing space from model description.

Signed-off-by: Sebastian Herbszt herb...@gmx.de
Reviewed-By: James Smart james.sm...@avagotech.com

diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 
4.0/drivers/scsi/lpfc/lpfc_init.c
--- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c  2015-04-15 06:18:24.673045138 
+0200
+++ 4.0/drivers/scsi/lpfc/lpfc_init.c   2015-04-22 21:03:39.203230409 +0200
@@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
phba-Port);
else if (max_speed == 0)
snprintf(descp, 255,
-   Emulex %s %s %s ,
+   Emulex %s %s %s,
m.name, m.bus, m.function);
else
snprintf(descp, 255,
--
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: [RFC PATCH 1/2] lpfc: add target infrastructure

[Sebastian Herbszt]

Hannes Reinecke wrote:
 On 06/14/2015 05:20 PM, Sebastian Herbszt wrote:
  Add target infrastructure.
  
  Signed-off-by: Sebastian Herbszt herb...@gmx.de
  ---
 Hmm. And that hooks into _which_ target mode infrastructure?

Currently it is SCST with the lpfc_scst module. I still need to figure out
how to plug it into LIO/TCM.
 
 Cheers,
 
 Hannes

Sebastian
--
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 2/2] lpfc: add target hooks

[Sebastian Herbszt]

Add target hooks.

Signed-off-by: Sebastian Herbszt herb...@gmx.de
---

diff -upNr 4.1-rc7.orig/drivers/scsi/Kconfig 4.1-rc7/drivers/scsi/Kconfig
--- 4.1-rc7.orig/drivers/scsi/Kconfig   2015-05-31 10:05:16.189204634 +0200
+++ 4.1-rc7/drivers/scsi/Kconfig2015-06-14 16:33:02.479543894 +0200
@@ -1299,6 +1299,12 @@ config SCSI_LPFC_DEBUG_FS
  This makes debugging information from the lpfc driver
  available via the debugfs filesystem.
 
+config SCSI_LPFC_TARGET
+   bool Emulex LightPulse Fibre Channel Target Support
+   depends on SCSI_LPFC
+   help
+ Support target mode.
+
 config SCSI_SIM710
tristate Simple 53c710 SCSI support (Compaq, NCR machines)
depends on (EISA || MCA)  SCSI
diff -upNr 4.1-rc7.orig/drivers/scsi/lpfc/Makefile 
4.1-rc7/drivers/scsi/lpfc/Makefile
--- 4.1-rc7.orig/drivers/scsi/lpfc/Makefile 2015-05-25 03:22:35.0 
+0200
+++ 4.1-rc7/drivers/scsi/lpfc/Makefile  2015-06-14 16:33:02.479543894 +0200
@@ -31,3 +31,8 @@ obj-$(CONFIG_SCSI_LPFC) := lpfc.o
 lpfc-objs := lpfc_mem.o lpfc_sli.o lpfc_ct.o lpfc_els.o lpfc_hbadisc.o \
lpfc_init.o lpfc_mbox.o lpfc_nportdisc.o lpfc_scsi.o lpfc_attr.o \
lpfc_vport.o lpfc_debugfs.o lpfc_bsg.o
+
+ifdef CONFIG_SCSI_LPFC_TARGET
+   ccflags-y += -DLPFC_TARGET_MODE
+   lpfc-objs += lpfc_target_api.o
+endif
diff -upNr 4.1-rc7.orig/drivers/scsi/lpfc/lpfc.h 
4.1-rc7/drivers/scsi/lpfc/lpfc.h
--- 4.1-rc7.orig/drivers/scsi/lpfc/lpfc.h   2015-05-25 03:22:35.0 
+0200
+++ 4.1-rc7/drivers/scsi/lpfc/lpfc.h2015-06-14 16:33:02.479543894 +0200
@@ -441,6 +441,9 @@ struct lpfc_vport {
unsigned long rcv_buffer_time_stamp;
uint32_t vport_flag;
 #define STATIC_VPORT   1
+#if defined LPFC_TARGET_MODE  defined _H_LPFC_TGT_API_BASE
+   tm_tgtport_t target_tgtport;
+#endif
 };
 
 struct hbq_s {
@@ -991,6 +994,15 @@ struct lpfc_hba {
spinlock_t devicelock;  /* lock for luns list */
mempool_t *device_data_mem_pool;
struct list_head luns;
+#if defined LPFC_TARGET_MODE  defined _H_LPFC_TGT_API_BASE
+   uint32_t cfg_initialize_link;
+   uint32_t cfg_fcp_mode;
+#define LPFC_FCP_MODE_INITIATOR1
+#define LPFC_FCP_MODE_TARGET   2
+   uint32_t poll_rsp_cnt;
+   uint32_t num_targets_bound;
+   tm_sliport_t target_sliport;
+#endif
 };
 
 static inline struct Scsi_Host *
diff -upNr 4.1-rc7.orig/drivers/scsi/lpfc/lpfc_attr.c 
4.1-rc7/drivers/scsi/lpfc/lpfc_attr.c
--- 4.1-rc7.orig/drivers/scsi/lpfc/lpfc_attr.c  2015-05-25 03:22:35.0 
+0200
+++ 4.1-rc7/drivers/scsi/lpfc/lpfc_attr.c   2015-06-14 16:33:02.483543894 
+0200
@@ -39,6 +39,10 @@
 #include lpfc_hw.h
 #include lpfc_sli.h
 #include lpfc_sli4.h
+#ifdef LPFC_TARGET_MODE
+#include lpfc_target_api.h
+#include lpfc_target_api_base.h
+#endif
 #include lpfc_nl.h
 #include lpfc_disc.h
 #include lpfc_scsi.h
@@ -1144,7 +1148,7 @@ board_mode_out:
  * zero on error
  * one for success
  **/
-static int
+int
 lpfc_get_hba_info(struct lpfc_hba *phba,
  uint32_t *mxri, uint32_t *axri,
  uint32_t *mrpi, uint32_t *arpi,
@@ -4571,6 +4575,30 @@ LPFC_ATTR_R(multi_ring_rctl, FC_RCTL_DD_
 LPFC_ATTR_R(multi_ring_type, FC_TYPE_IP, 1,
 255, Identifies TYPE for additional ring configuration);
 
+#ifdef LPFC_TARGET_MODE
+/*
+# lpfc_initialize_link:  Bring link up at initialization
+#0x0  = do NOT bring link up (MBX_INIT_LINK)
+#0x1  = bring link up (issue MBX_INIT_LINK)
+# Default value is 1.
+*/
+LPFC_ATTR_R(initialize_link, 1, 0, 1, Bring Link Up at initialization);
+
+/*
+# lpfc_fcp_mode: determines target/initiator behavior. This is expressed
+# as an array. Every successive pair indicates {n, c}, where n is
+# hba number and c is characteristics. Value range of c is [1, 2].
+# where LPFC_FCP_MODE_INITIATORis 1 and LPFC_FCP_MODE_TARGET is 2.
+# Its possible to be both a target and an initiator.
+# The default value can be modified with the pair {-1, c}.
+*/
+static int lpfc_fcp_mode[64] = {-1, LPFC_FCP_MODE_TARGET};
+static int num_lpfc_fcp_mode;
+module_param_array(lpfc_fcp_mode, int, num_lpfc_fcp_mode, 0);
+MODULE_PARM_DESC(lpfc_fcp_mode,
+   List of values determining target/initiator behavior);
+#endif
+
 /*
 # lpfc_fdmi_on: controls FDMI support.
 #   SetNOT Set
@@ -4806,6 +4834,9 @@ struct device_attribute *lpfc_hba_attrs[
dev_attr_lpfc_ack0,
dev_attr_lpfc_topology,
dev_attr_lpfc_scan_down,
+#ifdef LPFC_TARGET_MODE
+   dev_attr_lpfc_initialize_link,
+#endif
dev_attr_lpfc_link_speed,
dev_attr_lpfc_fcp_io_sched,
dev_attr_lpfc_fcp2_no_tgt_reset,
@@ -5808,6 +5839,27 @@ struct fc_function_template lpfc_vport_t
 void
 lpfc_get_cfgparam(struct lpfc_hba *phba)
 {
+#ifdef LPFC_TARGET_MODE
+   int i = 0, mode = LPFC_FCP_MODE_TARGET;
+
+   if (num_lpfc_fcp_mode  1) {
+   printk

[RFC PATCH 1/2] lpfc: add target infrastructure

[Sebastian Herbszt]

Add target infrastructure.

Signed-off-by: Sebastian Herbszt herb...@gmx.de
---

diff -upNr 4.1-rc7.orig/drivers/scsi/lpfc/lpfc_target_api.c 
4.1-rc7/drivers/scsi/lpfc/lpfc_target_api.c
--- 4.1-rc7.orig/drivers/scsi/lpfc/lpfc_target_api.c1970-01-01 
01:00:00.0 +0100
+++ 4.1-rc7/drivers/scsi/lpfc/lpfc_target_api.c 2015-06-14 16:31:26.079541076 
+0200
@@ -0,0 +1,902 @@
+/***
+ * This file is part of the Emulex Linux Device Driver for *
+ * Fibre Channel Host Bus Adapters.*
+ * Copyright (C) 2003-2008 Emulex.  All rights reserved.   *
+ * EMULEX and SLI are trademarks of Emulex.*
+ * www.emulex.com  *
+ * *
+ * This program is free software; you can redistribute it and/or   *
+ * modify it under the terms of version 2 of the GNU General   *
+ * Public License as published by the Free Software Foundation.*
+ * This program is distributed in the hope that it will be useful. *
+ * ALL EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND  *
+ * WARRANTIES, INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY,  *
+ * FITNESS FOR A PARTICULAR PURPOSE, OR NON-INFRINGEMENT, ARE  *
+ * DISCLAIMED, EXCEPT TO THE EXTENT THAT SUCH DISCLAIMERS ARE HELD *
+ * TO BE LEGALLY INVALID.  See the GNU General Public License for  *
+ * more details, a copy of which can be found in the file COPYING  *
+ * included with this package. *
+ ***/
+#include linux/pci.h
+#include linux/interrupt.h
+#include linux/delay.h
+#include linux/module.h
+
+#include scsi/scsi.h
+#include scsi/scsi_device.h
+#include scsi/scsi_host.h
+#include scsi/scsi_tcq.h
+#include scsi/scsi_transport_fc.h
+
+#include lpfc_version.h
+#include lpfc_hw4.h
+#include lpfc_hw.h
+#include lpfc_target_api.h
+#include lpfc_target_api_base.h
+#include lpfc_nl.h
+#include lpfc_sli.h
+#include lpfc_sli4.h
+#include lpfc_disc.h
+#include lpfc_scsi.h
+#include lpfc.h
+#include lpfc_logmsg.h
+#include lpfc_target_protos.h
+#include lpfc_crtn.h
+
+int lpfc_tgt_init(void);
+int lpfc_tgt_exit(void);
+
+int lpfctm_num_hba = 0;
+void lpfc_tm_down_link(tm_sliport_handle_t lpfc_port_handle);
+void lpfc_tm_hbq_free(struct lpfc_hba *phba, struct hbq_dmabuf *hbqbp);
+void lpfc_tm_tgtport_unbind(tm_tgtport_handle_t base_handle);
+
+extern int lpfc_sli_issue_mbox(struct lpfc_hba *, LPFC_MBOXQ_t *, uint32_t);
+extern int lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *, uint32_t, uint32_t);
+extern struct lpfc_vport *lpfc_find_vport_by_vpid(struct lpfc_hba *, uint16_t);
+void lpfc_tm_recv_unsol(struct lpfc_hba *phba,
+struct lpfc_sli_ring *pring, struct lpfc_iocbq *iocb);
+struct hbq_dmabuf *lpfc_tm_hbq_alloc(struct lpfc_hba *phba);
+void lpfc_tm_hbq_free(struct lpfc_hba *phba, struct hbq_dmabuf *hbqbp);
+int lpfc_get_hba_info(struct lpfc_hba *phba, uint32_t *mxri, uint32_t *axri,
+ uint32_t *mrpi, uint32_t *arpi, uint32_t *mvpi, uint32_t *avpi);
+
+extern struct lpfc_hbq_init *lpfc_hbq_defs[];
+
+int lpfc_target_sliport_init(struct lpfc_hba *phba, struct pci_dev *pci_dev)
+{
+   uint32_t   result;
+   tm_sliport_info_t *sliport_info;
+   tm_driver_data_t  *driver_data = lpfc_tgt_data;
+   tm_sliport_t  *sliport = phba-target_sliport;
+
+   /* set up parameters in port info */
+   sliport_info = phba-target_sliport.info;
+   sliport_info-sli_version = *(uint32_t *)phba-pcb;
+   sliport_info-iotag16_mask = 0xc000;
+   sliport_info-pcidev = phba-pcidev;
+
+   /* allow negotiation on init call if hbq num is zero in driver data */
+   sliport_info-max_tgt_contexts = phba-cfg_hba_queue_depth;
+   if (driver_data-tm_num_hbq_buf 
+   driver_data-tm_num_hbq_buf  sliport_info-max_tgt_contexts)
+   sliport_info-max_tgt_contexts = driver_data-tm_num_hbq_buf;
+
+   if (phba-sli3_options  LPFC_SLI3_HBQ_ENABLED) {
+   sliport_info-flags |= LPFC_TM_SLI3_HBQ_ENABLED;
+   sliport_info-flags |= LPFC_TM_HBQ_TAGS_SUPPORTED;
+   } else {
+   sliport_info-flags = ~LPFC_TM_SLI3_HBQ_ENABLED;
+   sliport_info-flags = ~LPFC_TM_HBQ_TAGS_SUPPORTED;
+   }
+
+   result = lpfc_tgt_data-tm_sliport_init((tm_sliport_handle_t *)phba,
+   sliport_info, phba-target_sliport.target_slihandle);
+
+   if (result == TM_RCD_SUCCESS) {
+   struct lpfc_sli *psli;
+   struct lpfc_sli_ring *pring;
+
+   psli = phba-sli;
+   pring = psli-ring[LPFC_EXTRA_RING];
+
+   /* protect against user brain damage */
+   if (sliport_info-max_tgt_contexts  phba-cfg_hba_queue_depth

Re: [PATCH] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize

[Sebastian Herbszt]

bstroes...@ts.fujitsu.com wrote:
 Hi James,
 
 We had some performance problems with RAID systems connected to LPe12k.
 AFAICS, the reason is a small bug in lpfc.ko, causing the IO-size to
 be smaller than expected.
 
 The patch below fixes it for us.
 
 Please CC me, I'm not on the list.
 
 Best regards
 Bodo
 
 
 --
 
 From: Bodo Stroesser bstroes...@ts.fujitsu.com
 Date: Fri, 29 May 2015 13:34:19 +0200
 Subject: [PATCH] lpfc: in sli3 use configured sg_seg_cnt for sg_tablesize
 
 Currently the module parameter lpfc_sg_seg_count does not have effect
 for sli3 devices.
 
 In lpfc_sli_driver_resource_setup(), which is used for sli3, the code
 writes the configured sg_seg_cnt into lpfc_template.sg_tablesize.
 But lpfc_template is the template used for sli4 only. Thus the value should
 correctly be written to lpfc_template_s3-sg_tablesize.
 
 This patch is for kernel 4.1-rc5, but is tested with lpfc 10.2.405.26 only.
 
 Signed-off-by: Bodo Stroesser bstroes...@ts.fujitsu.com
 ---
 
 --- a/drivers/scsi/lpfc/lpfc_init.c   2015-05-29 10:19:02.0 +0200
 +++ b/drivers/scsi/lpfc/lpfc_init.c   2015-05-29 10:19:56.0 +0200
 @@ -4988,7 +4988,7 @@ lpfc_sli_driver_resource_setup(struct lp
  
   /* Initialize the host templates the configured values. */
   lpfc_vport_template.sg_tablesize = phba-cfg_sg_seg_cnt;
 - lpfc_template.sg_tablesize = phba-cfg_sg_seg_cnt;
 + lpfc_template_s3.sg_tablesize = phba-cfg_sg_seg_cnt;
  
   /* There are going to be 2 reserved BDEs: 1 FCP cmnd + 1 FCP rsp */
   if (phba-cfg_enable_bg) {

Reviewed-by: Sebastian Herbszt herb...@gmx.de

Sebastian
--
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] lpfc: fix model description

[Sebastian Herbszt]

I wrote:
 Remove trailing space from model description.
 
 Signed-off-by: Sebastian Herbszt herb...@gmx.de
 
 diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 
 4.0/drivers/scsi/lpfc/lpfc_init.c
 --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c2015-04-15 06:18:24.673045138 
 +0200
 +++ 4.0/drivers/scsi/lpfc/lpfc_init.c 2015-04-22 21:03:39.203230409 +0200
 @@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
   phba-Port);
   else if (max_speed == 0)
   snprintf(descp, 255,
 - Emulex %s %s %s ,
 + Emulex %s %s %s,
   m.name, m.bus, m.function);
   else
   snprintf(descp, 255,

James,

any feedback?

Sebastian
--
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 v2 12/15] lpfc: Fix rport leak.

[Sebastian Herbszt]

James Smart wrote:
 Sebastian,
 
 Re: more than 1 space between a type declaration and a variable name - I 
 do not believe that's a hard requirement. It fully passes checkpatch. 
 Yes, consistent style use (aligning all variable names at same offset, 
 or always 1) would be good - but code has been there so long with 
 althernate styles it doesn't really matter at this point.  I did clean 
 up those in your last review as I needed to do a mod for the LS_RJT 
 behavior. But... this seems like a nit.   I did promise Christoph that I 
 would pick a good point and retrofit the sources for all sparse warnings 
 - and still owe him.
 
 Re: Checkpatch and string splitting. I understand we aren't passing 
 checkpatch for that rule, but joining them would have checkpatch 
 flagging us for beyond 80 character lines.

checkpatch seems to just follow what's mentioned in CodingStyle Chapter 2:
Breaking long lines and strings:

However, never break user-visible strings such as printk messages, because
that breaks the ability to grep for them.

The tool is actually smart enough to not flag such lines as LONG_LINE.

 I'd much rather have the 
 splits and keep the indenting for readability. We have also had this 
 error quite a bit in the past and believe we have been grandfathered as 
 there's a lot of this already.
 
 James B - any comments on the above ?
 
 -- james s

Sebastian
--
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 v2 06/15] lpfc: Add support for RDP ELS command.

[Sebastian Herbszt]

James Smart wrote:
 
 Add support for RDP ELS command.
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc.h  |   1 +
  drivers/scsi/lpfc/lpfc_crtn.h |   2 +
  drivers/scsi/lpfc/lpfc_els.c  | 420 
 ++
  drivers/scsi/lpfc/lpfc_hw.h   | 169 +
  drivers/scsi/lpfc/lpfc_hw4.h  | 201 
  drivers/scsi/lpfc/lpfc_mbox.c | 152 +++
  drivers/scsi/lpfc/lpfc_sli4.h |  10 +
  7 files changed, 955 insertions(+)

snip 

 diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
 index 3975276..724b61c 100644
 --- a/drivers/scsi/lpfc/lpfc_els.c
 +++ b/drivers/scsi/lpfc/lpfc_els.c
 @@ -4615,6 +4615,422 @@ lpfc_els_disc_plogi(struct lpfc_vport *vport)
   return sentplogi;
  }
  
 +void
 +lpfc_rdp_res_link_service(struct fc_rdp_link_service_desc *desc,
 + uint32_t word0)
 +{
 +
 + desc-tag = cpu_to_be32(RDP_LINK_SERVICE_DESC_TAG);
 + desc-payload.els_req = word0;
 + desc-length = cpu_to_be32(sizeof(desc-payload));
 +}
 +
 +void
 +lpfc_rdp_res_sfp_desc(struct fc_rdp_sfp_desc *desc,
 + uint8_t *page_a0, uint8_t *page_a2)
 +{
 + uint16_t wavelength;
 + uint16_t temperature;
 + uint16_t rx_power;
 + uint16_t tx_bias;
 + uint16_t tx_power;
 + uint16_t vcc;
 + uint16_t flag = 0;
 + struct sff_trasnceiver_codes_byte4 *trasn_code_byte4;
 + struct sff_trasnceiver_codes_byte5 *trasn_code_byte5;
 +
 + desc-tag = cpu_to_be32(RDP_SFP_DESC_TAG);
 +
 + trasn_code_byte4 = (struct sff_trasnceiver_codes_byte4 *)
 + page_a0[SSF_TRANSCEIVER_CODE_B4];
 + trasn_code_byte5 = (struct sff_trasnceiver_codes_byte5 *)
 + page_a0[SSF_TRANSCEIVER_CODE_B5];
 +
 + if ((trasn_code_byte4-fc_sw_laser) ||
 + (trasn_code_byte5-fc_sw_laser_sl) ||
 + (trasn_code_byte5-fc_sw_laser_sn)) {  /* check if its short WL */
 + flag |= (SFP_FLAG_PT_SWLASER  SFP_FLAG_PT_SHIFT);
 + } else if (trasn_code_byte4-fc_lw_laser) {
 + wavelength = (page_a0[SSF_WAVELENGTH_B1]  8) |
 + page_a0[SSF_WAVELENGTH_B0];
 + if (wavelength == SFP_WAVELENGTH_LC1310)
 + flag |= SFP_FLAG_PT_LWLASER_LC1310  SFP_FLAG_PT_SHIFT;
 + if (wavelength == SFP_WAVELENGTH_LL1550)
 + flag |= SFP_FLAG_PT_LWLASER_LL1550  SFP_FLAG_PT_SHIFT;
 + }
 + /* check if its SFP+ */
 + flag |= ((page_a0[SSF_IDENTIFIER] == SFF_PG0_IDENT_SFP) ?
 + SFP_FLAG_CT_SFP_PLUS : SFP_FLAG_CT_UNKNOWN)
 +  SFP_FLAG_CT_SHIFT;
 +
 + /* check if its OPTICAL */
 + flag |= ((page_a0[SSF_CONNECTOR] == SFF_PG0_CONNECTOR_LC) ?
 + SFP_FLAG_IS_OPTICAL_PORT : 0)
 +  SFP_FLAG_IS_OPTICAL_SHIFT;
 +
 + temperature = (page_a2[SFF_TEMPERATURE_B1]  8 |
 + page_a2[SFF_TEMPERATURE_B0]);
 + vcc = (page_a2[SFF_VCC_B1]  8 |
 + page_a2[SFF_VCC_B0]);
 + tx_power = (page_a2[SFF_TXPOWER_B1]  8 |
 + page_a2[SFF_TXPOWER_B0]);
 + tx_bias = (page_a2[SFF_TX_BIAS_CURRENT_B1]  8 |
 + page_a2[SFF_TX_BIAS_CURRENT_B0]);
 + rx_power = (page_a2[SFF_RXPOWER_B1]  8 |
 + page_a2[SFF_RXPOWER_B0]);
 + desc-sfp_info.temperature = cpu_to_be16(temperature);
 + desc-sfp_info.rx_power = cpu_to_be16(rx_power);
 + desc-sfp_info.tx_bias = cpu_to_be16(tx_bias);
 + desc-sfp_info.tx_power = cpu_to_be16(tx_power);
 + desc-sfp_info.vcc = cpu_to_be16(vcc);
 +
 + desc-sfp_info.flags = cpu_to_be16(flag);
 + desc-length = cpu_to_be32(sizeof(desc-sfp_info));
 +}
 +
 +void
 +lpfc_rdp_res_link_error(struct fc_rdp_link_error_status_desc *desc,
 + READ_LNK_VAR *stat)
 +{
 + uint32_t type;
 +
 + desc-tag = cpu_to_be32(RDP_LINK_ERROR_STATUS_DESC_TAG);
 +
 + type = VN_PT_PHY_PF_PORT  VN_PT_PHY_SHIFT;
 +
 + desc-info.port_type = cpu_to_be32(type);
 +
 + desc-info.link_status.link_failure_cnt =
 + cpu_to_be32(stat-linkFailureCnt);
 + desc-info.link_status.loss_of_synch_cnt =
 + cpu_to_be32(stat-lossSyncCnt);
 + desc-info.link_status.loss_of_signal_cnt =
 + cpu_to_be32(stat-lossSignalCnt);
 + desc-info.link_status.primitive_seq_proto_err =
 + cpu_to_be32(stat-primSeqErrCnt);
 + desc-info.link_status.invalid_trans_word =
 + cpu_to_be32(stat-invalidXmitWord);
 + desc-info.link_status.invalid_crc_cnt = cpu_to_be32(stat-crcCnt);
 +
 + desc-length = cpu_to_be32(sizeof(desc-info));
 +}
 +
 +void
 +lpfc_rdp_res_speed(struct fc_rdp_port_speed_desc *desc, struct lpfc_hba 
 *phba)
 +{
 + uint16_t rdp_cap = 0;
 + uint16_t rdp_speed;
 +
 + desc-tag = cpu_to_be32(RDP_PORT_SPEED_DESC_TAG);
 +
 + switch 

Re: [PATCH v2 03/15] lpfc: Add support for ELS LCB.

[Sebastian Herbszt]

James Smart wrote:
 
 Add support for ELS LCB.
 
 Also has a little whitespace fixing.
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc.h  |   1 +
  drivers/scsi/lpfc/lpfc_els.c  | 239 
 +-
  drivers/scsi/lpfc/lpfc_hw.h   |  32 ++
  drivers/scsi/lpfc/lpfc_hw4.h  |  27 -
  drivers/scsi/lpfc/lpfc_init.c |   2 +
  drivers/scsi/lpfc/lpfc_sli4.h |  11 ++
  6 files changed, 306 insertions(+), 6 deletions(-)

snip

 +/**
 + * lpfc_els_rcv_lcb - Process an unsolicited LCB
 + * @vport: pointer to a host virtual N_Port data structure.
 + * @cmdiocb: pointer to lpfc command iocb data structure.
 + * @ndlp: pointer to a node-list data structure.
 + *
 + * This routine processes an unsolicited LCB(LINK CABLE BEACON) IOCB.
 + * First, the payload of the unsolicited LCB is checked.
 + * Then based on Subcommand beacon will either turn on or off.
 + *
 + * Return code
 + * 0 - Sent the acc response
 + * 1 - Sent the reject response.
 + **/
 +static int
 +lpfc_els_rcv_lcb(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
 +  struct lpfc_nodelist *ndlp)
 +{
 + struct lpfc_hba *phba = vport-phba;
 + struct lpfc_dmabuf *pcmd;
 + IOCB_t *icmd;
 + uint8_t *lp;
 + struct fc_lcb_request_frame *beacon;
 + struct lpfc_lcb_context *lcb_context;
 + uint8_t state, rjt_err;
 + struct ls_rjt stat;
 +
 + icmd = cmdiocb-iocb;
 + pcmd = (struct lpfc_dmabuf *)cmdiocb-context2;
 + lp = (uint8_t *)pcmd-virt;
 + beacon = (struct fc_lcb_request_frame *)pcmd-virt;
 +
 + lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS,
 + 0192 ELS LCB Data x%x x%x x%x x%x sub x%x 
 + type x%x frequency %x duration x%x\n,
 + lp[0], lp[1], lp[2],
 + beacon-lcb_command,
 + beacon-lcb_sub_command,
 + beacon-lcb_type,
 + beacon-lcb_frequency,
 + be16_to_cpu(beacon-lcb_duration));
 +

checkpatch suggests to not split this user-visible string.

Sebastian
--
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 v2 12/15] lpfc: Fix rport leak.

[Sebastian Herbszt]

James Smart wrote:
 
 Fix rport leak.
 
 Correct locking and refcounting in tracking our rports
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc_disc.h|   4 +-
  drivers/scsi/lpfc/lpfc_els.c |  12 +++-
  drivers/scsi/lpfc/lpfc_hbadisc.c | 145 
 +++
  3 files changed, 79 insertions(+), 82 deletions(-)

snip

 diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c 
 b/drivers/scsi/lpfc/lpfc_hbadisc.c
 index 0dfa566..88af258 100644
 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
 +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
 @@ -106,6 +106,7 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
   struct lpfc_rport_data *rdata;
   struct lpfc_nodelist * ndlp;
   struct lpfc_vport *vport;
 + struct Scsi_Host *shost;
   struct lpfc_hba   *phba;
   struct lpfc_work_evt *evtp;
   int  put_node;
 @@ -146,49 +147,32 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
   if (ndlp-nlp_state == NLP_STE_MAPPED_NODE)
   return;
  
 - if (ndlp-nlp_type  NLP_FABRIC) {
 -
 - /* If the WWPN of the rport and ndlp don't match, ignore it */
 - if (rport-port_name != wwn_to_u64(ndlp-nlp_portname.u.wwn)) {
 - lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
 - 6789 rport name %lx != node port name %lx,
 - (unsigned long)rport-port_name,
 - (unsigned long)wwn_to_u64(
 - ndlp-nlp_portname.u.wwn));
 - put_node = rdata-pnode != NULL;
 - put_rport = ndlp-rport != NULL;
 - rdata-pnode = NULL;
 - ndlp-rport = NULL;
 - if (put_node)
 - lpfc_nlp_put(ndlp);
 - if (put_rport)
 - put_device(rport-dev);
 - return;
 - }
 -
 - put_node = rdata-pnode != NULL;
 - put_rport = ndlp-rport != NULL;
 - rdata-pnode = NULL;
 - ndlp-rport = NULL;
 - if (put_node)
 - lpfc_nlp_put(ndlp);
 - if (put_rport)
 - put_device(rport-dev);
 - return;
 - }
 + if (rport-port_name != wwn_to_u64(ndlp-nlp_portname.u.wwn))
 + lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
 + 6789 rport name %llx != node port name %llx,
 + rport-port_name,
 + wwn_to_u64(ndlp-nlp_portname.u.wwn));
  
   evtp = ndlp-dev_loss_evt;
  
 - if (!list_empty(evtp-evt_listp))
 + if (!list_empty(evtp-evt_listp)) {
 + lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE,
 + 6790 rport name %llx dev_loss_evt pending,
 + rport-port_name);
   return;
 + }
  
 - evtp-evt_arg1  = lpfc_nlp_get(ndlp);
 - ndlp-nlp_add_flag |= NLP_IN_DEV_LOSS;
 + shost = lpfc_shost_from_vport(vport);
 + spin_lock_irq(shost-host_lock);
 + ndlp-nlp_flag |= NLP_IN_DEV_LOSS;
 + spin_unlock_irq(shost-host_lock);
  
 - spin_lock_irq(phba-hbalock);
   /* We need to hold the node by incrementing the reference
* count until this queued work is done
*/
 + evtp-evt_arg1  = lpfc_nlp_get(ndlp);

Additional space here

 +
 + spin_lock_irq(phba-hbalock);
   if (evtp-evt_arg1) {
   evtp-evt = LPFC_EVT_DEV_LOSS;
   list_add_tail(evtp-evt_listp, phba-work_list);

snip

 @@ -4799,14 +4782,24 @@ lpfc_nlp_remove(struct lpfc_vport *vport, struct 
 lpfc_nodelist *ndlp)
   lpfc_cleanup_node(vport, ndlp);
  
   /*
 -  * We can get here with a non-NULL ndlp-rport because when we
 -  * unregister a rport we don't break the rport/node linkage.  So if we
 -  * do, make sure we don't leaving any dangling pointers behind.
 +  * ndlp-rport must be set to NULL before it reaches here
 +  * i.e. break rport/node link before doing lpfc_nlp_put for
 +  * registered rport and then drop the reference of rport.
*/
   if (ndlp-rport) {
 - rdata = ndlp-rport-dd_data;
 + /*
 +  * extra lpfc_nlp_put dropped the reference of ndlp
 +  * for registered rport so need to cleanup rport
 +  */
 + lpfc_printf_vlog(vport, KERN_WARNING, LOG_NODE,
 + 0940 removed node x%p DID x%x 
 +  rport not null %p\n,
 + ndlp, ndlp-nlp_DID, ndlp-rport);

checkpatch suggests to not split this user-visible string.

 + rport = ndlp-rport;
 + rdata = rport-dd_data;
   rdata-pnode = NULL;
   ndlp-rport = NULL;
 + 

Re: [PATCH v2 01/15] lpfc: The lpfc driver does not issue RFF_ID and RFT_ID in the correct sequence

[Sebastian Herbszt]

James Smart wrote:
 
 The lpfc driver does not issue RFF_ID and RFT_ID in the correct sequence
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc_hbadisc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c 
 b/drivers/scsi/lpfc/lpfc_hbadisc.c
 index 2500f15..f2db89f 100644
 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
 +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
 @@ -3868,11 +3868,11 @@ out:
  
   if (vport-port_state  LPFC_VPORT_READY) {
   /* Link up discovery requires Fabric registration. */
 - lpfc_ns_cmd(vport, SLI_CTNS_RFF_ID, 0, 0); /* Do this first! */
   lpfc_ns_cmd(vport, SLI_CTNS_RNN_ID, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RSNN_NN, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RSPN_ID, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RFT_ID, 0, 0);
 + lpfc_ns_cmd(vport, SLI_CTNS_RFF_ID, 0, 0);
  
   /* Issue SCR just before NameServer GID_FT Query */
   lpfc_issue_els_scr(vport, SCR_DID, 0);

Reviewed-by: Sebastian Herbszt herb...@gmx.de

Sebastian
--
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 6/9] lpfc: Fix cq_id masking problem.

[Sebastian Herbszt]

James Smart wrote:
 
 Fix cq_id masking problem.
 
 The driver inadvertainly constrained the id space to 8 bits, when its

inadvertently, it's

 a full 16 bits from hw. This resulted in bad id's being written to the
 hardware. Resulted in symptoms such as incomplete initialization, no
 detection of link events, etc.

Sebastian
--
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] lpfc: Fix to drop PLOGIs from fabric node till LOGO processing completes

[Sebastian Herbszt]

James Smart wrote:
 
 Fix to drop PLOGIs from fabric node till LOGO processing completes
 
 The domain controller PLOGI's concurrent with prior LOGO's/unreg_rpi's
 completing created a race condition where driver rpi ref count can
 inadvertantly hit 0 and the rpi attempted to be freed. This error

inadvertently

 sometimes resulted in Warning messages indicating kref.h via

warning

 lfpc_nlp_get+0x128.
 
 Correct by dropping any new PLOGI until the prior nport state has settled.

Sebastian
--
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 3/9] lpfc: Devices are not discovered during takeaway/giveback testing

[Sebastian Herbszt]

James Smart wrote:
 
 Devices are not discovered during takeaway/giveback testing
 
 When a remote nport changes it's DID, a new ndlp is used.

its

 However, we left the old ndlp state unchanged and still in
 a discovery state. The may stall discovery resulting in some

The?

 devices not being discovered.  Correct by swapping the state

two spaces

 of the 2 ndlp's when a DID swap is detected.

Sebastian
--
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 4/9] lpfc: Add support for using block multi-queue

[Sebastian Herbszt]

James Smart wrote:
 
 Add support for using block multi-queue
 
 With blk-mq support in the mid-layer, lpfc can do IO steering based
 on the information in the request tag.  This patch allows lpfc to use

two spaces

 blk-mq if enabled. If not enabled, we fall back into the emulex-internal
 affinity mappings.
 
 This feature can be turned on via CONFIG_SCSI_MQ_DEFAULT or passing
 scsi_mod.use_blk_mq=Y as a parameter to the kernel.
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc_init.c |  4 ++-
  drivers/scsi/lpfc/lpfc_scsi.c | 43 +
  drivers/scsi/lpfc/lpfc_scsi.h |  3 ++
  drivers/scsi/lpfc/lpfc_sli.c  | 74 
 ++-
  4 files changed, 72 insertions(+), 52 deletions(-)

snip

 diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
 index 116df9c..4a2a818 100644
 --- a/drivers/scsi/lpfc/lpfc_scsi.c
 +++ b/drivers/scsi/lpfc/lpfc_scsi.c
 @@ -3846,6 +3846,49 @@ lpfc_handle_fcp_err(struct lpfc_vport *vport, struct 
 lpfc_scsi_buf *lpfc_cmd,
  }
  
  /**
 + * lpfc_sli4_scmd_to_wqidx_distr - scsi command to SLI4 WQ index distribution
 + * @phba: Pointer to HBA context object.

@lpfc_cmd is missing

 + *
 + * This routine performs a roundrobin SCSI command to SLI4 FCP WQ index
 + * distribution.  This is called by __lpfc_sli_issue_iocb_s4() with the 
 hbalock

two spaces

 + * held.
 + * If scsi-mq is enabled, get the default block layer mapping of software 
 queues
 + * to hardware queues. This information is saved in request tag.
 + *
 + * Return: index into SLI4 fast-path FCP queue index.
 + **/
 +int lpfc_sli4_scmd_to_wqidx_distr(struct lpfc_hba *phba,
 +   struct lpfc_scsi_buf *lpfc_cmd)
 +{
 + struct scsi_cmnd *cmnd = lpfc_cmd-pCmd;
 + struct lpfc_vector_map_info *cpup;
 + int chann, cpu;
 + uint32_t tag;
 + uint16_t hwq;
 +
 + if (shost_use_blk_mq(cmnd-device-host)) {
 + tag = blk_mq_unique_tag(cmnd-request);
 + hwq = blk_mq_unique_tag_to_hwq(tag);
 +

remove new line?

 + return hwq;
 + }
 +
 + if (phba-cfg_fcp_io_sched == LPFC_FCP_SCHED_BY_CPU
 +  phba-cfg_fcp_io_channel  1) {
 + cpu = smp_processor_id();
 + if (cpu  phba-sli4_hba.num_present_cpu) {
 + cpup = phba-sli4_hba.cpu_map;
 + cpup += cpu;
 + return cpup-channel_id;
 + }
 + }

maybe add new line?

 + chann = atomic_add_return(1, phba-fcp_qidx);
 + chann = (chann % phba-cfg_fcp_io_channel);
 + return chann;
 +}
 +
 +
 +/**
   * lpfc_scsi_cmd_iocb_cmpl - Scsi cmnd IOCB completion routine
   * @phba: The Hba for which this call is being executed.
   * @pIocbIn: The command IOCBQ for the scsi cmnd.
 diff --git a/drivers/scsi/lpfc/lpfc_scsi.h b/drivers/scsi/lpfc/lpfc_scsi.h
 index 474e30c..18b9260 100644
 --- a/drivers/scsi/lpfc/lpfc_scsi.h
 +++ b/drivers/scsi/lpfc/lpfc_scsi.h
 @@ -184,3 +184,6 @@ struct lpfc_scsi_buf {
  #define FIND_FIRST_OAS_LUN0
  #define NO_MORE_OAS_LUN  -1
  #define NOT_OAS_ENABLED_LUN  NO_MORE_OAS_LUN
 +
 +int lpfc_sli4_scmd_to_wqidx_distr(struct lpfc_hba *phba,
 +   struct lpfc_scsi_buf *lpfc_cmd);
 diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
 index 41d3370..07df296 100644
 --- a/drivers/scsi/lpfc/lpfc_sli.c
 +++ b/drivers/scsi/lpfc/lpfc_sli.c

snip

 @@ -8807,27 +8777,29 @@ int
  lpfc_sli_calc_ring(struct lpfc_hba *phba, uint32_t ring_number,
   struct lpfc_iocbq *piocb)
  {
 - if (phba-sli_rev == LPFC_SLI_REV4) {
 - if (piocb-iocb_flag   (LPFC_IO_FCP | LPFC_USE_FCPWQIDX)) {
 - if (!(phba-cfg_fof) ||
 - (!(piocb-iocb_flag  LPFC_IO_FOF))) {
 - if (unlikely(!phba-sli4_hba.fcp_wq))
 - return LPFC_HBA_ERROR;
 - /*
 -  * for abort iocb fcp_wqidx should already
 -  * be setup based on what work queue we used.
 -  */
 - if (!(piocb-iocb_flag  LPFC_USE_FCPWQIDX))
 - piocb-fcp_wqidx =
 - lpfc_sli4_scmd_to_wqidx_distr(phba);
 - ring_number = MAX_SLI3_CONFIGURED_RINGS +
 - piocb-fcp_wqidx;
 - } else {
 - if (unlikely(!phba-sli4_hba.oas_wq))
 - return LPFC_HBA_ERROR;
 - piocb-fcp_wqidx = 0;
 - ring_number =  LPFC_FCP_OAS_RING;
 - }
 + if (phba-sli_rev  LPFC_SLI_REV4)
 +   

Re: [PATCH 1/9] lpfc: Check for active portpeerbeacon.

[Sebastian Herbszt]

James Smart wrote:
 
 Check for active portpeerbeacon.
 
 LCB requests to set Beacon would fail if the beacon was already enabled

beacon

 internally as the mailbox command used to query the state failes with

fails

 an already-set status.
 
 Correct by enhancing the check so we don't fail if if the already set

if twice

 status comes back.
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc_els.c | 19 +++
  drivers/scsi/lpfc/lpfc_hw4.h |  6 ++
  2 files changed, 21 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
 index 011c8d8..3aad09f 100644
 --- a/drivers/scsi/lpfc/lpfc_els.c
 +++ b/drivers/scsi/lpfc/lpfc_els.c
 @@ -5044,25 +5044,36 @@ lpfc_els_lcb_rsp(struct lpfc_hba *phba, LPFC_MBOXQ_t 
 *pmb)
   struct lpfc_iocbq *elsiocb;
   struct lpfc_nodelist *ndlp;
   struct ls_rjt *stat;
 + union lpfc_sli4_cfg_shdr *shdr;
   struct lpfc_lcb_context *lcb_context;
   struct fc_lcb_res_frame *lcb_res;
 - uint32_t cmdsize;
 + uint32_t cmdsize, shdr_status, shdr_add_status;
   int rc;
  
   mb = pmb-u.mb;
 -
   lcb_context = (struct lpfc_lcb_context *)pmb-context1;
   ndlp = lcb_context-ndlp;
   pmb-context1 = NULL;
   pmb-context2 = NULL;
  
 - if (mb-mbxStatus) {
 + shdr = (union lpfc_sli4_cfg_shdr *)
 + pmb-u.mqe.un.beacon_config.header.cfg_shdr;
 + shdr_status = bf_get(lpfc_mbox_hdr_status, shdr-response);
 + shdr_add_status = bf_get(lpfc_mbox_hdr_add_status, shdr-response);
 +
 + lpfc_printf_log(phba, KERN_INFO, LOG_MBOX,
 + 0194 SET_BEACON_CONFIG mailbox 
 + completed with status x%x add_status x%x,
 +  mbx status x%x\n,
 + shdr_status, shdr_add_status, mb-mbxStatus);

checkpatch suggests to not split this user-visible string.

Sebastian
--
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 7/9] lpfc: Fix scsi task management error message.

[Sebastian Herbszt]

James Smart wrote:
 
 Fix scsi task management error message.
 
 TMF's were getting error messages on FCP_RSP errors (underrun). Underruns
 aren't meaningful in the scenario. Change the error message to filter out
 these response check errors, and don't unconditionally mark the cmd as
 in error.
 
 Signed-off-by: Dick Kennedy dick.kenn...@avagotech.com
 Signed-off-by: James Smart james.sm...@avagotech.com
 ---
  drivers/scsi/lpfc/lpfc_scsi.c | 18 ++
  1 file changed, 10 insertions(+), 8 deletions(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
 index b3b195f..e5eb40d 100644
 --- a/drivers/scsi/lpfc/lpfc_scsi.c
 +++ b/drivers/scsi/lpfc/lpfc_scsi.c
 @@ -5011,13 +5011,16 @@ lpfc_send_taskmgmt(struct lpfc_vport *vport, struct 
 lpfc_rport_data *rdata,
 iocbq, iocbqrsp, lpfc_cmd-timeout);
   if ((status != IOCB_SUCCESS) ||
   (iocbqrsp-iocb.ulpStatus != IOSTAT_SUCCESS)) {
 - lpfc_printf_vlog(vport, KERN_ERR, LOG_FCP,
 -  0727 TMF %s to TGT %d LUN %llu failed (%d, %d) 
 -  iocb_flag x%x\n,
 -  lpfc_taskmgmt_name(task_mgmt_cmd),
 -  tgt_id, lun_id, iocbqrsp-iocb.ulpStatus,
 -  iocbqrsp-iocb.un.ulpWord[4],
 -  iocbq-iocb_flag);
 + if (status != IOCB_SUCCESS ||
 + iocbqrsp-iocb.ulpStatus != IOSTAT_FCP_RSP_ERROR)
 + lpfc_printf_vlog(vport, KERN_ERR, LOG_FCP,
 +  0727 TMF %s to TGT %d LUN %llu 
 +  failed (%d, %d) iocb_flag x%x\n,
 +  lpfc_taskmgmt_name(task_mgmt_cmd),
 +  tgt_id, lun_id,
 +  iocbqrsp-iocb.ulpStatus,
 +  iocbqrsp-iocb.un.ulpWord[4],
 +  iocbq-iocb_flag);

checkpatch suggests to not split this user-visible string.

Sebastian
--
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 kernel] commit 4fbdf9cb (lpfc: Fix for lun discovery issue with saturn adapter.)

[Sebastian Herbszt]

I wrote:
 Alexey Kardashevskiy wrote:
  On 05/06/2015 07:46 AM, Sebastian Herbszt wrote:
   James Smart wrote:
  
   Reviewed-By: James Smart james.sm...@emulex.com
  
  
   Alexey, Sebastian,
  
   Yes - this section needs to be reverted.  This patch is good.
  
   -- james s
  
   Reviewed-by: Sebastian Herbszt herb...@gmx.de
  
  
  Unfortunately, just this revert is not enough, it fixed one of my machines 
  but I have another machine with Emulex Corporation Saturn-X: LightPulse 
  Fibre Channel Host Adapter (rev 03) which does not boot - the booting 
  process stops at [  OK  ] Reached target Basic System.
  
  Any quick idea what else to revert between 4.0 and current upstream before 
  I dig further? Thanks!
 
 Is the other machine also POWER7?
 I tested the revert on x86 and it fixed my issue:
 
 4.1-rc4 (lpfc 10.5.0.0) - broken
 4.0 + lpfc 10.5.0.0 - broken
 4.1-rc4 (lpfc 10.5.0.0) + revert - ok
 4.0 + lpfc 10.5.0.0 + revert - ok

James,

are you sending this revert to Linus any time soon?

Support for SLI-3 adapters is broken on all platforms for more than
a month now [1] and POWER7 doesn't seem to like it at all [2].

[1] http://marc.info/?l=linux-scsim=142938432414173
[2] http://marc.info/?l=linux-scsim=143021737816241

Sebastian
--
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 kernel] commit 4fbdf9cb (lpfc: Fix for lun discovery issue with saturn adapter.)

[Sebastian Herbszt]

Alexey Kardashevskiy wrote:
 On 05/06/2015 07:46 AM, Sebastian Herbszt wrote:
  James Smart wrote:
 
  Reviewed-By: James Smart james.sm...@emulex.com
 
 
  Alexey, Sebastian,
 
  Yes - this section needs to be reverted.  This patch is good.
 
  -- james s
 
  Reviewed-by: Sebastian Herbszt herb...@gmx.de
 
 
 Unfortunately, just this revert is not enough, it fixed one of my machines 
 but I have another machine with Emulex Corporation Saturn-X: LightPulse 
 Fibre Channel Host Adapter (rev 03) which does not boot - the booting 
 process stops at [  OK  ] Reached target Basic System.
 
 Any quick idea what else to revert between 4.0 and current upstream before 
 I dig further? Thanks!

Is the other machine also POWER7?
I tested the revert on x86 and it fixed my issue:

4.1-rc4 (lpfc 10.5.0.0) - broken
4.0 + lpfc 10.5.0.0 - broken
4.1-rc4 (lpfc 10.5.0.0) + revert - ok
4.0 + lpfc 10.5.0.0 + revert - ok

Sebastian
--
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 kernel] commit 4fbdf9cb (lpfc: Fix for lun discovery issue with saturn adapter.)

[Sebastian Herbszt]

Alexey Kardashevskiy wrote:
 This reverts 4fbdf9cb is breaks LPFC on POWER7 machine, big endian kernel.
 
 This is the hardware used for verification:
 0005:01:00.0 Fibre Channel [0c04]: Emulex Corporation Saturn-X: LightPulse 
 Fibre Channel Host Adapter [10df:f100] (rev 03)
 0005:01:00.1 Fibre Channel [0c04]: Emulex Corporation Saturn-X: LightPulse 
 Fibre Channel Host Adapter [10df:f100] (rev 03)
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru

This issue is not specific to POWER7. I hit it on x86 [1] and James
promised to look at it.

[1] http://marc.info/?l=linux-scsim=142938432414173

Sebastian
--
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] lpfc: fix model description

[Sebastian Herbszt]

Bart Van Assche wrote:
 On 04/22/15 23:16, Sebastian Herbszt wrote:
  Remove trailing space from model description.
 
  Signed-off-by: Sebastian Herbszt herb...@gmx.de
 
  diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 
  4.0/drivers/scsi/lpfc/lpfc_init.c
  --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c  2015-04-15 06:18:24.673045138 
  +0200
  +++ 4.0/drivers/scsi/lpfc/lpfc_init.c   2015-04-22 21:03:39.203230409 
  +0200
  @@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
  phba-Port);
  else if (max_speed == 0)
  snprintf(descp, 255,
  -   Emulex %s %s %s ,
  +   Emulex %s %s %s,
  m.name, m.bus, m.function);
  else
  snprintf(descp, 255,
 
 Hello Sebastian,

Hello Bart,

 Since you are touching that code, please include the following 
 additional changes in your patch or in a separate patch:

I could follow up with a patch containing

 * Change the type of the third argument of lpfc_get_hba_model_desc()
from uint8_t * into char * since that pointer is used to store a
'\0'-terminated ASCII string.

-static void lpfc_get_hba_model_desc(struct lpfc_hba *, uint8_t *, uint8_t *);
+static void lpfc_get_hba_model_desc(struct lpfc_hba *, char *, char *);

and

-lpfc_get_hba_model_desc(struct lpfc_hba *phba, uint8_t *mdp, uint8_t *descp)
+lpfc_get_hba_model_desc(struct lpfc_hba *phba, char *mdp, char *descp)

since both mdp and descp are strings.

 * Add a size argument to lpfc_get_hba_model_desc() such that the callers
of this function can specify what the size is of the array descp
points at (sizeof(phba-ModelDesc) ?).

I am not sure whether this change is necessary because callers of
lpfc_get_hba_model_desc() are limited to lpfc_init.c and always use
the same arguments (phba-ModelName, phba-ModelDesc):

char ModelDesc[256];/* Model Description */
char ModelName[80]; /* Model Name */

 * Switch from snprintf() to scnprintf() because the latter function
guarantees '\0'-termination if the output has to be truncated.

Doesn't snprintf() guarantee this too? The only difference between
snprintf() and scnprintf() seems to be the return value.
snprintf() calls vsnprintf() and scnprintf() calls vscnprintf() which
calls vsnprintf() itself.

 Thanks,
 
 Bart.
 

Sebastian
--
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: [RFC PATCH 2/2] lpfc: add target hooks

[Sebastian Herbszt]

Andy Grover wrote:
 On 04/12/2015 07:17 AM, Sebastian Herbszt wrote:
  Add target hooks.
 
  Signed-off-by: Sebastian Herbszt herb...@gmx.de
 
  +#ifdef LPFC_TARGET_MODE
  +#include lpfc_target_api.h
  +#include lpfc_target_api_base.h
  +#endif
 
 Hi, great to see new fabrics posted.

The code is actually not new. This is just yet another attempt to get
it upstream [1].

 There are a great many ifdefs like this. Are they strictly necessary?

None should be necessary. I left them in to allow an easy way to test
the code without impact on the initiator part of lpfc.

 Thanks -- Regards -- Andy

[1] http://www.spinics.net/lists/kernel/msg1060127.html

Sebastian
--
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 03/14] lpfc: Add support for ELS LCB.

[Sebastian Herbszt]

James Smart wrote:
 
 Add support for ELS LCB.
 
 Also has a little whitespace fixing.
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc.h  |   1 +
  drivers/scsi/lpfc/lpfc_els.c  | 239 
 +-
  drivers/scsi/lpfc/lpfc_hw.h   |  32 ++
  drivers/scsi/lpfc/lpfc_hw4.h  |  27 -
  drivers/scsi/lpfc/lpfc_init.c |   2 +
  drivers/scsi/lpfc/lpfc_sli4.h |  11 ++
  6 files changed, 306 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
 index 9b81a34..3246d09 100644
 --- a/drivers/scsi/lpfc/lpfc.h
 +++ b/drivers/scsi/lpfc/lpfc.h
 @@ -230,6 +230,7 @@ struct lpfc_stats {
   uint32_t elsRcvRRQ;
   uint32_t elsRcvRTV;
   uint32_t elsRcvECHO;
 + uint32_t elsRcvLCB;
   uint32_t elsXmitFLOGI;
   uint32_t elsXmitFDISC;
   uint32_t elsXmitPLOGI;
 diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
 index 851e8ef..9f28dcb 100644
 --- a/drivers/scsi/lpfc/lpfc_els.c
 +++ b/drivers/scsi/lpfc/lpfc_els.c
 @@ -4587,16 +4587,16 @@ lpfc_els_disc_plogi(struct lpfc_vport *vport)
   if (!NLP_CHK_NODE_ACT(ndlp))
   continue;
   if (ndlp-nlp_state == NLP_STE_NPR_NODE 
 - (ndlp-nlp_flag  NLP_NPR_2B_DISC) != 0 
 - (ndlp-nlp_flag  NLP_DELAY_TMO) == 0 
 - (ndlp-nlp_flag  NLP_NPR_ADISC) == 0) {
 + (ndlp-nlp_flag  NLP_NPR_2B_DISC) != 0 
 + (ndlp-nlp_flag  NLP_DELAY_TMO) == 0 
 + (ndlp-nlp_flag  NLP_NPR_ADISC) == 0) {
   ndlp-nlp_prev_state = ndlp-nlp_state;
   lpfc_nlp_set_state(vport, ndlp, NLP_STE_PLOGI_ISSUE);
   lpfc_issue_els_plogi(vport, ndlp-nlp_DID, 0);
   sentplogi++;
   vport-num_disc_nodes++;
   if (vport-num_disc_nodes =
 - vport-cfg_discovery_threads) {
 + vport-cfg_discovery_threads) {
   spin_lock_irq(shost-host_lock);
   vport-fc_flag |= FC_NLP_MORE;
   spin_unlock_irq(shost-host_lock);
 @@ -4615,6 +4615,233 @@ lpfc_els_disc_plogi(struct lpfc_vport *vport)
   return sentplogi;
  }
  
 +static void
 +lpfc_els_lcb_rsp(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb)
 +{
 + MAILBOX_t *mb;
 + IOCB_t *icmd;
 + uint8_t *pcmd;
 + struct lpfc_iocbq *elsiocb;
 + struct lpfc_nodelist *ndlp;
 + struct ls_rjt *stat;
 + struct lpfc_lcb_context *lcb_context;
 + struct fc_lcb_res_frame  *lcb_res;

Additional space here

 + uint32_t cmdsize;
 + int rc;
 +
 + mb = pmb-u.mb;
 +
 + lcb_context  = (struct lpfc_lcb_context *)pmb-context1;

here

 + ndlp = lcb_context-ndlp;
 + pmb-context1 = NULL;
 + pmb-context2 = NULL;
 +
 + if (mb-mbxStatus) {
 + mempool_free(pmb, phba-mbox_mem_pool);
 + goto error;
 + }
 +
 + mempool_free(pmb, phba-mbox_mem_pool);
 +
 + cmdsize = sizeof(struct fc_lcb_res_frame);
 + elsiocb = lpfc_prep_els_iocb(phba-pport, 0, cmdsize,
 + lpfc_max_els_tries, ndlp,
 + ndlp-nlp_DID, ELS_CMD_ACC);
 +
 + /* Decrement the ndlp reference count from previous mbox command */
 + lpfc_nlp_put(ndlp);
 +
 + if (!elsiocb)
 + goto free_lcb_context;
 +
 + lcb_res = (struct fc_lcb_res_frame *)
 + (((struct lpfc_dmabuf *)elsiocb-context2)-virt);
 +
 + icmd = elsiocb-iocb;
 + icmd-ulpContext = lcb_context-rx_id;
 + icmd-unsli3.rcvsli3.ox_id = lcb_context-ox_id;
 +
 + pcmd = (uint8_t *)(((struct lpfc_dmabuf *)elsiocb-context2)-virt);
 + *((uint32_t *)(pcmd)) = ELS_CMD_ACC;
 + lcb_res-lcb_sub_command = lcb_context-sub_command;
 + lcb_res-lcb_type = lcb_context-type;
 + lcb_res-lcb_frequency = lcb_context-frequency;
 + elsiocb-iocb_cmpl = lpfc_cmpl_els_rsp;
 + phba-fc_stat.elsXmitACC++;
 + rc = lpfc_sli_issue_iocb(phba, LPFC_ELS_RING, elsiocb, 0);
 + if (rc == IOCB_ERROR)
 + lpfc_els_free_iocb(phba, elsiocb);
 +
 + kfree(lcb_context);
 + return;
 +
 +error:
 + cmdsize = sizeof(struct fc_lcb_res_frame);
 + elsiocb = lpfc_prep_els_iocb(phba-pport, 0, cmdsize,
 + lpfc_max_els_tries, ndlp,
 + ndlp-nlp_DID, ELS_CMD_LS_RJT);
 + lpfc_nlp_put(ndlp);
 + if (!elsiocb)
 + goto free_lcb_context;
 +
 + icmd = elsiocb-iocb;
 + icmd-ulpContext = lcb_context-rx_id;
 + icmd-unsli3.rcvsli3.ox_id = lcb_context-ox_id;
 + pcmd = (uint8_t *)(((struct lpfc_dmabuf *)elsiocb-context2)-virt);
 +
 + *((uint32_t *)(pcmd)) = ELS_CMD_LS_RJT;
 + stat = (struct ls_rjt *)(pcmd + 

Re: [PATCH 06/14] lpfc: Add support for RDP ELS command.

[Sebastian Herbszt]

James Smart wrote:
 
 Add support for RDP ELS command.
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc.h  |   1 +
  drivers/scsi/lpfc/lpfc_crtn.h |   2 +
  drivers/scsi/lpfc/lpfc_els.c  | 415 
 ++
  drivers/scsi/lpfc/lpfc_hw.h   | 169 +
  drivers/scsi/lpfc/lpfc_hw4.h  | 201 
  drivers/scsi/lpfc/lpfc_mbox.c | 152 
  drivers/scsi/lpfc/lpfc_sli4.h |  10 +
  7 files changed, 950 insertions(+)
 
 diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
 index 3246d09..a5a56fa 100644
 --- a/drivers/scsi/lpfc/lpfc.h
 +++ b/drivers/scsi/lpfc/lpfc.h
 @@ -231,6 +231,7 @@ struct lpfc_stats {
   uint32_t elsRcvRTV;
   uint32_t elsRcvECHO;
   uint32_t elsRcvLCB;
 + uint32_t elsRcvRDP;
   uint32_t elsXmitFLOGI;
   uint32_t elsXmitFDISC;
   uint32_t elsXmitPLOGI;
 diff --git a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h
 index 587e3e9..b0e6fe4 100644
 --- a/drivers/scsi/lpfc/lpfc_crtn.h
 +++ b/drivers/scsi/lpfc/lpfc_crtn.h
 @@ -498,3 +498,5 @@ bool lpfc_disable_oas_lun(struct lpfc_hba *, struct 
 lpfc_name *,
  bool lpfc_find_next_oas_lun(struct lpfc_hba *, struct lpfc_name *,
   struct lpfc_name *, uint64_t *, struct lpfc_name *,
   struct lpfc_name *, uint64_t *, uint32_t *);
 +int lpfc_sli4_dump_page_a0(struct lpfc_hba *phba, struct lpfcMboxq *mbox);
 +void lpfc_mbx_cmpl_rdp_page_a0(struct lpfc_hba *phba, LPFC_MBOXQ_t *pmb);
 diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
 index 9099a06..455a5dd 100644
 --- a/drivers/scsi/lpfc/lpfc_els.c
 +++ b/drivers/scsi/lpfc/lpfc_els.c
 @@ -4617,6 +4617,417 @@ lpfc_els_disc_plogi(struct lpfc_vport *vport)
   return sentplogi;
  }
  
 +void
 +lpfc_rdp_res_link_service(struct fc_rdp_link_service_desc *desc,
 + uint32_t word0)
 +{
 +
 + desc-tag = cpu_to_be32(RDP_LINK_SERVICE_DESC_TAG);
 + desc-payload.els_req = word0;
 + desc-length = cpu_to_be32(sizeof(desc-payload));
 +}
 +
 +void
 +lpfc_rdp_res_sfp_desc(struct fc_rdp_sfp_desc *desc,
 + uint8_t *page_a0, uint8_t *page_a2)
 +{
 + uint16_t wavelength;
 + uint16_t temperature;
 + uint16_t rx_power;
 + uint16_t tx_bias;
 + uint16_t tx_power;
 + uint16_t vcc;
 + uint16_t flag = 0;
 + struct sff_trasnceiver_codes_byte4 *trasn_code_byte4;
 + struct sff_trasnceiver_codes_byte5 *trasn_code_byte5;
 +
 + desc-tag = cpu_to_be32(RDP_SFP_DESC_TAG);
 +
 + trasn_code_byte4 = (struct sff_trasnceiver_codes_byte4 *)
 + page_a0[SSF_TRANSCEIVER_CODE_B4];
 + trasn_code_byte5 = (struct sff_trasnceiver_codes_byte5 *)
 + page_a0[SSF_TRANSCEIVER_CODE_B5];
 +
 + if ((trasn_code_byte4-fc_sw_laser) ||
 + (trasn_code_byte5-fc_sw_laser_sl) ||
 + (trasn_code_byte5-fc_sw_laser_sn)) {  /* check if its short WL */
 + flag |= (SFP_FLAG_PT_SWLASER  SFP_FLAG_PT_SHIFT);
 + } else if (trasn_code_byte4-fc_lw_laser) {
 + wavelength = (page_a0[SSF_WAVELENGTH_B1]  8) |
 + page_a0[SSF_WAVELENGTH_B0];
 + if (wavelength == SFP_WAVELENGTH_LC1310)
 + flag |= SFP_FLAG_PT_LWLASER_LC1310  SFP_FLAG_PT_SHIFT;
 + if (wavelength == SFP_WAVELENGTH_LL1550)
 + flag |= SFP_FLAG_PT_LWLASER_LL1550  SFP_FLAG_PT_SHIFT;
 + }
 + /* check if its SFP+ */
 + flag |= ((page_a0[SSF_IDENTIFIER] == SFF_PG0_IDENT_SFP) ?
 + SFP_FLAG_CT_SFP_PLUS : SFP_FLAG_CT_UNKNOWN)
 +  SFP_FLAG_CT_SHIFT;
 +
 + /* check if its OPTICAL */
 + flag |= ((page_a0[SSF_CONNECTOR] == SFF_PG0_CONNECTOR_LC) ?
 + SFP_FLAG_IS_OPTICAL_PORT : 0)
 +  SFP_FLAG_IS_OPTICAL_SHIFT;
 +
 + temperature = (page_a2[SFF_TEMPERATURE_B1]  8 |
 + page_a2[SFF_TEMPERATURE_B0]);
 + vcc = (page_a2[SFF_VCC_B1]  8 |
 + page_a2[SFF_VCC_B0]);
 + tx_power = (page_a2[SFF_TXPOWER_B1]  8 |
 + page_a2[SFF_TXPOWER_B0]);
 + tx_bias = (page_a2[SFF_TX_BIAS_CURRENT_B1]  8 |
 + page_a2[SFF_TX_BIAS_CURRENT_B0]);
 + rx_power = (page_a2[SFF_RXPOWER_B1]  8 |
 + page_a2[SFF_RXPOWER_B0]);
 + desc-sfp_info.temperature = cpu_to_be16(temperature);
 + desc-sfp_info.rx_power = cpu_to_be16(rx_power);
 + desc-sfp_info.tx_bias = cpu_to_be16(tx_bias);
 + desc-sfp_info.tx_power = cpu_to_be16(tx_power);
 + desc-sfp_info.vcc = cpu_to_be16(vcc);
 +
 + desc-sfp_info.flags = cpu_to_be16(flag);
 + desc-length = cpu_to_be32(sizeof(desc-sfp_info));
 +}
 +
 +void
 +lpfc_rdp_res_link_error(struct fc_rdp_link_error_status_desc *desc,
 + READ_LNK_VAR *stat)
 +{
 + uint32_t type;
 +

Re: [PATCH v3 10/20] lpfc: Fix provide host name and OS name in RSNN-NN FC-GS command

[Sebastian Herbszt]

James Smart wrote:
 Fixes those things that parse the result of what data is registered with 
 the fabric.
 
 -- james

Any particular thing you got on your mind?
qla2xxx does not provide this information (yet).

Sebastian
--
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 01/14] lpfc: The lpfc driver does not issue RFF_ID and RFT_ID in the correct sequence

[Sebastian Herbszt]

James Smart wrote:
 
 The lpfc driver does not issue RFF_ID and RFT_ID in the correct sequence
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc_hbadisc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c 
 b/drivers/scsi/lpfc/lpfc_hbadisc.c
 index 2500f15..f2db89f 100644
 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c
 +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
 @@ -3868,11 +3868,11 @@ out:
  
   if (vport-port_state  LPFC_VPORT_READY) {
   /* Link up discovery requires Fabric registration. */
 - lpfc_ns_cmd(vport, SLI_CTNS_RFF_ID, 0, 0); /* Do this first! */
   lpfc_ns_cmd(vport, SLI_CTNS_RNN_ID, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RSNN_NN, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RSPN_ID, 0, 0);
   lpfc_ns_cmd(vport, SLI_CTNS_RFT_ID, 0, 0);
 + lpfc_ns_cmd(vport, SLI_CTNS_RFF_ID, 0, 0);
  
   /* Issue SCR just before NameServer GID_FT Query */
   lpfc_issue_els_scr(vport, SCR_DID, 0);

Can you please elaborate on the correct command order?
SLI_CTNS_RFF_ID was added last in 2fb9bd8 and moved to the top in
92d7f7b with the comment Do this first!. Now it's moved back.

The libfc code suggests this is correct because it uses the same order.
qla2xxx on the other hand uses RFT_ID, RFF_ID, RNN_ID, RSNN_NN, but in
any case RFF_ID follows RFT_ID.

Sebastian
--
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 v3 18/20] lpfc: Fix for lun discovery issue with saturn adapter.

[Sebastian Herbszt]

James Smart wrote:
 
 Fix for lun discovery issue with saturn adapter.
 
 FCP_CMD payload was not always properly initialized on SLI-3 devices.
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc_scsi.c | 41 -
  1 file changed, 20 insertions(+), 21 deletions(-)

This patch actually introduced a lun discovery issue here:

[11046.353855] Emulex LightPulse Fibre Channel SCSI driver 10.5.0.0.
[11046.353890] Copyright(c) 2004-2015 Emulex.  All rights reserved.
[11046.363664] scsi host56: Emulex LPe12000 PCIe Fibre Channel Adapter on PCI 
bus 01 device 00 irq 16
[11048.737880] scsi host57: Emulex LPe12000 PCIe Fibre Channel Adapter on PCI 
bus 01 device 01 irq 17
[11049.516065] [0]: q2t_async_event:5689:qla2x00t(38): LIP reset occurred
[11049.518107] [0]: q2t_handle_imm_notify:5055:qla2x00t(38): LIP reset (I 0x80)
[11049.535184] [0]: q2t_async_event:5679:qla2x00t(38): LIP occurred
[11049.536043] lpfc :01:00.0: 0:1303 Link Up Event x1 received Data: x1 x1 
x8 x2 x0 x0 0
[11049.536063] lpfc :01:00.0: 0:1309 Link Up Event npiv not supported in 
loop topology
[11049.537132] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 TMO:x0
[11049.538056] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 TMO:x0
[11049.538948] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 TMO:x0
[11049.538965] lpfc :01:00.0: 0:(0):0100 FLOGI failure Status:x3/x18 TMO:x0
[11049.539119] [0]: q2t_handle_imm_notify:5096:qla2x00t(38): Port logout (S 
 - L 0x0)
[11049.539136] [0]: q2t_reset:933:sess for loop_id 0x7d not found
[11049.541847] [0]: q2t_async_event:5671:qla2x00t(38): Loop up occurred
[11049.541886] qla2xxx [:0b:05.0]-505f:38: Link is operational (2 Gbps).
[11049.541921] [0]: q2t_async_event:5695:qla2x00t(38): Port update async event 
0x8014 occurred
[11049.541961] [0]: q2t_clear_tgt_db:1057:qla2x00t: Clearing targets DB for 
target f523de50
[11049.541991] [0]: q2t_clear_tgt_db:1080:Finished clearing tgt f523de50 DB
[11049.542058] [0]: q2t_async_event:5695:qla2x00t(38): Port update async event 
0x8014 occurred
[11049.542063] [0]: q2t_clear_tgt_db:1057:qla2x00t: Clearing targets DB for 
target f523de50
[11049.542066] [0]: q2t_clear_tgt_db:1080:Finished clearing tgt f523de50 DB
[11049.542711] [0]: q2t_send_cmd_to_scst:4125:qla2x00t(38): Unable to find wwn 
login (loop_id=125), trying to create it manually
[11049.542749] [0]: q2t_sched_sess_work:1761:Scheduling work (type 0, prm 
f56a7500) to find session for param f602fe70 (size 4, tgt f523de50)
[11049.543886] [191]: q2t_sess_work_fn:6010:Sess work (tgt f523de50)
[11049.543922] [191]: q2t_exec_sess_work:5829:prm f56a7500
[11049.543953] [191]: q2t_make_local_sess:5784:loop_id 125
[11049.544370] [191]: q2t_create_sess:1417:Double sess f61f4568 found (s_id 
0:0:1, loop_id 125), updating to d_id 0:0:1, loop_id 125
[11049.544406] [191]: q2t_undelete_sess:1313:Undeleting sess f61f4568
[11049.544437] [191]: q2t_exec_sess_work:5910:Sending work cmd e7b83200 to SCST
[11049.544500] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
doesn't match decoded value 0
[11049.544555] [22539]: scst_parse_cmd:834:Suspicious CDB:
[11049.544582]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[11049.544611]0: 00 00 00 00 00 00   ..
[11049.552091] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
doesn't match decoded value 0
[11049.552122] [22539]: scst_parse_cmd:834:Suspicious CDB:
[11049.552136]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[11049.552152]0: 00 00 00 00 00 00   ..
[11049.560084] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
doesn't match decoded value 0
[11049.560115] [22539]: scst_parse_cmd:834:Suspicious CDB:
[11049.560129]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[11049.560144]0: 00 00 00 00 00 00   ..
[11049.568083] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
doesn't match decoded value 0
[11049.568113] [22539]: scst_parse_cmd:834:Suspicious CDB:
[11049.568127]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[11049.568143]0: 00 00 00 00 00 00   ..
[11049.568520] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
doesn't match decoded value 0
[11049.568551] [22539]: scst_parse_cmd:834:Suspicious CDB:
[11049.568565]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
[11049.568580]0: 00 00 00 00 00 00   ..
[11049.576090] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
length 36 for 

Re: [PATCH v3 18/20] lpfc: Fix for lun discovery issue with saturn adapter.

[Sebastian Herbszt]

I wrote:
 James Smart wrote:
  
  Fix for lun discovery issue with saturn adapter.
  
  FCP_CMD payload was not always properly initialized on SLI-3 devices.
  
  Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
  Signed-off-by: James Smart james.sm...@emulex.com
  ---
   drivers/scsi/lpfc/lpfc_scsi.c | 41 
  -
   1 file changed, 20 insertions(+), 21 deletions(-)
 
 This patch actually introduced a lun discovery issue here:
 
 [11046.353855] Emulex LightPulse Fibre Channel SCSI driver 10.5.0.0.
 [11046.353890] Copyright(c) 2004-2015 Emulex.  All rights reserved.
 [11046.363664] scsi host56: Emulex LPe12000 PCIe Fibre Channel Adapter on PCI 
 bus 01 device 00 irq 16
 [11048.737880] scsi host57: Emulex LPe12000 PCIe Fibre Channel Adapter on PCI 
 bus 01 device 01 irq 17
 [11049.516065] [0]: q2t_async_event:5689:qla2x00t(38): LIP reset occurred
 [11049.518107] [0]: q2t_handle_imm_notify:5055:qla2x00t(38): LIP reset (I 
 0x80)
 [11049.535184] [0]: q2t_async_event:5679:qla2x00t(38): LIP occurred
 [11049.536043] lpfc :01:00.0: 0:1303 Link Up Event x1 received Data: x1 
 x1 x8 x2 x0 x0 0
 [11049.536063] lpfc :01:00.0: 0:1309 Link Up Event npiv not supported in 
 loop topology
 [11049.537132] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 
 TMO:x0
 [11049.538056] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 
 TMO:x0
 [11049.538948] lpfc :01:00.0: 0:(0):2858 FLOGI failure Status:x3/x18 
 TMO:x0
 [11049.538965] lpfc :01:00.0: 0:(0):0100 FLOGI failure Status:x3/x18 
 TMO:x0
 [11049.539119] [0]: q2t_handle_imm_notify:5096:qla2x00t(38): Port logout (S 
  - L 0x0)
 [11049.539136] [0]: q2t_reset:933:sess for loop_id 0x7d not found
 [11049.541847] [0]: q2t_async_event:5671:qla2x00t(38): Loop up occurred
 [11049.541886] qla2xxx [:0b:05.0]-505f:38: Link is operational (2 Gbps).
 [11049.541921] [0]: q2t_async_event:5695:qla2x00t(38): Port update async 
 event 0x8014 occurred
 [11049.541961] [0]: q2t_clear_tgt_db:1057:qla2x00t: Clearing targets DB for 
 target f523de50
 [11049.541991] [0]: q2t_clear_tgt_db:1080:Finished clearing tgt f523de50 DB
 [11049.542058] [0]: q2t_async_event:5695:qla2x00t(38): Port update async 
 event 0x8014 occurred
 [11049.542063] [0]: q2t_clear_tgt_db:1057:qla2x00t: Clearing targets DB for 
 target f523de50
 [11049.542066] [0]: q2t_clear_tgt_db:1080:Finished clearing tgt f523de50 DB
 [11049.542711] [0]: q2t_send_cmd_to_scst:4125:qla2x00t(38): Unable to find 
 wwn login (loop_id=125), trying to create it manually
 [11049.542749] [0]: q2t_sched_sess_work:1761:Scheduling work (type 0, prm 
 f56a7500) to find session for param f602fe70 (size 4, tgt f523de50)
 [11049.543886] [191]: q2t_sess_work_fn:6010:Sess work (tgt f523de50)
 [11049.543922] [191]: q2t_exec_sess_work:5829:prm f56a7500
 [11049.543953] [191]: q2t_make_local_sess:5784:loop_id 125
 [11049.544370] [191]: q2t_create_sess:1417:Double sess f61f4568 found (s_id 
 0:0:1, loop_id 125), updating to d_id 0:0:1, loop_id 125
 [11049.544406] [191]: q2t_undelete_sess:1313:Undeleting sess f61f4568
 [11049.544437] [191]: q2t_exec_sess_work:5910:Sending work cmd e7b83200 to 
 SCST
 [11049.544500] [22539]: scst: scst_parse_cmd:832:Warning: expected transfer 
 length 36 for opcode TEST UNIT READY (handler vdisk_nullio, target qla2x00t) 
 doesn't match decoded value 0
 [11049.544555] [22539]: scst_parse_cmd:834:Suspicious CDB:
 [11049.544582]  (h)___0__1__2__3__4__5__6__7__8__9__A__B__C__D__E__F
 [11049.544611]0: 00 00 00 00 00 00   ..

The call to lpfc_fcpcmd_to_iocb() was moved from lpfc_scsi_prep_cmnd() to
lpfc_scsi_prep_dma_buf_s3(). It seems fcp_cmnd is not fully initialized
inside lpfc_scsi_prep_dma_buf_s3(); only fcpDl is set and other parts still
contain zeros. Therefore the call to lpfc_fcpcmd_to_iocb() seems premature.

Sebastian
--
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 v3 10/20] lpfc: Fix provide host name and OS name in RSNN-NN FC-GS command

[Sebastian Herbszt]

James Smart wrote:
 
 Fix provide host name and OS name in RSNN-NN FC-GS command
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc_ct.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
 index 61a32cd..5fb431a 100644
 --- a/drivers/scsi/lpfc/lpfc_ct.c
 +++ b/drivers/scsi/lpfc/lpfc_ct.c
 @@ -1074,8 +1074,25 @@ lpfc_vport_symbolic_node_name(struct lpfc_vport 
 *vport, char *symbol,
  
   lpfc_decode_firmware_rev(vport-phba, fwrev, 0);
  
 - n = snprintf(symbol, size, Emulex %s FV%s DV%s,
 - vport-phba-ModelName, fwrev, lpfc_release_version);
 + n = snprintf(symbol, size, Emulex %s, vport-phba-ModelName);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  FV%s, fwrev);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  DV%s, lpfc_release_version);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  HN:%s, init_utsname()-nodename);
 +
 + /* Note :- OS name is Linux */
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  OS:%s, init_utsname()-sysname);
 +
   return n;
  }

Does this actually fix anything? If not maybe drop fix from the
patch subject and description.

Sebastian
--
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

lpfc: Unknown ELS command x18 received from NPORT xfffc2b

[Sebastian Herbszt]

We recently started to get

[8043768.238110] lpfc :10:00.1: 1:(0):0115 Unknown ELS command x18 received 
from NPORT xfffc2b
[8130462.560283] lpfc :10:00.1: 1:(0):0115 Unknown ELS command x18 received 
from NPORT xfffc2b
[8216439.442882] lpfc :10:00.1: 1:(0):0115 Unknown ELS command x18 received 
from NPORT xfffc2b

Read Diagnostic Parameters (RDP) [1] support was introduced in Brocade Fabric 
OS 7.3 [2].

The following (untested) patch might silence the message.

[1] http://www.t11.org/ftp/t11/pub/fc/ls-3/14-033v0.pdf
[2] ftp://public.dhe.ibm.com/storage/san/fos7/v7.3.0c_ReleaseNotes_v1.0.pdf

Sebastian


diff -uNrp v4.0-1941-g78c/drivers/scsi/lpfc.orig/lpfc_els.c 
v4.0-1941-g78c/drivers/scsi/lpfc/lpfc_els.c
--- v4.0-1941-g78c/drivers/scsi/lpfc.orig/lpfc_els.c2015-02-12 
01:04:08.692134956 +0100
+++ v4.0-1941-g78c/drivers/scsi/lpfc/lpfc_els.c 2015-04-14 
21:44:31.591639045 +0200
@@ -6970,6 +6970,10 @@ lpfc_els_unsol_buffer(struct lpfc_hba *p
rjt_err = LSRJT_UNABLE_TPC;
rjt_exp = LSEXP_INVALID_OX_RX;
break;
+   case ELS_CMD_RDP:
+   rjt_err = LSRJT_UNABLE_TPC;
+   rjt_exp = LSEXP_REQ_UNSUPPORTED;
+   break;
default:
lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_UNSOL,
RCV ELS cmd: cmd:x%x did:x%x/ste:x%x,
diff -uNrp v4.0-1941-g78c/drivers/scsi/lpfc.orig/lpfc_hw.h 
v4.0-1941-g78c/drivers/scsi/lpfc/lpfc_hw.h
--- v4.0-1941-g78c/drivers/scsi/lpfc.orig/lpfc_hw.h 2014-09-03 
19:30:11.768273300 +0200
+++ v4.0-1941-g78c/drivers/scsi/lpfc/lpfc_hw.h  2015-04-14 
21:18:10.039566920 +0200
@@ -540,6 +540,7 @@ struct fc_vft_header {
 #define ELS_CMD_TEST  0x1100
 #define ELS_CMD_RRQ   0x1200
 #define ELS_CMD_REC   0x1300
+#define ELS_CMD_RDP   0x1800
 #define ELS_CMD_PRLI  0x20100014
 #define ELS_CMD_PRLO  0x21100014
 #define ELS_CMD_PRLO_ACC  0x02100014
@@ -577,6 +578,7 @@ struct fc_vft_header {
 #define ELS_CMD_TEST  0x11
 #define ELS_CMD_RRQ   0x12
 #define ELS_CMD_REC   0x13
+#define ELS_CMD_RDP   0x18
 #define ELS_CMD_PRLI  0x14001020
 #define ELS_CMD_PRLO  0x14001021
 #define ELS_CMD_PRLO_ACC  0x14001002
--
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 10/21] lpfc: Fix provide host name and OS name in RSNN-NN FC-GS command

[Sebastian Herbszt]

James Smart wrote:
 Fix provide host name and OS name in RSNN-NN FC-GS command
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc_ct.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_ct.c b/drivers/scsi/lpfc/lpfc_ct.c
 index 61a32cd..c259eb0 100644
 --- a/drivers/scsi/lpfc/lpfc_ct.c
 +++ b/drivers/scsi/lpfc/lpfc_ct.c
 @@ -1074,8 +1074,25 @@ lpfc_vport_symbolic_node_name(struct lpfc_vport 
 *vport, char *symbol,
  
   lpfc_decode_firmware_rev(vport-phba, fwrev, 0);
  
 - n = snprintf(symbol, size, Emulex %s FV%s DV%s,
 - vport-phba-ModelName, fwrev, lpfc_release_version);
 + n = snprintf(symbol, size, Emulex %s, vport-phba-ModelName);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  FV%s, fwrev);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  DV%s, lpfc_release_version);
 +
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  HN:%s, init_utsname()-nodename);
 +
 + /* Note :- OS name is Linux as per requirement BZ168199. */
 + if (size  n)
 + return n;
 + n += snprintf(symbol + n, size - n,  OS:%s, init_utsname()-sysname);
 +
   return n;
  }

Care to disclose which bugzilla contains BZ168199?

Sebastian
--
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 01/21] lpfc: Fix the iteration count to match the 30 sec comment in the routine lpfc_pci_function_reset

[Sebastian Herbszt]

James Smart wrote:
 Fix the iteration count to match the 30 sec comment in the routine 
 lpfc_pci_function_reset
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc_init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
 index 0b2c53a..2b5b910 100644
 --- a/drivers/scsi/lpfc/lpfc_init.c
 +++ b/drivers/scsi/lpfc/lpfc_init.c
 @@ -7953,7 +7953,7 @@ wait:
* up to 30 seconds. If the port doesn't respond, treat
* it as an error.
*/
 - for (rdy_chk = 0; rdy_chk  3000; rdy_chk++) {
 + for (rdy_chk = 0; rdy_chk  1500; rdy_chk++) {
   if (lpfc_readl(phba-sli4_hba.u.if_type2.
   STATUSregaddr, reg_data.word0)) {
   rc = -ENODEV;

Reviewed-by: Sebastian Herbszt herb...@gmx.de
--
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 15/21] lpfc: Implement support for wire-only DIF devices

[Sebastian Herbszt]

James Smart wrote:
 Implement support for wire-only DIF devices
 
 This patch adds the ability to support auto-enablement of wire-only DIF
 (DIF is generated on TX to target, stripped on RX; OS unaware DIF in use)
 for a select set of devices. Currently, there is only 1 device vendor
 supported: 3PARdata. When the feature is enabled, Inquiry commands are
 trapped, the vendor matched, and DIF enablement checked. In 3Par's case,
 there's a vendor specific check to see if the LUN supports DIF.
 If supported, DIF will be enabled on a per-lun basis.  The driver will
 trap READS/WRITEs from the OS, check for LUN DIF enablement, and if set,
 turns on write-only DIF.
 
 Signed-off-by: Dick Kennedy dick.kenn...@emulex.com
 Signed-off-by: James Smart james.sm...@emulex.com
 ---
  drivers/scsi/lpfc/lpfc.h |  41 
  drivers/scsi/lpfc/lpfc_attr.c|  65 ++-
  drivers/scsi/lpfc/lpfc_crtn.h|   3 +-
  drivers/scsi/lpfc/lpfc_hbadisc.c |   3 +
  drivers/scsi/lpfc/lpfc_init.c|  24 ++-
  drivers/scsi/lpfc/lpfc_mbox.c|   4 +-
  drivers/scsi/lpfc/lpfc_scsi.c| 410 
 ---
  drivers/scsi/lpfc/lpfc_scsi.h|   7 +-
  drivers/scsi/lpfc/lpfc_sli.c |  36 ++--
  9 files changed, 532 insertions(+), 61 deletions(-)

snip

 diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
 index faf0e8c..891e2d1 100644
 --- a/drivers/scsi/lpfc/lpfc_attr.c
 +++ b/drivers/scsi/lpfc/lpfc_attr.c
 @@ -135,16 +135,29 @@ lpfc_bg_info_show(struct device *dev, struct 
 device_attribute *attr,
   struct Scsi_Host *shost = class_to_shost(dev);
   struct lpfc_vport *vport = (struct lpfc_vport *) shost-hostdata;
   struct lpfc_hba   *phba = vport-phba;
 + int len = 0;
  
 - if (phba-cfg_enable_bg)
 + if (phba-cfg_enable_bg) {
   if (phba-sli3_options  LPFC_SLI3_BG_ENABLED)
 - return snprintf(buf, PAGE_SIZE, BlockGuard Enabled\n);
 + len +=  snprintf(buf, PAGE_SIZE,
 +  BlockGuard Enabled\n);
   else
 - return snprintf(buf, PAGE_SIZE,
 + len +=  snprintf(buf, PAGE_SIZE,
   BlockGuard Not Supported\n);
 - else
 - return snprintf(buf, PAGE_SIZE,
 + } else {
 + len +=  snprintf(buf, PAGE_SIZE,
   BlockGuard Disabled\n);
 + }
 +
 + if (phba-cfg_external_dif) {
 + if (phba-sli3_options  LPFC_SLI3_BG_ENABLED)
 + len += snprintf(buf + len, PAGE_SIZE,
 + External DIF Enabled\n);
 + else
 + len += snprintf(buf + len, PAGE_SIZE,
 + External DIF Not Supported\n);
 + }
 + return len;
  }
  
  static ssize_t
 @@ -4681,6 +4694,30 @@ LPFC_ATTR_R(EnableXLane, 0, 0, 1, Enable Express Lane 
 Feature.);
  */
  LPFC_ATTR_RW(XLanePriority, 0, 0x0, 0x7f, CS_CTL for Express Lane 
 Feature.);
  
 +
 +/*
 + * For T10 DIF / protection data support, the driver supports 4 modes
 + * of operation.
 + *
 + * Mode 1: (lpfc_enable_bg=1 lpfc_external_dif=1)
 + * All normal T10 DIF devices are supported.
 + * External DIF devices are supported.
 + *
 + * Mode 2: (lpfc_enable_bg=0 lpfc_external_dif=1)
 + * If you don't want to have the extra overhead of the upper SCSI Layer
 + * supporting T10-DIF, but you still want to support External DIF devices.
 + * Normal T10 DIF devices are NOT supported.
 + * External DIF devices are supported.
 + *
 + * Mode 3: (lpfc_enable_bg=1 lpfc_external_dif=0)
 + * All normal T10 DIF devices are supported.
 + * External DIF devices are NOT supported.
 + *
 + * Mode 4: (lpfc_enable_bg=0 lpfc_external_dif=1)
 + * No normal T10-DIF and no external DIF devices supported,
 + * This would be the driver default values for these module parameters.
 + */

Not lpfc_external_dif=0?

  /*
  # lpfc_enable_bg: Enable BlockGuard (Emulex's Implementation of T10-DIF)
  #   0  = BlockGuard disabled (default)
 @@ -4690,6 +4727,15 @@ LPFC_ATTR_RW(XLanePriority, 0, 0x0, 0x7f, CS_CTL for 
 Express Lane Feature.);
  LPFC_ATTR_R(enable_bg, 0, 0, 1, Enable BlockGuard Support);
  
  /*
 +# lpfc_external_dif: Enable External DIF support on select devices
 +#   0  = External DIF disabled (default)
 +#   1  = External DIF enabled
 +# Value range is [0,1]. Default value is 0.
 +*/
 +LPFC_ATTR_R(external_dif, 0, 0, 1,
 + External T10-DIF Support, on select devices);
 +
 +/*
  # lpfc_fcp_look_ahead: Look ahead for completions in FCP start routine
  #   0  = disabled (default)
  #   1  = enabled

Sebastian
--
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] scsi_host: fix comment

[Sebastian Herbszt]

Commit 1abf635 (scsi: use 64-bit value for 'max_luns') changed the order
of Scsi_Host members. Update the comment to reflect this.

Signed-off-by: Sebastian Herbszt herb...@gmx.de

diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index ba20347..d4e0509 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -603,13 +603,13 @@ struct Scsi_Host {
unsigned long last_reset;
 
 
/*
 * These three parameters can be used to allow for wide scsi,
 * and for host adapters that support multiple busses
-* The first two should be set to 1 more than the actual max id
+* The last two should be set to 1 more than the actual max id
 * or lun (e.g. 8 for SCSI parallel systems).
 */
unsigned int max_channel;
unsigned int max_id;
u64 max_lun;
 
--
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