30 May, 2019

2 commits

  • When the user tries to remove a zfcp port via sysfs, we only rejected it if
    there are zfcp unit children under the port. With purely automatically
    scanned LUNs there are no zfcp units but only SCSI devices. In such cases,
    the port_remove erroneously continued. We close the port and this
    implicitly closes all LUNs under the port. The SCSI devices survive with
    their private zfcp_scsi_dev still holding a reference to the "removed"
    zfcp_port (still allocated but invisible in sysfs) [zfcp_get_port_by_wwpn
    in zfcp_scsi_slave_alloc]. This is not a problem as long as the fc_rport
    stays blocked. Once (auto) port scan brings back the removed port, we
    unblock its fc_rport again by design. However, there is no mechanism that
    would recover (open) the LUNs under the port (no "ersfs_3" without
    zfcp_unit [zfcp_erp_strategy_followup_success]). Any pending or new I/O to
    such LUN leads to repeated:

    Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK

    See also v4.10 commit 6f2ce1c6af37 ("scsi: zfcp: fix rport unblock race
    with LUN recovery"). Even a manual LUN recovery
    (echo 0 > /sys/bus/scsi/devices/H:C:T:L/zfcp_failed)
    does not help, as the LUN links to the old "removed" port which remains
    to lack ZFCP_STATUS_COMMON_RUNNING [zfcp_erp_required_act].
    The only workaround is to first ensure that the fc_rport is blocked
    (e.g. port_remove again in case it was re-discovered by (auto) port scan),
    then delete the SCSI devices, and finally re-discover by (auto) port scan.
    The port scan includes an fc_rport unblock, which in turn triggers
    a new scan on the scsi target to freshly get new pure auto scan LUNs.

    Fix this by rejecting port_remove also if there are SCSI devices
    (even without any zfcp_unit) under this port. Re-use mechanics from v3.7
    commit d99b601b6338 ("[SCSI] zfcp: restore refcount check on port_remove").
    However, we have to give up zfcp_sysfs_port_units_mutex earlier in unit_add
    to prevent a deadlock with scsi_host scan taking shost->scan_mutex first
    and then zfcp_sysfs_port_units_mutex now in our zfcp_scsi_slave_alloc().

    Signed-off-by: Steffen Maier
    Fixes: b62a8d9b45b9 ("[SCSI] zfcp: Use SCSI device data zfcp scsi dev instead of zfcp unit")
    Fixes: f8210e34887e ("[SCSI] zfcp: Allow midlayer to scan for LUNs when running in NPIV mode")
    Cc: #2.6.37+
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • With this early return due to zfcp_unit child(ren), we don't use the
    zfcp_port reference from the earlier zfcp_get_port_by_wwpn() anymore and
    need to put it.

    Signed-off-by: Steffen Maier
    Fixes: d99b601b6338 ("[SCSI] zfcp: restore refcount check on port_remove")
    Cc: #3.7+
    Reviewed-by: Jens Remus
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     

28 Mar, 2019

3 commits

  • If an incoming ELS of type RSCN contains more than one element, zfcp
    suboptimally causes repeated erp trigger NOP trace records for each
    previously failed port. These could be ports that went away. It loops over
    each RSCN element, and for each of those in an inner loop over all
    zfcp_ports.

    The trigger to recover failed ports should be just the reception of some
    RSCN, no matter how many elements it has. So we can loop over failed ports
    separately, and only then loop over each RSCN element to handle the
    non-failed ports.

    The call chain was:

    zfcp_fc_incoming_rscn
    for (i = 1; i < no_entries; i++)
    _zfcp_fc_incoming_rscn
    list_for_each_entry(port, &adapter->port_list, list)
    if (masked port->d_id match) zfcp_fc_test_link
    if (!port->d_id) zfcp_erp_port_reopen "fcrscn1" 1) port_list, list) d_id) zfcp_erp_port_reopen "fcrscn1" < no_entries; i++)
    _zfcp_fc_incoming_rscn
    list_for_each_entry(port, &adapter->port_list, list)
    if (masked port->d_id match) zfcp_fc_test_link

    Abbreviated example trace records before this code change:

    Tag : fcrscn1
    WWPN : 0x500507630310d327
    ERP want : 0x02
    ERP need : 0x02

    Tag : fcrscn1
    WWPN : 0x500507630310d327
    ERP want : 0x02
    ERP need : 0x00 NOP => superfluous trace record

    The last trace entry repeats if there are more than 2 RSCN elements.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Reviewed-by: Jens Remus
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Suppose more than one non-NPIV FCP device is active on the same channel.
    Send I/O to storage and have some of the pending I/O run into a SCSI
    command timeout, e.g. due to bit errors on the fibre. Now the error
    situation stops. However, we saw FCP requests continue to timeout in the
    channel. The abort will be successful, but the subsequent TUR fails.
    Scsi_eh starts. The LUN reset fails. The target reset fails. The host
    reset only did an FCP device recovery. However, for non-NPIV FCP devices,
    this does not close and reopen ports on the SAN-side if other non-NPIV FCP
    device(s) share the same open ports.

    In order to resolve the continuing FCP request timeouts, we need to
    explicitly close and reopen ports on the SAN-side.

    This was missing since the beginning of zfcp in v2.6.0 history commit
    ea127f975424 ("[PATCH] s390 (7/7): zfcp host adapter.").

    Note: The FSF requests for forced port reopen could run into FSF request
    timeouts due to other reasons. This would trigger an internal FCP device
    recovery. Pending forced port reopen recoveries would get dismissed. So
    some ports might not get fully reopened during this host reset handler.
    However, subsequent I/O would trigger the above described escalation and
    eventually all ports would be forced reopen to resolve any continuing FCP
    request timeouts due to earlier bit errors.

    Signed-off-by: Steffen Maier
    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    Cc: #3.0+
    Reviewed-by: Jens Remus
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • An already deleted SCSI device can exist on the Scsi_Host and remain there
    because something still holds a reference. A new SCSI device with the same
    H:C:T:L and FCP device, target port WWPN, and FCP LUN can be created. When
    we try to unblock an rport, we still find the deleted SCSI device and
    return early because the zfcp_scsi_dev of that SCSI device is not
    ZFCP_STATUS_COMMON_UNBLOCKED. Hence we miss to unblock the rport, even if
    the new proper SCSI device would be in good state.

    Therefore, skip deleted SCSI devices when iterating the sdevs of the shost.
    [cf. __scsi_device_lookup{_by_target}() or scsi_device_get()]

    The following abbreviated trace sequence can indicate such problem:

    Area : REC
    Tag : ersfs_3
    LUN : 0x4045400300000000
    WWPN : 0x50050763031bd327
    LUN status : 0x40000000 not ZFCP_STATUS_COMMON_UNBLOCKED
    Ready count : n not incremented yet
    Running count : 0x00000000
    ERP want : 0x01
    ERP need : 0xc1 ZFCP_ERP_ACTION_NONE

    Area : REC
    Tag : ersfs_3
    LUN : 0x4045400300000000
    WWPN : 0x50050763031bd327
    LUN status : 0x41000000
    Ready count : n+1
    Running count : 0x00000000
    ERP want : 0x01
    ERP need : 0x01

    ...

    Area : REC
    Level : 4 only with increased trace level
    Tag : ertru_l
    LUN : 0x4045400300000000
    WWPN : 0x50050763031bd327
    LUN status : 0x40000000
    Request ID : 0x0000000000000000
    ERP status : 0x01800000
    ERP step : 0x1000
    ERP action : 0x01
    ERP count : 0x00

    NOT followed by a trace record with tag "scpaddy"
    for WWPN 0x50050763031bd327.

    Signed-off-by: Steffen Maier
    Fixes: 6f2ce1c6af37 ("scsi: zfcp: fix rport unblock race with LUN recovery")
    Cc: #2.6.32+
    Reviewed-by: Jens Remus
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     

06 Mar, 2019

1 commit

  • Pull s390 updates from Martin Schwidefsky:

    - A copy of Arnds compat wrapper generation series

    - Pass information about the KVM guest to the host in form the control
    program code and the control program version code

    - Map IOV resources to support PCI physical functions on s390

    - Add vector load and store alignment hints to improve performance

    - Use the "jdd" constraint with gcc 9 to make jump labels working again

    - Remove amode workaround for old z/VM releases from the DCSS code

    - Add support for in-kernel performance measurements using the CPU
    measurement counter facility

    - Introduce a new PMU device cpum_cf_diag to capture counters and store
    thenn as event raw data.

    - Bug fixes and cleanups

    * tag 's390-5.1-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux: (54 commits)
    Revert "s390/cpum_cf: Add kernel message exaplanations"
    s390/dasd: fix read device characteristic with CONFIG_VMAP_STACK=y
    s390/suspend: fix prefix register reset in swsusp_arch_resume
    s390: warn about clearing als implied facilities
    s390: allow overriding facilities via command line
    s390: clean up redundant facilities list setup
    s390/als: remove duplicated in-place implementation of stfle
    s390/cio: Use cpa range elsewhere within vfio-ccw
    s390/cio: Fix vfio-ccw handling of recursive TICs
    s390: vfio_ap: link the vfio_ap devices to the vfio_ap bus subsystem
    s390/cpum_cf: Handle EBUSY return code from CPU counter facility reservation
    s390/cpum_cf: Add kernel message exaplanations
    s390/cpum_cf_diag: Add support for s390 counter facility diagnostic trace
    s390/cpum_cf: add ctr_stcctm() function
    s390/cpum_cf: move common functions into a separate file
    s390/cpum_cf: introduce kernel_cpumcf_avail() function
    s390/cpu_mf: replace stcctm5() with the stcctm() function
    s390/cpu_mf: add store cpu counter multiple instruction support
    s390/cpum_cf: Add minimal in-kernel interface for counter measurements
    s390/cpum_cf: introduce kernel_cpumcf_alert() to obtain measurement alerts
    ...

    Linus Torvalds
     

07 Feb, 2019

1 commit


29 Jan, 2019

1 commit

  • Since v2.6.35 commit 683229845f17 ("[SCSI] zfcp: Report scatter-gather
    limits to SCSI and block layer"), zfcp set dma_parms.max_segment_size ==
    PAGE_SIZE (but without using the setter dma_set_max_seg_size()) and
    scsi_host_template.dma_boundary == PAGE_SIZE - 1.

    v5.0-rc1 commit 50c2e9107f17 ("scsi: introduce a max_segment_size
    host_template parameters") introduced a new field
    scsi_host_template.max_segment_size. If an LLDD such as zfcp does not set
    it, scsi_host_alloc() uses BLK_MAX_SEGMENT_SIZE = 65536 for
    Scsi_Host.max_segment_size. __scsi_init_queue() announced the minimum of
    Scsi_Host.max_segment_size and dma_parms.max_segment_size to the block
    layer. For zfcp: min(65536, 4096) == 4096 which was still good.

    v5.0 commit a8cf59a6692c ("scsi: communicate max segment size to the DMA
    mapping code") announces Scsi_Host.max_segment_size to the block layer and
    overwrites dma_parms.max_segment_size with Scsi_Host.max_segment_size. For
    zfcp dma_parms.max_segment_size == Scsi_Host.max_segment_size == 65536
    which is also reflected in block queue limits.

    $ cd /sys/bus/ccw/drivers/zfcp
    $ cd 0.0.3c40/host5/rport-5:0-4/target5:0:4/5:0:4:10/block/sdi/queue
    $ cat max_segment_size
    65536

    Zfcp I/O still works because dma_boundary implicitly still keeps the
    effective max segment size
    Fixes: 50c2e9107f ("scsi: introduce a max_segment_size host_template parameters")
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     

19 Dec, 2018

1 commit

  • Most SCSI drivers want to enable "clustering", that is merging of
    segments so that they might span more than a single page. Remove the
    ENABLE_CLUSTERING define, and require drivers to explicitly set
    DISABLE_CLUSTERING to disable this feature.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Martin K. Petersen

    Christoph Hellwig
     

08 Dec, 2018

3 commits

  • Complements

    v2.6.35 commit 64deb6efdc55 ("[SCSI] zfcp: Use status_read_buf_num
    provided by FCP channel") which replaced the hardcoded 16 with a
    variable value

    Also complements already existing fixups for above commit

    v2.6.35 commit 8d88cf3f3b9a ("[SCSI] zfcp: Update status read mempool")
    v3.10 commit 9edf7d75ee5f ("[SCSI] zfcp: status read buffers on first adapter open with link down")

    Signed-off-by: Steffen Maier
    Reviewed-by: Jens Remus
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Suppose adapter (open) recovery is between opened QDIO queues and before
    (the end of) initial posting of status read buffers (SRBs). This time
    window can be seconds long due to FSF_PROT_HOST_CONNECTION_INITIALIZING
    causing by design looping with exponential increase sleeps in the function
    performing exchange config data during recovery
    [zfcp_erp_adapter_strat_fsf_xconf()]. Recovery triggered by local link up.

    Suppose an event occurs for which the FCP channel would send an unsolicited
    notification to zfcp by means of a previously posted SRB. We saw it with
    local cable pull (link down) in multi-initiator zoning with multiple
    NPIV-enabled subchannels of the same shared FCP channel.

    As soon as zfcp_erp_adapter_strategy_open_fsf() starts posting the initial
    status read buffers from within the adapter's ERP thread, the channel does
    send an unsolicited notification.

    Since v2.6.27 commit d26ab06ede83 ("[SCSI] zfcp: receiving an unsolicted
    status can lead to I/O stall"), zfcp_fsf_status_read_handler() schedules
    adapter->stat_work to re-fill the just consumed SRB from a work item.

    Now the ERP thread and the work item post SRBs in parallel. Both contexts
    call the helper function zfcp_status_read_refill(). The tracking of
    missing (to be posted / re-filled) SRBs is not thread-safe due to separate
    atomic_read() and atomic_dec(), in order to depend on posting
    success. Hence, both contexts can see
    atomic_read(&adapter->stat_miss) == 1. One of the two contexts posts
    one too many SRB. Zfcp gets QDIO_ERROR_SLSB_STATE on the output queue
    (trace tag "qdireq1") leading to zfcp_erp_adapter_shutdown() in
    zfcp_qdio_handler_error().

    An obvious and seemingly clean fix would be to schedule stat_work from the
    ERP thread and wait for it to finish. This would serialize all SRB
    re-fills. However, we already have another work item wait on the ERP
    thread: adapter->scan_work runs zfcp_fc_scan_ports() which calls
    zfcp_fc_eval_gpn_ft(). The latter calls zfcp_erp_wait() to wait for all the
    open port recoveries during zfcp auto port scan, but in fact it waits for
    any pending recovery including an adapter recovery. This approach leads to
    a deadlock. [see also v3.19 commit 18f87a67e6d6 ("zfcp: auto port scan
    resiliency"); v2.6.37 commit d3e1088d6873
    ("[SCSI] zfcp: No ERP escalation on gpn_ft eval");
    v2.6.28 commit fca55b6fb587
    ("[SCSI] zfcp: fix deadlock between wq triggered port scan and ERP")
    fixing v2.6.27 commit c57a39a45a76
    ("[SCSI] zfcp: wait until adapter is finished with ERP during auto-port");
    v2.6.27 commit cc8c282963bd
    ("[SCSI] zfcp: Automatically attach remote ports")]

    Instead make the accounting of missing SRBs atomic for parallel execution
    in both the ERP thread and adapter->stat_work.

    Signed-off-by: Steffen Maier
    Fixes: d26ab06ede83 ("[SCSI] zfcp: receiving an unsolicted status can lead to I/O stall")
    Cc: #2.6.27+
    Reviewed-by: Jens Remus
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Introduce separate zfcp module parameters to individually select support
    for: DIF which should work (zfcp.dif, which used to be DIF+DIX, disabled)
    or DIX+DIF which can cause trouble (zfcp.dix, new, disabled).

    If DIX is enabled, we warn on zfcp driver initialization. As before, this
    also reduces the maximum I/O request size to half, to support the worst
    case of merged single sector requests with one protection data scatter
    gather element per sector. This can impact the maximum throughput.

    In DIF-only mode (zfcp.dif=1 zfcp.dix=0), we can use the full maximum I/O
    request size as there is no protection data for zfcp.

    Signed-off-by: Steffen Maier
    Signed-off-by: Fedor Loshakov
    Reviewed-by: Jens Remus
    Signed-off-by: Martin K. Petersen

    Fedor Loshakov
     

16 Nov, 2018

22 commits

  • This was introduced with v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup
    code in zfcp_erp.c") but would now suppress helpful -Wswitch compiler
    warnings when building with W=1 such as the following forced example:

    drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_setup_act':
    drivers/s390/scsi/zfcp_erp.c:220:2: warning: enumeration value 'ZFCP_ERP_ACTION_REOPEN_PORT' not handled in switch [-Wswitch]
    switch (need) {
    ^~~~~~

    But then again, only with W=1 we would notice unhandled enum cases.
    Without the default cases and a missed unhandled enum case, the code might
    perform unforeseen things we might not want...

    As of today, we never run through the removed default case, so removing it
    is no functional change. In the future, we never should run through a
    default case but introduce the necessary specific case(s) to handle new
    functionality.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • This was introduced with v4.18 commit 8c3d20aada70 ("scsi: zfcp: fix
    missing REC trigger trace for all objects in ERP_FAILED") but would now
    suppress helpful -Wswitch compiler warnings when building with W=1 such as
    the following forced example:

    drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_handle_failed':
    drivers/s390/scsi/zfcp_erp.c:126:2: warning: enumeration value 'ZFCP_ERP_ACTION_REOPEN_PORT_FORCED' not handled in switch [-Wswitch]
    switch (want) {
    ^~~~~~

    But then again, only with W=1 we would notice unhandled enum cases.
    Without the default cases and a missed unhandled enum case, the code might
    perform unforeseen things we might not want...

    As of today, we never run through the removed default case, so removing it
    is no functional change. In the future, we never should run through a
    default case but introduce the necessary specific case(s) to handle new
    functionality.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • For some reason the already existing substring "fall through" in the
    comment is not sufficient for GCC to silence -Wimplicit-fallthrough.

    CC [M] drivers/s390/scsi/zfcp_erp.o
    drivers/s390/scsi/zfcp_erp.c: In function 'zfcp_erp_lun_strategy':
    drivers/s390/scsi/zfcp_erp.c:1065:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if (atomic_read(&zfcp_sdev->status) & ZFCP_STATUS_COMMON_OPEN)
    ^
    drivers/s390/scsi/zfcp_erp.c:1068:2: note: here
    case ZFCP_ERP_STEP_LUN_CLOSING:
    ^~~~

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Improve whatever the following simple invocation reported:
    $ ./scripts/kernel-doc -none drivers/s390/scsi/*.h

    While at it, improve some related kdoc,
    including struct zfcp_fsf_ct_els in zfcp_fsf.h.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • While at it also improve some copy & paste kdoc mistakes.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • zfcp: : LUN 0x0 on port 0x5005076......... ...
    zfcp: : LUN 0x1000000000000 on port 0x5005076......... ...

    should be

    zfcp: : LUN 0x0000000000000000 on port 0x5005076......... ...
    zfcp: : LUN 0x0001000000000000 on port 0x5005076.........
    is already in use by CSS., MIF Image ID .

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • With that instead of just "int" it becomes clear which functions return
    this type and which ones also accept it as argument they just pass through
    in some cases or modify in other cases. v2.6.27 commit 287ac01acf22
    ("[SCSI] zfcp: Cleanup code in zfcp_erp.c") introduced the enum which was
    cpp defines previously.

    Silence some false -Wswitch compiler warning cases with individual
    NOP cases. When adding more enum values and building with W=1 we
    would get compiler warnings about missed new cases.

    Consistently use the variable name "result", so change "retval" in
    zfcp_erp_strategy() to "result". This avoids confusion with other compile
    unit variables "retval" having different semantics and type.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Use the already defined enum for this purpose to get at least some build
    checking (even though an enum is type equivalent to an int in C). v2.6.27
    commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in zfcp_erp.c") introduced
    the enum which was cpp defines previously.

    Since struct zfcp_erp_action type is embedded into other structures living
    in zfcp_def.h, we have to move enum zfcp_erp_act_type from its private
    definition in zfcp_erp.c to the zfcp-global zfcp_def.h

    Silence some false -Wswitch compiler warning cases with individual NOP
    cases. When adding more enum values and building with W=1 we would get
    compiler warnings about missed new cases.

    Add missing break statements in some of the above switch cases. No
    functional change, but making it future-proof. I think all of these should
    have had a break statement ever since, even if these switch cases happened
    to be the last ones in the switch statement body.

    "Fall through" in the context of switch case usually means not to have a
    break and fall through to the subsequent switch case. However, I think this
    old comment meant that here we do not have an _early return_ in the switch
    case but the code path continues after the switch case body.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • &zfcp_erp_action.action ==> &zfcp_erp_action.type

    While at it, make use of the already defined enum for this purpose to get
    at least some build checking (even though an enum is type equivalent to an
    int in C). v2.6.27 commit 287ac01acf22 ("[SCSI] zfcp: Cleanup code in
    zfcp_erp.c") introduced the enum which was cpp defines previously.

    To prevent compiler warnings with the switch(act->type), we have to
    separate the recently added eyecatchers from enum zfcp_erp_act_type.

    Since struct zfcp_erp_action type is embedded into other structures living
    in zfcp_def.h, we have to move enum zfcp_erp_act_type from its private
    definition in zfcp_erp.c to the zfcp-global zfcp_def.h.

    Silence one false -Wswitch compiler warning case: LUNs as the leaves in our
    object tree do not have any follow-up success recovery.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • v2.6.30 commit 5ffd51a5e495 ("[SCSI] zfcp: replace current ERP logging with
    a more convenient version") changed trace record distinguishing from a
    numerical ID to a 7 character string called "trace tag". While starting to
    use function arguments with different type and semantics, it did not change
    the argument name accordingly.

    v2.6.38 commit ae0904f60fab ("[SCSI] zfcp: Redesign of the debug tracing
    for recovery actions.") renamed variable names "id" into "tag" but only
    within zfcp_dbf.*, not within zfcp_erp.c.

    This was a bit confusing since the remainder of zfcp does use the term
    "trace tag". Also "id" is quite generic and it's not obvious for what.
    Just unify it consistently and use the "dbf" prefix to relate the arguments
    to the code in zfcp_dbf.*.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • zfcp_erp_thread_setup() update complements v2.6.32 commit 347c6a965dc1
    ("[SCSI] zfcp: Use kthread API for zfcp erp thread").

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • The CDB is just a part inside of FCP_CMND, see zfcp_fc_scsi_to_fcp().
    While at it, fix the device driver reaction: adapter not LUN shutdown.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • There is no point for double bookkeeping especially just for tracing. The
    trace can take it from the QTCB which always exists for non-SRB responses
    traced with zfcp_dbf_hba_fsf_res().

    As a side effect, this removes an alignment hole and reduces the size of
    struct zfcp_fsf_req, and thus of each pending request, by 8 bytes.

    Before:
    $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko
    ...
    struct fsf_qtcb * qtcb; /* 144 8 */
    u32 seq_no; /* 152 4 */
    /* XXX 4 bytes hole, try to pack */
    void * data; /* 160 8 */
    ...
    /* size: 296, cachelines: 2, members: 14 */
    /* sum members: 288, holes: 2, sum holes: 8 */
    /* last cacheline: 40 bytes */
    After:
    $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko
    ...
    struct fsf_qtcb * qtcb; /* 144 8 */
    void * data; /* 152 8 */
    ...
    /* size: 288, cachelines: 2, members: 13 */
    /* sum members: 284, holes: 1, sum holes: 4 */

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Status read buffers (SRBs, unsolicited notifications) never use a QTCB
    [zfcp_fsf_req_create()]. zfcp_fsf_req_send() already uses this to
    distinguish SRBs from other FSF request types. We can re-use this method in
    zfcp_fsf_req_complete(). Introduce a helper function to make the check for
    req->qtcb less magic.

    SRBs always are FSF_QTCB_UNSOLICITED_STATUS, so we can hard-code this for
    the two trace functions dealing with SRBs.

    All other FSF request types have a QTCB and we can get the fsf_command from
    there.

    zfcp_dbf_hba_fsf_response() and thus zfcp_dbf_hba_fsf_res() are only called
    for non-SRB requests so it's safe to dereference the QTCB
    [zfcp_fsf_req_complete() returns early on SRB, else calls
    zfcp_fsf_protstatus_eval() which calls zfcp_dbf_hba_fsf_response()]. In
    zfcp_scsi_forget_cmnd() we guard the QTCB dereference with a preceding NULL
    check and rely on boolean shortcut evaluation.

    As a side effect, this causes an alignment hole which we can close in
    a later patch after having cleaned up all fields of struct zfcp_fsf_req.
    Before:
    $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko
    ...
    u32 status; /* 136 4 */
    u32 fsf_command; /* 140 4 */
    struct fsf_qtcb * qtcb; /* 144 8 */
    ...
    After:
    $ pahole -C zfcp_fsf_req drivers/s390/scsi/zfcp.ko
    ...
    u32 status; /* 136 4 */
    /* XXX 4 bytes hole, try to pack */
    struct fsf_qtcb * qtcb; /* 144 8 */
    ...

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Have structures just before the structures that use them (without
    disrupting sequences of using structures such as zfcp_unit and
    zfcp_scsi_dev):

    - zfcp_adapter_mempool embedded in zfcp_adapter,

    - zfcp_latenc... embedded in zfcp_scsi_dev.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • In contrast to struct fsf_qual_latency_info, the ones here are not FSF but
    software defined zfcp-internal.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • v2.6.10 history commit 4062e12b2ba2 ("[PATCH] s390: zfcp act enhancements")
    extended this mask by one nibble with the introduction of
    ZFCP_STATUS_COMMON_ACCESS_DENIED == 0x00800000 for ACT (access control
    table).

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Also clarify namespace prefix for the timeout used for FSF requests on
    behalf of SCSI error recovery: It is zfcp_fsf_ not zfcp_scsi_.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • While struct zfcp_adapter contains a pointer to zfcp_reqlist, the pointer
    field does not need to know the structure or even a prototype.

    The prototype was introduced with v2.6.34 commit b6bd2fb92a7b ("[SCSI]
    zfcp: Move FSF request tracking code to new file").

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • Since commit 663e0890e31c ("[SCSI] zfcp: remove access control tables
    interface") these helper functions are only used for auto port scan in
    zfcp_fc.c. Also change them to the corresponding namespace prefix.

    This is a small cleanup for the miscellaneous catchall compile unit
    zfcp_aux.c.

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier
     
  • mempool_destroy has taken null pointer check into account. so remove the
    redundant check.

    Signed-off-by: zhong jiang
    Acked-by: Benjamin Block
    [maier@linux.ibm.com: depends on v4.3 4e3ca3e033d1 ("mm/mempool: allow NULL `pool' pointer in mempool_destroy()")]
    Signed-off-by: Steffen Maier
    Signed-off-by: Martin K. Petersen

    zhong jiang
     

02 Jul, 2018

1 commit

  • ccw "busid" should always be NUL-terminated, as evident from e.g.
    get_ccwdev_by_busid doing "return (strcmp(bus_id, dev_name(dev)) == 0)".

    Replace all strncpy initializing busid with strlcpy. This fixes the
    following gcc 8 warnings:

    drivers/s390/scsi/zfcp_aux.c:104:2: warning: 'strncpy' specified bound 20
    equals destination size [-Wstringop-truncation]
    strncpy(busid, token, ZFCP_BUS_ID_SIZE);

    drivers/s390/block/dasd_eer.c:316:2: warning: 'strncpy' specified bound 10
    equals destination size [-Wstringop-truncation]
    strncpy(header.busid, dev_name(&device->cdev->dev), DASD_EER_BUSID_SIZE);

    drivers/s390/block/dasd_eer.c:359:2: warning: 'strncpy' specified bound 10
    equals destination size [-Wstringop-truncation]
    strncpy(header.busid, dev_name(&device->cdev->dev), DASD_EER_BUSID_SIZE);

    drivers/s390/block/dasd_devmap.c:429:3: warning: 'strncpy' specified bound
    20 equals destination size [-Wstringop-truncation]
    strncpy(new->bus_id, bus_id, DASD_BUS_ID_SIZE);

    Acked-by: Stefan Haberland
    Acked-by: Steffen Maier
    Reviewed-by: Heiko Carstens
    Signed-off-by: Vasily Gorbik
    Signed-off-by: Martin Schwidefsky

    Vasily Gorbik
     

11 Jun, 2018

1 commit

  • Pull SCSI updates from James Bottomley:
    "This is mostly updates to the usual drivers: ufs, qedf, mpt3sas, lpfc,
    xfcp, hisi_sas, cxlflash, qla2xxx.

    In the absence of Nic, we're also taking target updates which are
    mostly minor except for the tcmu refactor.

    The only real core change to worry about is the removal of high page
    bouncing (in sas, storvsc and iscsi). This has been well tested and no
    problems have shown up so far"

    * tag 'scsi-misc' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi: (268 commits)
    scsi: lpfc: update driver version to 12.0.0.4
    scsi: lpfc: Fix port initialization failure.
    scsi: lpfc: Fix 16gb hbas failing cq create.
    scsi: lpfc: Fix crash in blk_mq layer when executing modprobe -r lpfc
    scsi: lpfc: correct oversubscription of nvme io requests for an adapter
    scsi: lpfc: Fix MDS diagnostics failure (Rx < Tx)
    scsi: hisi_sas: Mark PHY as in reset for nexus reset
    scsi: hisi_sas: Fix return value when get_free_slot() failed
    scsi: hisi_sas: Terminate STP reject quickly for v2 hw
    scsi: hisi_sas: Add v2 hw force PHY function for internal ATA command
    scsi: hisi_sas: Include TMF elements in struct hisi_sas_slot
    scsi: hisi_sas: Try wait commands before before controller reset
    scsi: hisi_sas: Init disks after controller reset
    scsi: hisi_sas: Create a scsi_host_template per HW module
    scsi: hisi_sas: Reset disks when discovered
    scsi: hisi_sas: Add LED feature for v3 hw
    scsi: hisi_sas: Change common allocation mode of device id
    scsi: hisi_sas: change slot index allocation mode
    scsi: hisi_sas: Introduce hisi_sas_phy_set_linkrate()
    scsi: hisi_sas: fix a typo in hisi_sas_task_prep()
    ...

    Linus Torvalds
     

18 May, 2018

4 commits

  • The comment on fsf_qtcb_bottom_port.supported_speed did read as if the field
    can only assume one of two possible values (i.e. 0x1 for 1 GBit/s or 0x2 for
    2 GBit/s). This is not true for two reasons: first it is a flag field and
    can thus assume any combination and second there are meanwhile more speeds.

    Clarify comment on fsf_qtcb_bottom_port.supported_speed and add a comment to
    fsf_qtcb_bottom_config.fc_link_speed.

    Signed-off-by: Jens Remus
    Reviewed-by: Steffen Maier
    Reviewed-by: Fedor Loshakov
    Acked-by: Benjamin Block
    Acked-by: Hendrik Brueckner
    Signed-off-by: Steffen Maier
    Signed-off-by: Martin K. Petersen

    Jens Remus
     
  • Add port speed capabilities as defined in FC-LS RPSC ELS that have a
    counterpart FC_PORTSPEED_* defined in scsi/scsi_transport_fc.h.

    Suggested-by: Steffen Maier
    Signed-off-by: Jens Remus
    Reviewed-by: Steffen Maier
    Reviewed-by: Fedor Loshakov
    Acked-by: Hendrik Brueckner
    Acked-by: Benjamin Block
    Signed-off-by: Steffen Maier
    Signed-off-by: Martin K. Petersen

    Jens Remus
     
  • Otherwise iterating with list_for_each() over the adapter->erp_ready_head
    and adapter->erp_running_head lists can lead to an infinite loop. See commit
    "zfcp: fix infinite iteration on erp_ready_head list".

    The run-time check is only performed for debug kernels which have the kernel
    lock validator enabled. Following is an example of the warning that is
    reported, if the ERP lock is not held when calling zfcp_dbf_rec_trig():

    WARNING: CPU: 0 PID: 604 at drivers/s390/scsi/zfcp_dbf.c:288 zfcp_dbf_rec_trig+0x172/0x188
    Modules linked in: ...
    CPU: 0 PID: 604 Comm: kworker/u128:3 Not tainted 4.16.0-... #1
    Hardware name: IBM 2964 N96 702 (z/VM 6.4.0)
    Workqueue: zfcp_q_0.0.1906 zfcp_scsi_rport_work
    Krnl PSW : 00000000330fdbf9 00000000367e9728 (zfcp_dbf_rec_trig+0x172/0x188)
    R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3
    Krnl GPRS: 00000000c57a5d99 3288200000000000 0000000000000000 000000006cc82740
    00000000009d09d6 0000000000000000 00000000000000ff 0000000000000000
    0000000000000000 0000000000e1b5fe 000000006de01d38 0000000076130958
    000000006cc82548 000000006de01a98 00000000009d09d6 000000006a6d3c80
    Krnl Code: 00000000009d0ad2: eb7ff0b80004 lmg %r7,%r15,184(%r15)
    00000000009d0ad8: c0f4000d7dd0 brcl 15,b80678
    #00000000009d0ade: a7f40001 brc 15,9d0ae0
    >00000000009d0ae2: a7f4ff7d brc 15,9d09dc
    00000000009d0ae6: e340f0f00004 lg %r4,240(%r15)
    00000000009d0aec: eb7ff0b80004 lmg %r7,%r15,184(%r15)
    00000000009d0af2: 07f4 bcr 15,%r4
    00000000009d0af4: 0707 bcr 0,%r7
    Call Trace:
    ([] zfcp_dbf_rec_trig+0x66/0x188)
    [] zfcp_scsi_rport_work+0x98/0x190
    [] process_one_work+0x3d4/0x6f8
    [] worker_thread+0x232/0x418
    [] kthread+0x166/0x178
    [] kernel_thread_starter+0x6/0xc
    [] kernel_thread_starter+0x0/0xc
    2 locks held by kworker/u128:3/604:
    #0: ((wq_completion)name){+.+.}, at: [] process_one_work+0x1dc/0x6f8
    #1: ((work_completion)(&port->rport_work)){+.+.}, at: [] process_one_work+0x1dc/0x6f8
    Last Breaking-Event-Address:
    [] zfcp_dbf_rec_trig+0x16e/0x188
    ---[ end trace b2f4020572e2c124 ]---

    Suggested-by: Steffen Maier
    Signed-off-by: Jens Remus
    Reviewed-by: Benjamin Block
    Reviewed-by: Steffen Maier
    Signed-off-by: Steffen Maier
    Signed-off-by: Martin K. Petersen

    Jens Remus
     
  • I just happened to see the function header indentation of
    zfcp_fc_enqueue_event() and I picked some more from checkpatch:

    $ checkpatch.pl --strict -f drivers/s390/scsi/zfcp_fc.c
    ...
    CHECK: Alignment should match open parenthesis
    #113: FILE: drivers/s390/scsi/zfcp_fc.c:113:
    + fc_host_post_event(adapter->scsi_host, fc_get_event_number(),
    + event->code, event->data);

    CHECK: Blank lines aren't necessary before a close brace '}'
    #118: FILE: drivers/s390/scsi/zfcp_fc.c:118:
    +
    +}
    ...

    The change complements v2.6.36 commit 2d1e547f7523 ("[SCSI] zfcp: Post
    events through FC transport class").

    Signed-off-by: Steffen Maier
    Reviewed-by: Benjamin Block
    Signed-off-by: Martin K. Petersen

    Steffen Maier