Re: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination

[Willy Tarreau]

On Tue, Feb 07, 2017 at 06:12:34PM +0100, Willy Tarreau wrote:
> On Tue, Feb 07, 2017 at 09:02:51AM -0800, James Bottomley wrote:
> > On Tue, 2017-02-07 at 07:59 +0100, Willy Tarreau wrote:
> > > Hi James,
> > > 
> > > On Mon, Feb 06, 2017 at 10:38:48PM -0800, James Bottomley wrote:
> > > > On Mon, 2017-02-06 at 23:26 +0100, Willy Tarreau wrote:
> > > (...)
> > > > > We don't have the referenced commit above in 3.10 so we should be
> > > > > safe. Additionally I checked that neither 4.4 nor 3.12 have them 
> > > > > either, so that makes me feel confident that we can skip it in 
> > > > > 3.10 as well.
> > > > 
> > > > The original was also racy with respect to multiple commands, so 
> > > > the above fixed the race as well.
> > > 
> > > OK so I tried to backport it to 3.10. I dropped a few parts which 
> > > were addressing this one marked for stable 4.4+ :
> > > 7ff723a ("scsi: mpt3sas: Unblock device after controller reset")
> > > 
> > > And I got the attached patch. All I know is that it builds. I'd 
> > > appreciate it if someone could confirm its validity, in which case
> > > I'll add it.
> > 
> > The two patches apply without fuzz to your tree and the combination is
> > a far better bug fix than the original regardless of whether 7ff723a
> > exists in your tree or not.  By messing with the patches all you do is
> > add the potential for introducing new bugs for no benefit, so why take
> > risk for no upside?
> 
> Just because I'm suggested to apply this fix which is supposed to fix
> a regression brought by 7ff723a which itself is marked to fix 4.4+ only
> and which doesn't apply to 3.10. So now I'm getting confused because
> you say that these patches apply without fuzz but one part definitely
> is rejected and the other one has to be applied by hand. I want not
> to take a risk but I'm faced with these options :
>   - drop all these patches and stay as 3.10.104 is
>   - merge the "secure erase premature" + the the part of the patch
> that supposedly fixes the regression it introduced
>   - merge this fix + 7ff723a + whatever it depends on (not fond of
> it)
> 
> In all cases I don't even have the hardware to validate anything. I'd
> be more tempted with the first two options. If you think I'm taking
> risks by backporting the relevant part of the fix, I'll simply drop
> them all and leave the code as it is now.

So I could backport the fix marked for 4.4+ (7ff723a) and the one
suggested by Sathya (ffb5845). The context was slightly different
but the changes obvious enough to look good. If everyone is OK, I'll
add these two commits. Here are the backports.

Willy
>From acd34b89fe261c88398e26bd305552eb7808 Mon Sep 17 00:00:00 2001
From: Suganath Prabu S 
Date: Thu, 17 Nov 2016 16:15:58 +0530
Subject: scsi: mpt3sas: Unblock device after controller reset

commit 7ff723ad0f87feba43dda45fdae71206063dd7d4 upstream.

While issuing any ATA passthrough command to firmware the driver will
block the device. But it will unblock the device only if the I/O
completes through the ISR path. If a controller reset occurs before
command completion the device will remain in blocked state.

Make sure we unblock the device following a controller reset if an ATA
passthrough command was queued.

[mkp: clarified patch description]

Cc:  # v4.4+
Fixes: ac6c2a93bd07 ("mpt3sas: Fix for SATA drive in blocked state, after diag 
reset")
Signed-off-by: Suganath Prabu S 
Signed-off-by: Martin K. Petersen 
[wt: adjust context]
Signed-off-by: Willy Tarreau 
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index e414b71..8979403 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3390,6 +3390,11 @@ _scsih_check_volume_delete_events(struct MPT3SAS_ADAPTER 
*ioc,
le16_to_cpu(event_data->VolDevHandle));
 }
 
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
+
 /**
  * _scsih_flush_running_cmds - completing outstanding commands.
  * @ioc: per adapter object
@@ -3411,6 +3416,9 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
if (!scmd)
continue;
count++;
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device,
+   SDEV_RUNNING);
mpt3sas_base_free_smid(ioc, smid);
scsi_dma_unmap(scmd);
if (ioc->pci_error_recovery)
@@ -3515,11 +3523,6 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
ioc_status)
SAM_STAT_CHECK_CONDITION;
 }
 
-static inline bool 

[PATCH] scsi/sd: release scan_mutex during sync_cache and start_stop

[Song Liu]

When a device is deleted through sysfs handle "delete", the code
locks shost->scan_mutex. If multiple devices are deleted at the
same time, these deletes will be handled in series.

On the other hand, sd_shutdown() sometimes issues long latency
commands: sync cache and start_stop. It is not necessary for these
commands to run in series.

To reduce latency of parallel "delete" requests, this patch unlock
shost->scan_mutex before long latency commands and relock the mutex
after the command.

Signed-off-by: Song Liu 
---
 drivers/scsi/sd.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2ea3568..39aa209 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3241,6 +3241,8 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, 
int start)
 static void sd_shutdown(struct device *dev)
 {
struct scsi_disk *sdkp = dev_get_drvdata(dev);
+   struct scsi_device *sdev = sdkp->device;
+   struct Scsi_Host *shost = sdev->host;
 
if (!sdkp)
return; /* this can happen */
@@ -3249,13 +3251,17 @@ static void sd_shutdown(struct device *dev)
return;
 
if (sdkp->WCE && sdkp->media_present) {
+   mutex_unlock(>scan_mutex);
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
sd_sync_cache(sdkp);
+   mutex_lock(>scan_mutex);
}
 
if (system_state != SYSTEM_RESTART && sdkp->device->manage_start_stop) {
+   mutex_unlock(>scan_mutex);
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
sd_start_stop_device(sdkp, 0);
+   mutex_lock(>scan_mutex);
}
 }
 
-- 
2.9.3

Re: [PATCH -next] scsi: qedi: Fix possible memory leak in qedi_iscsi_update_conn()

[Rangankar, Manish]

On 07/02/17 8:22 PM, "Wei Yongjun"  wrote:

>From: Wei Yongjun 
>
>'conn_info' is malloced in qedi_iscsi_update_conn() and should be
>freed before leaving from the error handling cases, otherwise it
>will cause memory leak.
>
>Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
>driver framework.")
>Signed-off-by: Wei Yongjun 
>---
> drivers/scsi/qedi/qedi_iscsi.c | 4 
> 1 file changed, 4 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_iscsi.c
>b/drivers/scsi/qedi/qedi_iscsi.c
>index d6a2054..eb64469 100644
>--- a/drivers/scsi/qedi/qedi_iscsi.c
>+++ b/drivers/scsi/qedi/qedi_iscsi.c
>@@ -453,13 +453,9 @@ static int qedi_iscsi_update_conn(struct qedi_ctx
>*qedi,
>   if (rval) {
>   rval = -ENXIO;
>   QEDI_ERR(>dbg_ctx, "Could not update connection\n");
>-  goto update_conn_err;
>   }
> 
>   kfree(conn_info);
>-  rval = 0;
>-
>-update_conn_err:
>   return rval;
> }
>

Acked-by: Manish Rangankar 

Re: [PATCH net-next v2 08/12] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h

[Nicholas A. Bellinger]

Hi Florian,

On Tue, 2017-02-07 at 15:03 -0800, Florian Fainelli wrote:
> From: Russell King 
> 
> drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration 
> of function 'try_module_get' [-Werror=implicit-function-declaration]
> 
> Add linux/module.h to iscsi_target_login.c.
> 
> Signed-off-by: Russell King 
> Reviewed-by: Bart Van Assche 
> ---

Acked-by: Nicholas Bellinger 

Re: [PATCH v2 06/14] qla2xxx: Improve T10-DIF/PI handling in driver.

[Nicholas A. Bellinger]

On Fri, 2017-02-03 at 14:40 -0800, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> Add routines to support T10 DIF tag.
> 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Anil Gurumurthy 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_dbg.h |   1 +
>  drivers/scsi/qla2xxx/qla_def.h |  17 ++
>  drivers/scsi/qla2xxx/qla_target.c  | 598 
> +
>  drivers/scsi/qla2xxx/qla_target.h  |  37 ++-
>  drivers/scsi/qla2xxx/tcm_qla2xxx.c |  84 +-
>  5 files changed, 465 insertions(+), 272 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_dbg.h b/drivers/scsi/qla2xxx/qla_dbg.h
> index e1fc4e6..c6bffe9 100644
> --- a/drivers/scsi/qla2xxx/qla_dbg.h
> +++ b/drivers/scsi/qla2xxx/qla_dbg.h
> @@ -348,6 +348,7 @@ void __attribute__((format (printf, 4, 5)))
>  #define ql_dbg_tgt   0x4000 /* Target mode */
>  #define ql_dbg_tgt_mgt   0x2000 /* Target mode management */
>  #define ql_dbg_tgt_tmr   0x1000 /* Target mode task management */
> +#define ql_dbg_tgt_dif  0x0800 /* Target mode dif */
>  
>  extern int qla27xx_dump_mpi_ram(struct qla_hw_data *, uint32_t, uint32_t *,
>   uint32_t, void **);
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 8bc..d6436fc 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -2189,6 +2189,23 @@ struct qlt_plogi_ack_t {
>   void*fcport;
>  };
>  
> +enum qla_tgt_prot_op {
> + QLA_PROT_NORMAL  = 0,
> + QLA_PROT_DIN_INSERT,
> + QLA_PROT_DOUT_INSERT,
> + QLA_PROT_DIN_STRIP,
> + QLA_PROT_DOUT_STRIP,
> + QLA_PROT_DIN_PASS,
> + QLA_PROT_DOUT_PASS,
> +};
> +
> +enum qla_tgt_prot_type {
> + QLA_TGT_PROT_TYPE0,
> + QLA_TGT_PROT_TYPE1,
> + QLA_TGT_PROT_TYPE2,
> + QLA_TGT_PROT_TYPE3,
> +};
> +

I don't get it, why are you duplicating target_prot_op and
target_prot_type..?

Re: [PATCH 4/5] target: Fix multi-session dynamic se_node_acl double free OOPs

[Nicholas A. Bellinger]

On Tue, 2017-02-07 at 15:12 -0800, Christoph Hellwig wrote:
> And the real patch after compile fixing it is here of course:
> 

Getting rid of the extra se_node_acl->acl_free_comp seems to make sense
here..

The only potential issue is if returning from configfs syscall
rmdir /sys/kernel/config/target/$FABRIC/$WWN/$TPGT/acls/$INITIATOR/
before se_node_acl memory is released has implications when the
underlying ../$WWN/$TPGT/ is removed immediately after.

In any event, I'll verify this patch with the original test case and use
it instead if everything looks OK.

> diff --git a/drivers/target/target_core_internal.h 
> b/drivers/target/target_core_internal.h
> index 9ab7090f7c83..96c38f30069d 100644
> --- a/drivers/target/target_core_internal.h
> +++ b/drivers/target/target_core_internal.h
> @@ -152,6 +152,7 @@ void  target_qf_do_work(struct work_struct *work);
>  bool target_check_wce(struct se_device *dev);
>  bool target_check_fua(struct se_device *dev);
>  void __target_execute_cmd(struct se_cmd *, bool);
> +void target_put_nacl(struct se_node_acl *);
>  
>  /* target_core_stat.c */
>  void target_stat_setup_dev_default_groups(struct se_device *);
> diff --git a/drivers/target/target_core_tpg.c 
> b/drivers/target/target_core_tpg.c
> index d99752c6cd60..08738c87e5b0 100644
> --- a/drivers/target/target_core_tpg.c
> +++ b/drivers/target/target_core_tpg.c
> @@ -197,7 +197,6 @@ static struct se_node_acl *target_alloc_node_acl(struct 
> se_portal_group *tpg,
>   INIT_LIST_HEAD(>acl_sess_list);
>   INIT_HLIST_HEAD(>lun_entry_hlist);
>   kref_init(>acl_kref);
> - init_completion(>acl_free_comp);
>   spin_lock_init(>nacl_sess_lock);
>   mutex_init(>lun_entry_mutex);
>   atomic_set(>acl_pr_ref_count, 0);
> @@ -370,21 +369,6 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
> *acl)
>   target_shutdown_sessions(acl);
>  
>   target_put_nacl(acl);
> - /*
> -  * Wait for last target_put_nacl() to complete in target_complete_nacl()
> -  * for active fabric session transport_deregister_session() callbacks.
> -  */
> - wait_for_completion(>acl_free_comp);
> -
> - core_tpg_wait_for_nacl_pr_ref(acl);
> - core_free_device_list_for_node(acl, tpg);
> -
> - pr_debug("%s_TPG[%hu] - Deleted ACL with TCQ Depth: %d for %s"
> - " Initiator Node: %s\n", tpg->se_tpg_tfo->get_fabric_name(),
> - tpg->se_tpg_tfo->tpg_get_tag(tpg), acl->queue_depth,
> - tpg->se_tpg_tfo->get_fabric_name(), acl->initiatorname);
> -
> - kfree(acl);
>  }
>  
>  /*   core_tpg_set_initiator_node_queue_depth():
> diff --git a/drivers/target/target_core_transport.c 
> b/drivers/target/target_core_transport.c
> index 1cadc9eefa21..9ebd86a8ef41 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -453,19 +453,25 @@ ssize_t target_show_dynamic_sessions(struct 
> se_portal_group *se_tpg, char *page)
>  }
>  EXPORT_SYMBOL(target_show_dynamic_sessions);
>  
> -static void target_complete_nacl(struct kref *kref)
> +static void target_destroy_nacl(struct kref *kref)
>  {
>   struct se_node_acl *nacl = container_of(kref,
>   struct se_node_acl, acl_kref);
> + struct se_portal_group *se_tpg = nacl->se_tpg;
>  
> - complete(>acl_free_comp);
> + mutex_lock(_tpg->acl_node_mutex);
> + list_del(>acl_list);
> + mutex_unlock(_tpg->acl_node_mutex);
> +
> + core_tpg_wait_for_nacl_pr_ref(nacl);
> + core_free_device_list_for_node(nacl, se_tpg);
> + kfree(nacl);
>  }
>  
>  void target_put_nacl(struct se_node_acl *nacl)
>  {
> - kref_put(>acl_kref, target_complete_nacl);
> + kref_put(>acl_kref, target_destroy_nacl);
>  }
> -EXPORT_SYMBOL(target_put_nacl);
>  
>  void transport_deregister_session_configfs(struct se_session *se_sess)
>  {
> @@ -499,12 +505,40 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);
>  void transport_free_session(struct se_session *se_sess)
>  {
>   struct se_node_acl *se_nacl = se_sess->se_node_acl;
> +
>   /*
>* Drop the se_node_acl->nacl_kref obtained from within
>* core_tpg_get_initiator_node_acl().
>*/
>   if (se_nacl) {
> + struct se_portal_group *se_tpg = se_nacl->se_tpg;
> + const struct target_core_fabric_ops *se_tfo = 
> se_tpg->se_tpg_tfo;
> + unsigned long flags;
> + bool stop = false;
> +
>   se_sess->se_node_acl = NULL;
> +
> + /*
> +  * Also determine if we need to drop the extra ->cmd_kref if
> +  * it had been previously dynamically generated, and
> +  * the endpoint is not caching dynamic ACLs.
> +  */
> + mutex_lock(_tpg->acl_node_mutex);
> + if (se_nacl->dynamic_node_acl &&
> + !se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
> + spin_lock_irqsave(_nacl->nacl_sess_lock, flags);
> 

Re: [PATCH v2 07/18] lpfc: NVME Initiator: Base modifications Part E

[James Smart]

On 2/7/2017 1:03 AM, Johannes Thumshirn wrote:


Yes but patch  03/18 'lpfc: NVME Initiator: Base modifications Part A'
still has calls to lpfc_sli_hbq_count(phba) (and in fact introduces this
change).



I realize I cut these in a silly way.  In the v1 patches, I had a big 
patch that I then cut into 6 parts, by file.  In the v2 patches, I tried 
to keep the patches as is, and address the comments in the respective 
patch the comment came from. Which resulted in 3/8 with an old 
reference, but patch 8/8 being the one that reverted this reverence. 
Sorry..  I'll recut and repost.


-- james

Re: [PATCH] scsi: aacraid: avoid open-coded upper_32_bits

[Martin K. Petersen]

> "Arnd" == Arnd Bergmann  writes:

Arnd> Shifting a dma_addr_t right by 32 bits causes a compile-time
Arnd> warning when that type is only 32 bit wide:

Arnd> drivers/scsi/aacraid/src.c: In function 'aac_src_start_adapter':
Arnd> drivers/scsi/aacraid/src.c:414:29: error: right shift count >=
Arnd> width of type [-Werror=shift-count-overflow]

Arnd> This changes the driver to use the predefined macros consistently,
Arnd> including one correct but open-coded upper_32_bits() instance.

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH net-next v2 02/12] net: cgroups: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

net/core/netprio_cgroup.c:303:16: error: expected declaration specifiers or 
'...' before string constant
MODULE_LICENSE("GPL v2");
   ^~~~

Add linux/module.h to fix this.

Signed-off-by: Russell King 
---
 net/core/netprio_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c
index 2ec86fc552df..756637dc7a57 100644
--- a/net/core/netprio_cgroup.c
+++ b/net/core/netprio_cgroup.c
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

[PATCH net-next v2 00/12] net: dsa: remove unnecessary phy.h include

[Florian Fainelli]

Hi all,

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

In order to make this change, several drivers need to be updated to
include necessary headers that they were picking up through this
include.  This has resulted in a much larger patch series.

I'm assuming the 0-day builder has had 24 hours with this series, and
hasn't reported any further issues with it - the last issue was two
weeks ago (before I became ill) which I fixed over the last weekend.

I'm hoping this doesn't conflict with what's already in net-next...

David, this should probably go via your tree considering the diffstat.

Changes in v2:

- took Russell's patch series
- removed Qualcomm EMAC patch
- rebased against net-next/master

Russell King (12):
  net: sunrpc: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: cgroups: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: macb: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: lan78xx: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: bgmac: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: fman: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: mvneta: fix build errors when linux/phy*.h is removed from
net/dsa.h
  iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h
  MIPS: Octeon: Remove unnecessary MODULE_*()
  net: liquidio: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: ath5k: fix build errors when linux/phy*.h is removed from
net/dsa.h
  net: dsa: remove unnecessary phy*.h includes

 arch/mips/cavium-octeon/octeon-platform.c | 4 
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 drivers/net/ethernet/cadence/macb.h   | 2 ++
 drivers/net/ethernet/cavium/liquidio/lio_main.c   | 1 +
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c| 1 +
 drivers/net/ethernet/cavium/liquidio/octeon_console.c | 1 +
 drivers/net/ethernet/freescale/fman/fman_memac.c  | 1 +
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 drivers/net/usb/lan78xx.c | 1 +
 drivers/net/wireless/ath/ath5k/ahb.c  | 2 +-
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 include/net/dsa.h | 5 +++--
 net/core/netprio_cgroup.c | 1 +
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c| 1 +
 15 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.9.3

Re: [PATCH 4/5] target: Fix multi-session dynamic se_node_acl double free OOPs

[Christoph Hellwig]

And the real patch after compile fixing it is here of course:

diff --git a/drivers/target/target_core_internal.h 
b/drivers/target/target_core_internal.h
index 9ab7090f7c83..96c38f30069d 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -152,6 +152,7 @@ voidtarget_qf_do_work(struct work_struct *work);
 bool   target_check_wce(struct se_device *dev);
 bool   target_check_fua(struct se_device *dev);
 void   __target_execute_cmd(struct se_cmd *, bool);
+void   target_put_nacl(struct se_node_acl *);
 
 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index d99752c6cd60..08738c87e5b0 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -197,7 +197,6 @@ static struct se_node_acl *target_alloc_node_acl(struct 
se_portal_group *tpg,
INIT_LIST_HEAD(>acl_sess_list);
INIT_HLIST_HEAD(>lun_entry_hlist);
kref_init(>acl_kref);
-   init_completion(>acl_free_comp);
spin_lock_init(>nacl_sess_lock);
mutex_init(>lun_entry_mutex);
atomic_set(>acl_pr_ref_count, 0);
@@ -370,21 +369,6 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
target_shutdown_sessions(acl);
 
target_put_nacl(acl);
-   /*
-* Wait for last target_put_nacl() to complete in target_complete_nacl()
-* for active fabric session transport_deregister_session() callbacks.
-*/
-   wait_for_completion(>acl_free_comp);
-
-   core_tpg_wait_for_nacl_pr_ref(acl);
-   core_free_device_list_for_node(acl, tpg);
-
-   pr_debug("%s_TPG[%hu] - Deleted ACL with TCQ Depth: %d for %s"
-   " Initiator Node: %s\n", tpg->se_tpg_tfo->get_fabric_name(),
-   tpg->se_tpg_tfo->tpg_get_tag(tpg), acl->queue_depth,
-   tpg->se_tpg_tfo->get_fabric_name(), acl->initiatorname);
-
-   kfree(acl);
 }
 
 /* core_tpg_set_initiator_node_queue_depth():
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 1cadc9eefa21..9ebd86a8ef41 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -453,19 +453,25 @@ ssize_t target_show_dynamic_sessions(struct 
se_portal_group *se_tpg, char *page)
 }
 EXPORT_SYMBOL(target_show_dynamic_sessions);
 
-static void target_complete_nacl(struct kref *kref)
+static void target_destroy_nacl(struct kref *kref)
 {
struct se_node_acl *nacl = container_of(kref,
struct se_node_acl, acl_kref);
+   struct se_portal_group *se_tpg = nacl->se_tpg;
 
-   complete(>acl_free_comp);
+   mutex_lock(_tpg->acl_node_mutex);
+   list_del(>acl_list);
+   mutex_unlock(_tpg->acl_node_mutex);
+
+   core_tpg_wait_for_nacl_pr_ref(nacl);
+   core_free_device_list_for_node(nacl, se_tpg);
+   kfree(nacl);
 }
 
 void target_put_nacl(struct se_node_acl *nacl)
 {
-   kref_put(>acl_kref, target_complete_nacl);
+   kref_put(>acl_kref, target_destroy_nacl);
 }
-EXPORT_SYMBOL(target_put_nacl);
 
 void transport_deregister_session_configfs(struct se_session *se_sess)
 {
@@ -499,12 +505,40 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);
 void transport_free_session(struct se_session *se_sess)
 {
struct se_node_acl *se_nacl = se_sess->se_node_acl;
+
/*
 * Drop the se_node_acl->nacl_kref obtained from within
 * core_tpg_get_initiator_node_acl().
 */
if (se_nacl) {
+   struct se_portal_group *se_tpg = se_nacl->se_tpg;
+   const struct target_core_fabric_ops *se_tfo = 
se_tpg->se_tpg_tfo;
+   unsigned long flags;
+   bool stop = false;
+
se_sess->se_node_acl = NULL;
+
+   /*
+* Also determine if we need to drop the extra ->cmd_kref if
+* it had been previously dynamically generated, and
+* the endpoint is not caching dynamic ACLs.
+*/
+   mutex_lock(_tpg->acl_node_mutex);
+   if (se_nacl->dynamic_node_acl &&
+   !se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
+   spin_lock_irqsave(_nacl->nacl_sess_lock, flags);
+   if (list_empty(_nacl->acl_sess_list))
+   stop = true;
+   spin_unlock_irqrestore(_nacl->nacl_sess_lock, flags);
+
+   if (stop)
+   list_del_init(_nacl->acl_list);
+   }
+   mutex_unlock(_tpg->acl_node_mutex);
+
+   if (stop)
+   target_put_nacl(se_nacl);
+
target_put_nacl(se_nacl);
}
if (se_sess->sess_cmd_map) {
@@ -518,16 +552,12 @@ EXPORT_SYMBOL(transport_free_session);
 void 

[PATCH net-next v2 04/12] net: lan78xx: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/usb/lan78xx.c:394:33: sparse: expected ; at end of declaration
drivers/net/usb/lan78xx.c:394:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/usb/lan78xx.c:394:33: sparse: got interface
drivers/net/usb/lan78xx.c:403:1: sparse: Expected ; at the end of type 
declaration
drivers/net/usb/lan78xx.c:403:1: sparse: got }

Add linux/phy.h to lan78xx.c

Signed-off-by: Russell King 
---
 drivers/net/usb/lan78xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 08f8703e4d54..9889a70ff4f6 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "lan78xx.h"
 
 #define DRIVER_AUTHOR  "WOOJUNG HUH "
-- 
2.9.3

[PATCH net-next v2 03/12] net: macb: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/cadence/macb.h:862:33: sparse: expected ; at end of 
declaration
drivers/net/ethernet/cadence/macb.h:862:33: sparse: Expected } at end of 
struct-union-enum-specifier
drivers/net/ethernet/cadence/macb.h:862:33: sparse: got phy_interface
drivers/net/ethernet/cadence/macb.h:877:1: sparse: Expected ; at the end of 
type declaration
drivers/net/ethernet/cadence/macb.h:877:1: sparse: got }
In file included from drivers/net/ethernet/cadence/macb_pci.c:29:0:
drivers/net/ethernet/cadence/macb.h:862:2: error: unknown type name 
'phy_interface_t'
 phy_interface_t  phy_interface;
 ^~~

Add linux/phy.h to macb.h

Signed-off-by: Russell King 
Acked-by: Nicolas Ferre 
---
 drivers/net/ethernet/cadence/macb.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h 
b/drivers/net/ethernet/cadence/macb.h
index a2cf91223003..234a49eaccfd 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -10,6 +10,8 @@
 #ifndef _MACB_H
 #define _MACB_H
 
+#include 
+
 #define MACB_GREGS_NBR 16
 #define MACB_GREGS_VERSION 2
 #define MACB_MAX_QUEUES 8
-- 
2.9.3

[PATCH net-next v2 01/12] net: sunrpc: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

Removing linux/phy.h from net/dsa.h reveals a build error in the sunrpc
code:

net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_rdma_bc_put':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:277:2: error: implicit declaration 
of function 'module_put' [-Werror=implicit-function-declaration]
net/sunrpc/xprtrdma/svc_rdma_backchannel.c: In function 'xprt_setup_rdma_bc':
net/sunrpc/xprtrdma/svc_rdma_backchannel.c:348:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Fix this by adding linux/module.h to svc_rdma_backchannel.c

Signed-off-by: Russell King 
Acked-by: Anna Schumaker 
---
 net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c 
b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
index 288e35c2d8f4..cb1e48e54eb1 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c
@@ -4,6 +4,7 @@
  * Support for backward direction RPCs on RPC/RDMA (server-side).
  */
 
+#include 
 #include 
 #include "xprt_rdma.h"
 
-- 
2.9.3

[PATCH net-next v2 09/12] MIPS: Octeon: Remove unnecessary MODULE_*()

[Florian Fainelli]

From: Russell King 

octeon-platform.c can not be built as a module for two reasons:

(a) the Makefile doesn't allow it:
obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o

(b) the multiple *_initcall() statements, each of which are translated
to a module_init() call when attempting a module build, become
aliases to init_module().  Having more than one alias will cause a
build error.

Hence, rather than adding a linux/module.h include, remove the redundant
MODULE_*() from this file.

Acked-by: David Daney 
Signed-off-by: Russell King 
---
 arch/mips/cavium-octeon/octeon-platform.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-platform.c 
b/arch/mips/cavium-octeon/octeon-platform.c
index 37a932d9148c..8297ce714c5e 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -1060,7 +1060,3 @@ static int __init octeon_publish_devices(void)
return of_platform_bus_probe(NULL, octeon_ids, NULL);
 }
 arch_initcall(octeon_publish_devices);
-
-MODULE_AUTHOR("David Daney ");
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("Platform driver for Octeon SOC");
-- 
2.9.3

Re: [PATCH 4/5] target: Fix multi-session dynamic se_node_acl double free OOPs

[Christoph Hellwig]

On Tue, Feb 07, 2017 at 01:17:49PM +, Nicholas A. Bellinger wrote:
> - list_del(>acl_list);
> + list_del_init(>acl_list);

All these list_del_init changes don't make sense to me - the whole target
code never does a list_empty check on ->acl_list.


Looking further I think all nacls should be switched to the direct
free from the kref release callback scheme - there is no real need
to wait for freeing the nacl I think.

Untested patch below:

diff --git a/drivers/target/target_core_internal.h 
b/drivers/target/target_core_internal.h
index 9ab7090f7c83..96c38f30069d 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -152,6 +152,7 @@ voidtarget_qf_do_work(struct work_struct *work);
 bool   target_check_wce(struct se_device *dev);
 bool   target_check_fua(struct se_device *dev);
 void   __target_execute_cmd(struct se_cmd *, bool);
+void   target_put_nacl(struct se_node_acl *);
 
 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index d99752c6cd60..08738c87e5b0 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -197,7 +197,6 @@ static struct se_node_acl *target_alloc_node_acl(struct 
se_portal_group *tpg,
INIT_LIST_HEAD(>acl_sess_list);
INIT_HLIST_HEAD(>lun_entry_hlist);
kref_init(>acl_kref);
-   init_completion(>acl_free_comp);
spin_lock_init(>nacl_sess_lock);
mutex_init(>lun_entry_mutex);
atomic_set(>acl_pr_ref_count, 0);
@@ -370,21 +369,6 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
target_shutdown_sessions(acl);
 
target_put_nacl(acl);
-   /*
-* Wait for last target_put_nacl() to complete in target_complete_nacl()
-* for active fabric session transport_deregister_session() callbacks.
-*/
-   wait_for_completion(>acl_free_comp);
-
-   core_tpg_wait_for_nacl_pr_ref(acl);
-   core_free_device_list_for_node(acl, tpg);
-
-   pr_debug("%s_TPG[%hu] - Deleted ACL with TCQ Depth: %d for %s"
-   " Initiator Node: %s\n", tpg->se_tpg_tfo->get_fabric_name(),
-   tpg->se_tpg_tfo->tpg_get_tag(tpg), acl->queue_depth,
-   tpg->se_tpg_tfo->get_fabric_name(), acl->initiatorname);
-
-   kfree(acl);
 }
 
 /* core_tpg_set_initiator_node_queue_depth():
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 1cadc9eefa21..72718283e553 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -453,19 +453,25 @@ ssize_t target_show_dynamic_sessions(struct 
se_portal_group *se_tpg, char *page)
 }
 EXPORT_SYMBOL(target_show_dynamic_sessions);
 
-static void target_complete_nacl(struct kref *kref)
+static void target_destroy_nacl(struct kref *kref)
 {
struct se_node_acl *nacl = container_of(kref,
struct se_node_acl, acl_kref);
+   struct se_portal_group *se_tpg = nacl->se_tpg;
 
-   complete(>acl_free_comp);
+   mutex_lock(_tpg->acl_node_mutex);
+   list_del(>acl_list);
+   mutex_unlock(_tpg->acl_node_mutex);
+
+   core_tpg_wait_for_nacl_pr_ref(nacl);
+   core_free_device_list_for_node(nacl, se_tpg);
+   kfree(nacl);
 }
 
 void target_put_nacl(struct se_node_acl *nacl)
 {
-   kref_put(>acl_kref, target_complete_nacl);
+   kref_put(>acl_kref, target_destroy_nacl);
 }
-EXPORT_SYMBOL(target_put_nacl);
 
 void transport_deregister_session_configfs(struct se_session *se_sess)
 {
@@ -499,12 +505,39 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);
 void transport_free_session(struct se_session *se_sess)
 {
struct se_node_acl *se_nacl = se_sess->se_node_acl;
+
/*
 * Drop the se_node_acl->nacl_kref obtained from within
 * core_tpg_get_initiator_node_acl().
 */
if (se_nacl) {
+   struct se_portal_group *se_tpg = se_nacl->se_tpg;
+   const struct target_core_fabric_ops *se_tfo = 
se_tpg->se_tpg_tfo;
+   unsigned long flags;
+
se_sess->se_node_acl = NULL;
+
+   /*
+* Also determine if we need to drop the extra ->cmd_kref if
+* it had been previously dynamically generated, and
+* the endpoint is not caching dynamic ACLs.
+*/
+   mutex_lock(_tpg->acl_node_mutex);
+   if (se_nacl->dynamic_node_acl &&
+   !se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
+   spin_lock_irqsave(_nacl->nacl_sess_lock, flags);
+   if (list_empty(_nacl->acl_sess_list))
+   stop = true;
+   spin_unlock_irqrestore(_nacl->nacl_sess_lock, flags);
+
+   if (stop)
+   

[PATCH net-next v2 05/12] net: bgmac: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/broadcom/bgmac.c:1015:17: error: dereferencing pointer to 
incomplete type 'struct mii_bus'
drivers/net/ethernet/broadcom/bgmac.c:1185:2: error: implicit declaration of 
function 'phy_start' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1198:2: error: implicit declaration of 
function 'phy_stop' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1239:9: error: implicit declaration of 
function 'phy_mii_ioctl' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1389:28: error: 
'phy_ethtool_get_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1390:28: error: 
'phy_ethtool_set_link_ksettings' undeclared here (not in a function)
drivers/net/ethernet/broadcom/bgmac.c:1403:13: error: dereferencing pointer to 
incomplete type 'struct phy_device'
drivers/net/ethernet/broadcom/bgmac.c:1417:3: error: implicit declaration of 
function 'phy_print_status' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1424:26: error: storage size of 
'fphy_status' isn't known
drivers/net/ethernet/broadcom/bgmac.c:1424:9: error: variable 'fphy_status' has 
initializer but incomplete type
drivers/net/ethernet/broadcom/bgmac.c:1425:11: warning: excess elements in 
struct initializer
drivers/net/ethernet/broadcom/bgmac.c:1425:3: error: unknown field 'link' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1426:12: note: in expansion of macro 
'SPEED_1000'
drivers/net/ethernet/broadcom/bgmac.c:1426:3: error: unknown field 'speed' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1427:13: note: in expansion of macro 
'DUPLEX_FULL'
drivers/net/ethernet/broadcom/bgmac.c:1427:3: error: unknown field 'duplex' 
specified in initializer
drivers/net/ethernet/broadcom/bgmac.c:1432:12: error: implicit declaration of 
function 'fixed_phy_register' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1432:31: error: 'PHY_POLL' undeclared 
(first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1438:8: error: implicit declaration of 
function 'phy_connect_direct' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1439:6: error: 'PHY_INTERFACE_MODE_MII' 
undeclared (first use in this function)
drivers/net/ethernet/broadcom/bgmac.c:1521:2: error: implicit declaration of 
function 'phy_disconnect' [-Werror=implicit-function-declaration]
drivers/net/ethernet/broadcom/bgmac.c:1541:15: error: expected declaration 
specifiers or '...' before string constant

Add linux/phy.h to bgmac.c

Signed-off-by: Russell King 
Acked-by: Rafał Miłecki 
---
 drivers/net/ethernet/broadcom/bgmac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c 
b/drivers/net/ethernet/broadcom/bgmac.c
index fe88126b1e0c..20fe2520da42 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "bgmac.h"
 
 static bool bgmac_wait_value(struct bgmac *bgmac, u16 reg, u32 mask,
-- 
2.9.3

[PATCH net-next v2 11/12] net: ath5k: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

Fix these errors reported by the 0-day builder by replacing the
linux/export.h include with linux/module.h.

In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1463:1: warning: data definition has no type or storage 
class
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1463:1: error: type defaults to 'int' in declaration of 
'module_init' [-Werror=implicit-int]
 module_init(__driver##_init); \
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
include/linux/device.h:1468:1: warning: data definition has no type or storage 
class
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
include/linux/device.h:1468:1: error: type defaults to 'int' in declaration of 
'module_exit' [-Werror=implicit-int]
 module_exit(__driver##_exit);
 ^
include/linux/platform_device.h:228:2: note: in expansion of macro 
'module_driver'
  module_driver(__platform_driver, platform_driver_register, \
  ^
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: warning: parameter names (without 
types) in function declaration
In file included from include/linux/platform_device.h:14:0,
 from drivers/net/wireless/ath/ath5k/ahb.c:20:
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_exit' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1464:20: note: in definition of macro 'module_driver'
 static void __exit __driver##_exit(void) \
^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~
drivers/net/wireless/ath/ath5k/ahb.c:233:24: warning: 'ath_ahb_driver_init' 
defined but not used [-Wunused-function]
 module_platform_driver(ath_ahb_driver);
^
include/linux/device.h:1459:19: note: in definition of macro 'module_driver'
 static int __init __driver##_init(void) \
   ^~~~
drivers/net/wireless/ath/ath5k/ahb.c:233:1: note: in expansion of macro 
'module_platform_driver'
 module_platform_driver(ath_ahb_driver);
 ^~

Signed-off-by: Russell King 
Acked-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath5k/ahb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath5k/ahb.c 
b/drivers/net/wireless/ath/ath5k/ahb.c
index 2ca88b593e4c..c0794f5988b3 100644
--- a/drivers/net/wireless/ath/ath5k/ahb.c
+++ b/drivers/net/wireless/ath/ath5k/ahb.c
@@ -16,10 +16,10 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include "ath5k.h"
 #include "debug.h"
-- 
2.9.3

[PATCH net-next v2 08/12] iscsi: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/target/iscsi/iscsi_target_login.c:1135:7: error: implicit declaration 
of function 'try_module_get' [-Werror=implicit-function-declaration]

Add linux/module.h to iscsi_target_login.c.

Signed-off-by: Russell King 
Reviewed-by: Bart Van Assche 
---
 drivers/target/iscsi/iscsi_target_login.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 450f51deb2a2..eab274d17b5c 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -17,6 +17,7 @@
  
**/
 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

Re: [PATCH v1 0/8] scsi: ufs: enhancements, bug fixes and debug support

[Martin K. Petersen]

> "Subhash" == Subhash Jadavani  writes:

Subhash> This patch series adds following things: - Gear scaling during
Subhash> clock scaling to save additional power - Make clock scaling
Subhash> independent clock gating so that we can set lower clock gating
Subhash> timeout than clock scaling window - one bug fix to clock
Subhash> scaling - additional debug support

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH net-next v2 10/12] net: liquidio: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: type defaults to 
'int' in declaration of 'MODULE_AUTHOR'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:30: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: type defaults to 
'int' in declaration of 'MODULE_DESCRIPTION'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:31: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: type defaults to 
'int' in declaration of 'MODULE_LICENSE'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:32: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: expected 
declaration specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: type defaults to 
'int' in declaration of 'MODULE_VERSION'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:33: error: function 
declaration isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:36: error: expected ')' 
before 'int'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:37: error: expected ')' 
before string constant
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: error: type defaults to 
'int' in declaration of 'MODULE_DEVICE_TABLE'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:325: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: error: type defaults 
to 'int' in declaration of 'module_init'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3250: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: data 
definition has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: error: type defaults 
to 'int' in declaration of 'module_exit'
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c:3251: warning: parameter 
names (without types) in function declaration
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: type defaults to 
'int' in declaration of 'MODULE_AUTHOR'
drivers/net/ethernet/cavium/liquidio/lio_main.c:36: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: type defaults to 
'int' in declaration of 'MODULE_DESCRIPTION'
drivers/net/ethernet/cavium/liquidio/lio_main.c:37: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: type defaults to 
'int' in declaration of 'MODULE_LICENSE'
drivers/net/ethernet/cavium/liquidio/lio_main.c:38: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: expected declaration 
specifiers or '...' before string constant
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: warning: data definition 
has no type or storage class
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: type defaults to 
'int' in declaration of 'MODULE_VERSION'
drivers/net/ethernet/cavium/liquidio/lio_main.c:39: error: function declaration 
isn't a prototype
drivers/net/ethernet/cavium/liquidio/lio_main.c:40: 

[PATCH net-next v2 07/12] net: mvneta: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/marvell/mvneta.c:2694:26: error: storage size of 'status' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:26: error: storage size of 'changed' 
isn't known
drivers/net/ethernet/marvell/mvneta.c:2695:9: error: variable 'changed' has 
initializer but incomplete type
drivers/net/ethernet/marvell/mvneta.c:2709:2: error: implicit declaration of 
function 'fixed_phy_update_state' [-Werror=implicit-function-declaration]

Add linux/phy_fixed.h to mvneta.c

Signed-off-by: Russell King 
Acked-by: Thomas Petazzoni 
---
 drivers/net/ethernet/marvell/mvneta.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/marvell/mvneta.c 
b/drivers/net/ethernet/marvell/mvneta.c
index 0f4d1697be46..fdf71720e707 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

[PATCH net-next v2 06/12] net: fman: fix build errors when linux/phy*.h is removed from net/dsa.h

[Florian Fainelli]

From: Russell King 

drivers/net/ethernet/freescale/fman/fman_memac.c:519:21: error: dereferencing 
pointer to incomplete type 'struct fixed_phy_status'

Add linux/phy_fixed.h to fman_memac.c

Signed-off-by: Russell King 
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c 
b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 71a5ded9d1de..cd6a53eaf161 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* PCS registers */
-- 
2.9.3

[PATCH net-next v2 12/12] net: dsa: remove unnecessary phy*.h includes

[Florian Fainelli]

From: Russell King 

Including phy.h and phy_fixed.h into net/dsa.h causes phy*.h to be an
unnecessary dependency for quite a large amount of the kernel.  There's
very little which actually requires definitions from phy.h in net/dsa.h
- the include itself only wants the declaration of a couple of
structures and IFNAMSIZ.

Add linux/if.h for IFNAMSIZ, declarations for the structures, phy.h to
mv88e6xxx.h as it needs it for phy_interface_t, and remove both phy.h
and phy_fixed.h from net/dsa.h.

This patch reduces from around 800 files rebuilt to around 40 - even
with ccache, the time difference is noticable.

Tested-by: Vivien Didelot 
Reviewed-by: Florian Fainelli 
Signed-off-by: Russell King 
---
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 include/net/dsa.h | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h 
b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 8a21800374f3..91c4dd25c2d3 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef UINT64_MAX
 #define UINT64_MAX (u64)(~((u64)0))
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b49b2004891e..4e13e695f025 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -11,17 +11,18 @@
 #ifndef __LINUX_NET_DSA_H
 #define __LINUX_NET_DSA_H
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 
 struct tc_action;
+struct phy_device;
+struct fixed_phy_status;
 
 enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = 0,
-- 
2.9.3

Re: [PATCH 5/5] target: Fix COMPARE_AND_WRITE ref leak for non GOOD status

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 1/5] target: Don't BUG_ON during NodeACL dynamic -> explicit conversion

[Christoph Hellwig]

On Tue, Feb 07, 2017 at 01:17:46PM +, Nicholas A. Bellinger wrote:
> + if (orig->se_lun_acl != NULL) {
> + pr_warn_ratelimited("Detected existing explicit"
> + " se_lun_acl->se_lun_group reference for %s"
> + " mapped_lun: %llu, ignoring\n",
> +  nacl->initiatorname, mapped_lun);

The ignoring in the message confused the heck out of me first.  But it 
seems that's just an incorrect leftover from the original message, as the
changelog also says fail instead.  With that fixed up (and maybe the
whole message in a single string literal on a single line):

Reviewed-by: Christoph Hellwig 

Re: [PATCH 3/5] target: Fix early transport_generic_handle_tmr abort scenario

[Christoph Hellwig]

On Tue, Feb 07, 2017 at 01:17:48PM +, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger 
> 
> This patch fixes a bug where incoming task management requests
> can be explicitly aborted during an active LUN_RESET, but who's
> struct work_struct are canceled in-flight before execution.
> 
> This occurs when core_tmr_drain_tmr_list() invokes cancel_work_sync()
> for the incoming se_tmr_req->task_cmd->work, resulting in cmd->work
> for target_tmr_work() never getting invoked and the aborted TMR
> waiting indefinately within transport_wait_for_tasks().
> 
> To address this case, perform a CMD_T_ABORTED check early in
> transport_generic_handle_tmr(), and invoke the normal path via
> transport_cmd_check_stop_to_fabric() to complete any TMR kthreads
> blocked waiting for CMD_T_STOP in transport_wait_for_tasks().
> 
> Also, move the TRANSPORT_ISTATE_PROCESSING assignment earlier
> into transport_generic_handle_tmr() so the existing check in
> core_tmr_drain_tmr_list() avoids attempting abort the incoming
> se_tmr_req->task_cmd->work if it has already been queued into
> se_device->tmr_wq.
> 
> Reported-by: Rob Millner 
> Tested-by: Rob Millner 
> Cc: Rob Millner 
> Cc: sta...@vger.kernel.org # 3.14+
> Signed-off-by: Nicholas Bellinger 
> ---
>  drivers/target/target_core_transport.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_transport.c 
> b/drivers/target/target_core_transport.c
> index 1cadc9e..8b69843 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -3110,7 +3110,6 @@ static void target_tmr_work(struct work_struct *work)
>   spin_unlock_irqrestore(>t_state_lock, flags);
>   goto check_stop;
>   }
> - cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
>   spin_unlock_irqrestore(>t_state_lock, flags);
>  
>   cmd->se_tfo->queue_tm_rsp(cmd);
> @@ -3123,11 +3122,25 @@ int transport_generic_handle_tmr(
>   struct se_cmd *cmd)
>  {
>   unsigned long flags;
> + bool aborted = false;
>  
>   spin_lock_irqsave(>t_state_lock, flags);
> - cmd->transport_state |= CMD_T_ACTIVE;
> + if (cmd->transport_state & CMD_T_ABORTED) {
> + aborted = true;
> + } else {
> + cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
> + cmd->transport_state |= CMD_T_ACTIVE;
> + }
>   spin_unlock_irqrestore(>t_state_lock, flags);
>  
> + if (aborted) {
> + pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d"
> + "ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function,
> + cmd->se_tmr_req->ref_task_tag, cmd->tag);

This seems pretty noisy for something that could happen during normal
operation.  Otherwise:

Reviewed-by: Christoph Hellwig 

Re: [PATCH 2/5] target: Use correct SCSI status during EXTENDED_COPY exception

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig 

Re: [PATCH][V2] scsi: aacraid: rcode is unsigned and should be signed int

[Martin K. Petersen]

> "Colin" == Colin King  writes:

Colin> aac_fib_send can return -ve error returns and hence rcode should
Colin> be signed. Currently the rcode >= 0 check is always true and -ve
Colin> errors are not being checked.

Colin> Thanks to Dan Carpenter for spotting my original broken fix to
Colin> this issue.

Applied to 4.11/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 13/39] megaraid_sas : set residual bytes count during IO compeltion

[Martin K. Petersen]

> "Kashyap" == Kashyap Desai  writes:

Kashyap,

Kashyap> Data length will be always guaranteed to be a multiple
Kashyap> of the logical block size until and unless we have some
Kashyap> firmware defect.  In past, We have seen some partial/complete
Kashyap> DMA data length return from firmware was not aligned with
Kashyap> logical block size. Eventually, root caused + fixed in
Kashyap> firmware.

Great. Thanks for confirming!

-- 
Martin K. Petersen  Oracle Linux Engineering

RE: [PATCH] scsi: aacraid: avoid open-coded upper_32_bits

[Raghava Aditya Renukunta]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, February 7, 2017 5:00 AM
> To: dl-esc-Aacraid Linux Driver ; James E.J.
> Bottomley ; Martin K. Petersen
> 
> Cc: Arnd Bergmann ; Johannes Thumshirn
> ; Raghava Aditya Renukunta
> ; Dave Carroll
> ; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] scsi: aacraid: avoid open-coded upper_32_bits
> 
> EXTERNAL EMAIL
> 
> 
> Shifting a dma_addr_t right by 32 bits causes a compile-time warning when
> that type is only 32 bit wide:
> 
> drivers/scsi/aacraid/src.c: In function 'aac_src_start_adapter':
> drivers/scsi/aacraid/src.c:414:29: error: right shift count >= width of type 
> [-
> Werror=shift-count-overflow]
> 
> This changes the driver to use the predefined macros consistently, including
> one correct but open-coded upper_32_bits() instance.
> 
> Fixes: d1ef4da8487f ("scsi: aacraid: added support for init_struct_8")
> Fixes: 423400e64d37 ("scsi: aacraid: Include HBA direct interface")
> Signed-off-by: Arnd Bergmann 
> ---

Reviewed-by: Raghava Aditya Renukunta 

RE: [PATCH][V2] scsi: aacraid: rcode is unsigned and should be signed int

[Raghava Aditya Renukunta]

> -Original Message-
> From: Colin King [mailto:colin.k...@canonical.com]
> Sent: Tuesday, February 7, 2017 3:51 AM
> To: dl-esc-Aacraid Linux Driver ; James E . J .
> Bottomley ; Martin K . Petersen
> ; linux-scsi@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH][V2] scsi: aacraid: rcode is unsigned and should be signed int
> 
> EXTERNAL EMAIL
> 
> 
> From: Colin Ian King 
> 
> aac_fib_send can return -ve error returns and hence rcode should
> be signed. Currently the rcode >= 0 check is always true and -ve
> errors are not being checked.
> 
> Thanks to Dan Carpenter for spotting my original broken fix to this
> issue.
> 
> Signed-off-by: Colin Ian King 
> ---

Reviewed-by: Raghava Aditya Renukunta 

Re: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination

[Willy Tarreau]

On Tue, Feb 07, 2017 at 09:02:51AM -0800, James Bottomley wrote:
> On Tue, 2017-02-07 at 07:59 +0100, Willy Tarreau wrote:
> > Hi James,
> > 
> > On Mon, Feb 06, 2017 at 10:38:48PM -0800, James Bottomley wrote:
> > > On Mon, 2017-02-06 at 23:26 +0100, Willy Tarreau wrote:
> > (...)
> > > > We don't have the referenced commit above in 3.10 so we should be
> > > > safe. Additionally I checked that neither 4.4 nor 3.12 have them 
> > > > either, so that makes me feel confident that we can skip it in 
> > > > 3.10 as well.
> > > 
> > > The original was also racy with respect to multiple commands, so 
> > > the above fixed the race as well.
> > 
> > OK so I tried to backport it to 3.10. I dropped a few parts which 
> > were addressing this one marked for stable 4.4+ :
> > 7ff723a ("scsi: mpt3sas: Unblock device after controller reset")
> > 
> > And I got the attached patch. All I know is that it builds. I'd 
> > appreciate it if someone could confirm its validity, in which case
> > I'll add it.
> 
> The two patches apply without fuzz to your tree and the combination is
> a far better bug fix than the original regardless of whether 7ff723a
> exists in your tree or not.  By messing with the patches all you do is
> add the potential for introducing new bugs for no benefit, so why take
> risk for no upside?

Just because I'm suggested to apply this fix which is supposed to fix
a regression brought by 7ff723a which itself is marked to fix 4.4+ only
and which doesn't apply to 3.10. So now I'm getting confused because
you say that these patches apply without fuzz but one part definitely
is rejected and the other one has to be applied by hand. I want not
to take a risk but I'm faced with these options :
  - drop all these patches and stay as 3.10.104 is
  - merge the "secure erase premature" + the the part of the patch
that supposedly fixes the regression it introduced
  - merge this fix + 7ff723a + whatever it depends on (not fond of
it)

In all cases I don't even have the hardware to validate anything. I'd
be more tempted with the first two options. If you think I'm taking
risks by backporting the relevant part of the fix, I'll simply drop
them all and leave the code as it is now.

Thanks,
Willy

Re: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination

[James Bottomley]

On Tue, 2017-02-07 at 07:59 +0100, Willy Tarreau wrote:
> Hi James,
> 
> On Mon, Feb 06, 2017 at 10:38:48PM -0800, James Bottomley wrote:
> > On Mon, 2017-02-06 at 23:26 +0100, Willy Tarreau wrote:
> (...)
> > > We don't have the referenced commit above in 3.10 so we should be
> > > safe. Additionally I checked that neither 4.4 nor 3.12 have them 
> > > either, so that makes me feel confident that we can skip it in 
> > > 3.10 as well.
> > 
> > The original was also racy with respect to multiple commands, so 
> > the above fixed the race as well.
> 
> OK so I tried to backport it to 3.10. I dropped a few parts which 
> were addressing this one marked for stable 4.4+ :
> 7ff723a ("scsi: mpt3sas: Unblock device after controller reset")
> 
> And I got the attached patch. All I know is that it builds. I'd 
> appreciate it if someone could confirm its validity, in which case
> I'll add it.

The two patches apply without fuzz to your tree and the combination is
a far better bug fix than the original regardless of whether 7ff723a
exists in your tree or not.  By messing with the patches all you do is
add the potential for introducing new bugs for no benefit, so why take
risk for no upside?

James

Re: [Lsf-pc] LSF/MM Question

[James Bottomley]

On Tue, 2017-02-07 at 16:09 +, Jim Mostek via Lsf-pc wrote:
> wondering about the upcoming Linux Storage Filesystem & MM summint in
> March. LSF/MM Question
> 
> What presentations are there so far?

LSF/MM is not really a conference, it's a summit.  That means it's
going to be discussion driven rather than presentation driven.  The
discussion agenda will be fluid (because issues come up as other things
are discussed) but the outline should begin to take shape a couple of
weeks beforehand.

James

Re: [PATCH 00/10] mpt3sas: full mq support

[Hannes Reinecke]

On 02/07/2017 04:40 PM, Christoph Hellwig wrote:
> On Tue, Feb 07, 2017 at 04:39:01PM +0100, Hannes Reinecke wrote:
>> But we do; we're getting the index/tag/smid from the high-priority list,
>> which is separated from the normal SCSI I/O tag space.
>> (which reminds me; there's another cleanup patch to be had in
>> _ctl_do_mpt_command(), but that's beside the point).
> 
> The calls to blk_mq_tagset_busy_iter added in patch 8 indicate the
> contrary.
> 
Right. Now I see what you mean.
We should have used reserved_tags here.
Sadly we still don't have an interface on actually _allocate_ reserved
tags, have we?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 00/10] mpt3sas: full mq support

[Christoph Hellwig]

On Tue, Feb 07, 2017 at 04:39:01PM +0100, Hannes Reinecke wrote:
> But we do; we're getting the index/tag/smid from the high-priority list,
> which is separated from the normal SCSI I/O tag space.
> (which reminds me; there's another cleanup patch to be had in
> _ctl_do_mpt_command(), but that's beside the point).

The calls to blk_mq_tagset_busy_iter added in patch 8 indicate the
contrary.

Re: [PATCH 00/10] mpt3sas: full mq support

[Hannes Reinecke]

On 02/07/2017 04:34 PM, Christoph Hellwig wrote:
> On Tue, Feb 07, 2017 at 03:38:51PM +0100, Hannes Reinecke wrote:
>> The SCSI passthrough commands pass in pre-formatted SGLs, so the driver
>> just has to map them.
>> If we were converting that we first have to re-format the
>> (driver-specific) SGLs into linux sg lists, only to have them converted
>> back into driver-specific ones once queuecommand is called.
>> You sure it's worth the effort?
>>
>> The driver already reserves some tags for precisely this use-case, so it
>> won't conflict with normal I/O operation.
>> So where's the problem with that?
> 
> If it was an entirely separate path that would be easy, but it's
> not - see all the poking into the tag maps that your patch 8
> includes.  If it was just a few tags on the side not interacting
> with the scsi or blk-mq it wouldn't be such a problem.
> 
But we do; we're getting the index/tag/smid from the high-priority list,
which is separated from the normal SCSI I/O tag space.
(which reminds me; there's another cleanup patch to be had in
_ctl_do_mpt_command(), but that's beside the point).

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 00/10] mpt3sas: full mq support

[Christoph Hellwig]

On Tue, Feb 07, 2017 at 03:38:51PM +0100, Hannes Reinecke wrote:
> The SCSI passthrough commands pass in pre-formatted SGLs, so the driver
> just has to map them.
> If we were converting that we first have to re-format the
> (driver-specific) SGLs into linux sg lists, only to have them converted
> back into driver-specific ones once queuecommand is called.
> You sure it's worth the effort?
> 
> The driver already reserves some tags for precisely this use-case, so it
> won't conflict with normal I/O operation.
> So where's the problem with that?

If it was an entirely separate path that would be easy, but it's
not - see all the poking into the tag maps that your patch 8
includes.  If it was just a few tags on the side not interacting
with the scsi or blk-mq it wouldn't be such a problem.

[patch] aacraid: information leak in aac_send_raw_srb()

[Dan Carpenter]

The aac_srb_reply struct ends in a 2 byte hole so we end up leaking a
bit of information to user space.

Fixes: 423400e64d37 ("scsi: aacraid: Include HBA direct interface")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 614842a9eb07..12dc867b7c74 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -948,6 +948,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void 
__user * arg)
&((struct aac_native_hba *)srbfib->hw_fib_va)->resp.err;
struct aac_srb_reply reply;
 
+   memset(, 0, sizeof(reply));
reply.status = ST_OK;
if (srbfib->flags & FIB_CONTEXT_FLAG_FASTRESP) {
/* fast response */

[PATCH -next] scsi: qedi: Fix possible memory leak in qedi_iscsi_update_conn()

[Wei Yongjun]

From: Wei Yongjun 

'conn_info' is malloced in qedi_iscsi_update_conn() and should be
freed before leaving from the error handling cases, otherwise it
will cause memory leak.

Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
driver framework.")
Signed-off-by: Wei Yongjun 
---
 drivers/scsi/qedi/qedi_iscsi.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index d6a2054..eb64469 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -453,13 +453,9 @@ static int qedi_iscsi_update_conn(struct qedi_ctx *qedi,
if (rval) {
rval = -ENXIO;
QEDI_ERR(>dbg_ctx, "Could not update connection\n");
-   goto update_conn_err;
}
 
kfree(conn_info);
-   rval = 0;
-
-update_conn_err:
return rval;
 }

Re: [PATCH 00/10] mpt3sas: full mq support

[Hannes Reinecke]

On 02/07/2017 02:19 PM, Christoph Hellwig wrote:
> Patch 1-7 look fine to me with minor fixups, and I'd love to see
> them go into 4.11.  The last one looks really questionable,
> and 8 and 9 will need some work so that the MPT passthrough ioctls
> either go away or make use of struct request and the block layer
> and SCSI infrastructure.
> 
Hmm. Which is quite a bit of effort for very little gain.

The SCSI passthrough commands pass in pre-formatted SGLs, so the driver
just has to map them.
If we were converting that we first have to re-format the
(driver-specific) SGLs into linux sg lists, only to have them converted
back into driver-specific ones once queuecommand is called.
You sure it's worth the effort?

The driver already reserves some tags for precisely this use-case, so it
won't conflict with normal I/O operation.
So where's the problem with that?

I know the SCSI passthrough operations are decidedly ugly, but if I were
to change them I'd rather move them over to bsg once we converted bsg to
operate without a request queue.
But for now ... not sure.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH RFC] enclosure: fix symlinks creation

[Maurizio Lombardi]

James,

one of our customers complains that in some cases, when using multipath,
the enclosure driver fails to create the symlinks in sysfs.

This is an example of what happens:

[   19.251902] scsi 0:0:27:0: Direct-Access SEAGATE  ST8000NM0075 E002 
PQ: 0 ANSI: 6
[   19.261874] scsi 0:0:27:0: SSP: handle(0x0027), 
sas_addr(0x5000c50085826dd6), phy(34), device_name(0x5000c50085826dd4)
[   19.274656] scsi 0:0:27:0: SSP: enclosure_logical_id(0x500093d001be4000), 
slot(57)
[   19.283944] scsi 0:0:27:0: SSP: enclosure level(0x), connector name( 
)
[   19.292933] scsi 0:0:27:0: qdepth(254), tagged(1), simple(0), ordered(0), 
scsi_l
[...]
[   60.066524] enclosure_add_device(0:0:27:0) called, enclosure_add_links() 
failed with error -2
[...]
[   75.119146] sd 0:0:27:0: Attached scsi generic sg27 type 0
[   75.126722] sd 0:0:27:0: [sdaa] 15628053168 512-byte logical blocks: (8.00 
TB/7.27 TiB)
[   75.126723] sd 0:0:27:0: [sdaa] 4096-byte physical blocks
[   75.129059] sd 0:0:27:0: [sdaa] Write Protect is off
[   75.129061] sd 0:0:27:0: [sdaa] Mode Sense: db 00 10 08
[   75.130417] sd 0:0:27:0: [sdaa] Write cache: enabled, read cache: enabled, 
supports DPO and FUA
[   75.158088] sd 0:0:27:0: [sdaa] Attached SCSI disk
[...]
[   75.192722] enclosure_add_device(0:0:27:0) called, device already exists

The first time enclosure_add_device() is called, the symlinks creation failed.
The second time it would have succeeded, but the enclosure_add_device() 
function returned immediately
without retrying to create the symlinks.

Maurizio Lombardi (1):
  enclosure: fix sysfs symlinks creation when using multipath

 drivers/misc/enclosure.c  | 16 ++--
 include/linux/enclosure.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
Maurizio Lombardi

[PATCH RFC] enclosure: fix sysfs symlinks creation when using multipath

[Maurizio Lombardi]

With multipath, it may happen that the same device is passed
to enclosure_add_device() multiple times and that the enclosure_add_links()
function fails to create the symlinks because the device's sysfs
directory entry is still NULL.
In this case, the links will never be created because all the subsequent
calls to enclosure_add_device() will immediately fail with EEXIST.

This patch modifies the code so the driver will detect this condition
and will retry to create the symlinks when enclosure_add_device() is called.

Signed-off-by: Maurizio Lombardi 
---
 drivers/misc/enclosure.c  | 16 ++--
 include/linux/enclosure.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 65fed71..a856c98 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -375,21 +375,33 @@ int enclosure_add_device(struct enclosure_device *edev, 
int component,
 struct device *dev)
 {
struct enclosure_component *cdev;
+   int error;
 
if (!edev || component >= edev->components)
return -EINVAL;
 
cdev = >component[component];
 
-   if (cdev->dev == dev)
+   if (cdev->dev == dev) {
+   if (!cdev->links_created) {
+   error = enclosure_add_links(cdev);
+   if (!error)
+   cdev->links_created = 1;
+   }
return -EEXIST;
+   }
 
if (cdev->dev)
enclosure_remove_links(cdev);
 
put_device(cdev->dev);
cdev->dev = get_device(dev);
-   return enclosure_add_links(cdev);
+   error = enclosure_add_links(cdev);
+   if (!error)
+   cdev->links_created = 1;
+   else
+   cdev->links_created = 0;
+   return error;
 }
 EXPORT_SYMBOL_GPL(enclosure_add_device);
 
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index a4cf57c..c3bdc4c 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -97,6 +97,7 @@ struct enclosure_component {
struct device cdev;
struct device *dev;
enum enclosure_component_type type;
+   int links_created;
int number;
int fault;
int active;
-- 
Maurizio Lombardi

[PATCH] scsi: aacraid: fix information leak on hbainfo.driver_name

[Colin King]

From: Colin Ian King 

The driver_name field is not initialized and hence information
on the stack is being leaked to userspace on the copy_to_user.
Fix this.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/aacraid/commctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 614842a..eb48d0a 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -1015,7 +1015,7 @@ static int aac_get_pci_info(struct aac_dev* dev, void 
__user *arg)
 
 static int aac_get_hba_info(struct aac_dev *dev, void __user *arg)
 {
-   struct aac_hba_info hbainfo;
+   struct aac_hba_info hbainfo = { 0 };
 
hbainfo.adapter_number  = (u8) dev->id;
hbainfo.system_io_bus_number= dev->pdev->bus->number;
-- 
2.10.2

[PATCH 1/5] target: Don't BUG_ON during NodeACL dynamic -> explicit conversion

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

After the v4.2+ RCU conversion to se_node_acl->lun_entry_hlist,
a BUG_ON() was added in core_enable_device_list_for_node() to
detect when the passed *lun does not match the existing
orig->se_lun pointer reference.

However, this scenario can occur happen when a dynamically
generated NodeACL is being converted to an explicit NodeACL,
when the explicit NodeACL contains a different LUN mapping
than the default provided by the WWN endpoint.

So instead of triggering BUG_ON(), go ahead and fail instead
following the original pre RCU conversion logic.

Reported-by: Benjamin ESTRABAUD 
Cc: Benjamin ESTRABAUD 
Cc: sta...@vger.kernel.org # 4.2+
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_device.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index 1ebd13e..23e89af 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -345,14 +345,22 @@ int core_enable_device_list_for_node(

lockdep_is_held(>lun_entry_mutex));
 
if (orig_lun != lun) {
-   pr_err("Existing orig->se_lun doesn't match new lun"
-  " for dynamic -> explicit NodeACL conversion:"
-   " %s\n", nacl->initiatorname);
+   pr_warn_ratelimited("Existing orig->se_lun doesn't 
match"
+   " new lun for dynamic -> explicit NodeACL"
+   " conversion: %s\n", nacl->initiatorname);
+   mutex_unlock(>lun_entry_mutex);
+   kfree(new);
+   return -EINVAL;
+   }
+   if (orig->se_lun_acl != NULL) {
+   pr_warn_ratelimited("Detected existing explicit"
+   " se_lun_acl->se_lun_group reference for %s"
+   " mapped_lun: %llu, ignoring\n",
+nacl->initiatorname, mapped_lun);
mutex_unlock(>lun_entry_mutex);
kfree(new);
return -EINVAL;
}
-   BUG_ON(orig->se_lun_acl != NULL);
 
rcu_assign_pointer(new->se_lun, lun);
rcu_assign_pointer(new->se_lun_acl, lun_acl);
-- 
1.9.1

[PATCH 2/5] target: Use correct SCSI status during EXTENDED_COPY exception

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

This patch adds the missing target_complete_cmd() SCSI status
parameter change in target_xcopy_do_work(), that was originally
missing in commit 926317de33.

It correctly propigates up the correct SCSI status during
EXTENDED_COPY exception cases, instead of always using the
hardcoded SAM_STAT_CHECK_CONDITION from original code.

This is required for ESX host environments that expect to
hit SAM_STAT_RESERVATION_CONFLICT for certain scenarios,
and SAM_STAT_CHECK_CONDITION results in non-retriable
status for these cases.

Reported-by: Nixon Vincent 
Tested-by: Nixon Vincent 
Cc: Nixon Vincent 
Cc: sta...@vger.kernel.org # 3.14+
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_xcopy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_xcopy.c 
b/drivers/target/target_core_xcopy.c
index d828b3b..cac5a20 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -864,7 +864,7 @@ static void target_xcopy_do_work(struct work_struct *work)
" CHECK_CONDITION -> sending response\n", rc);
ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
}
-   target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION);
+   target_complete_cmd(ec_cmd, ec_cmd->scsi_status);
 }
 
 sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
-- 
1.9.1

Re: [patch] scsi: qedi: silence sprintf() overflow warning

[walter harms]

Am 07.02.2017 14:01, schrieb Dan Carpenter:
> The problem here is this:
> 
>   sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no);
> 
> host_buf is 16 character so we only have 6 characters left for
> ->host_no.  But ->host_no is set in scsi_host_alloc():
> 
>   index = ida_simple_get(_index_ida, 0, 0, GFP_KERNEL);
> 
> It could theoretically go up to 0x800 so we need space for 10
> digits.
> 
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
> index 5eda21d903e9..0dcf3b08230c 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -1735,7 +1735,7 @@ static int __qedi_probe(struct pci_dev *pdev, int mode)
>   u32 dp_module = 0;
>   u8 dp_level = 0;
>   bool is_vf = false;
> - char host_buf[16];
> + char host_buf[20];
>   struct qed_link_params link_params;
>   struct qed_slowpath_params sp_params;
>   struct qed_probe_params qed_params;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

any chance to use snprintf here ?
 sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no);

or something like asprint() :)

if ever anyone change the type to very_long_type in the future it would simply 
break
but not hurt.

re,
 wh

[PATCH 4/5] target: Fix multi-session dynamic se_node_acl double free OOPs

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

This patch addresses a long-standing bug with multi-session
(eg: iscsi-target + iser-target) se_node_acl dynamic free
withini transport_deregister_session().

This bug is caused when a storage endpoint is configured with
demo-mode (generate_node_acls = 1 + cache_dynamic_acls = 1)
initiators, and initiator login creates a new dynamic node acl
and attaches two sessions to it.

After that, demo-mode for the storage instance is disabled via
configfs (generate_node_acls = 0 + cache_dynamic_acls = 0) and
the existing dynamic acl is never converted to an explicit ACL.

The end result is dynamic acl resources are released twice when
the sessions are shutdown in transport_deregister_session().

If the storage instance is not changed to disable demo-mode,
or the dynamic acl is converted to an explict ACL, or there
is only a single session associated with the dynamic ACL,
the bug is not triggered.

To address this big, move the release of dynamic se_node_acl
memory into target_complete_nacl() so it's only freed once
when se_node_acl->acl_kref reaches zero.

Reported-by: Rob Millner 
Tested-by: Rob Millner 
Cc: Rob Millner 
Cc: sta...@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_tpg.c   |  4 +-
 drivers/target/target_core_transport.c | 69 +-
 include/target/target_core_base.h  |  1 +
 3 files changed, 46 insertions(+), 28 deletions(-)

diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index d99752c..4f32c49 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -364,7 +364,7 @@ void core_tpg_del_initiator_node_acl(struct se_node_acl 
*acl)
mutex_lock(>acl_node_mutex);
if (acl->dynamic_node_acl)
acl->dynamic_node_acl = 0;
-   list_del(>acl_list);
+   list_del_init(>acl_list);
mutex_unlock(>acl_node_mutex);
 
target_shutdown_sessions(acl);
@@ -540,7 +540,7 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
 * in transport_deregister_session().
 */
list_for_each_entry_safe(nacl, nacl_tmp, _list, acl_list) {
-   list_del(>acl_list);
+   list_del_init(>acl_list);
 
core_tpg_wait_for_nacl_pr_ref(nacl);
core_free_device_list_for_node(nacl, se_tpg);
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 8b69843..bd62dd4 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -457,8 +457,20 @@ static void target_complete_nacl(struct kref *kref)
 {
struct se_node_acl *nacl = container_of(kref,
struct se_node_acl, acl_kref);
+   struct se_portal_group *se_tpg = nacl->se_tpg;
 
-   complete(>acl_free_comp);
+   if (!nacl->dynamic_stop) {
+   complete(>acl_free_comp);
+   return;
+   }
+
+   mutex_lock(_tpg->acl_node_mutex);
+   list_del_init(>acl_list);
+   mutex_unlock(_tpg->acl_node_mutex);
+
+   core_tpg_wait_for_nacl_pr_ref(nacl);
+   core_free_device_list_for_node(nacl, se_tpg);
+   kfree(nacl);
 }
 
 void target_put_nacl(struct se_node_acl *nacl)
@@ -499,12 +511,39 @@ void transport_deregister_session_configfs(struct 
se_session *se_sess)
 void transport_free_session(struct se_session *se_sess)
 {
struct se_node_acl *se_nacl = se_sess->se_node_acl;
+
/*
 * Drop the se_node_acl->nacl_kref obtained from within
 * core_tpg_get_initiator_node_acl().
 */
if (se_nacl) {
+   struct se_portal_group *se_tpg = se_nacl->se_tpg;
+   const struct target_core_fabric_ops *se_tfo = 
se_tpg->se_tpg_tfo;
+   unsigned long flags;
+
se_sess->se_node_acl = NULL;
+
+   /*
+* Also determine if we need to drop the extra ->cmd_kref if
+* it had been previously dynamically generated, and
+* the endpoint is not caching dynamic ACLs.
+*/
+   mutex_lock(_tpg->acl_node_mutex);
+   if (se_nacl->dynamic_node_acl &&
+   !se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
+   spin_lock_irqsave(_nacl->nacl_sess_lock, flags);
+   if (list_empty(_nacl->acl_sess_list))
+   se_nacl->dynamic_stop = true;
+   spin_unlock_irqrestore(_nacl->nacl_sess_lock, flags);
+
+   if (se_nacl->dynamic_stop)
+   list_del_init(_nacl->acl_list);
+   }
+   mutex_unlock(_tpg->acl_node_mutex);
+
+   if (se_nacl->dynamic_stop)
+   target_put_nacl(se_nacl);
+

[PATCH 3/5] target: Fix early transport_generic_handle_tmr abort scenario

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

This patch fixes a bug where incoming task management requests
can be explicitly aborted during an active LUN_RESET, but who's
struct work_struct are canceled in-flight before execution.

This occurs when core_tmr_drain_tmr_list() invokes cancel_work_sync()
for the incoming se_tmr_req->task_cmd->work, resulting in cmd->work
for target_tmr_work() never getting invoked and the aborted TMR
waiting indefinately within transport_wait_for_tasks().

To address this case, perform a CMD_T_ABORTED check early in
transport_generic_handle_tmr(), and invoke the normal path via
transport_cmd_check_stop_to_fabric() to complete any TMR kthreads
blocked waiting for CMD_T_STOP in transport_wait_for_tasks().

Also, move the TRANSPORT_ISTATE_PROCESSING assignment earlier
into transport_generic_handle_tmr() so the existing check in
core_tmr_drain_tmr_list() avoids attempting abort the incoming
se_tmr_req->task_cmd->work if it has already been queued into
se_device->tmr_wq.

Reported-by: Rob Millner 
Tested-by: Rob Millner 
Cc: Rob Millner 
Cc: sta...@vger.kernel.org # 3.14+
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_transport.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 1cadc9e..8b69843 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -3110,7 +3110,6 @@ static void target_tmr_work(struct work_struct *work)
spin_unlock_irqrestore(>t_state_lock, flags);
goto check_stop;
}
-   cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
spin_unlock_irqrestore(>t_state_lock, flags);
 
cmd->se_tfo->queue_tm_rsp(cmd);
@@ -3123,11 +3122,25 @@ int transport_generic_handle_tmr(
struct se_cmd *cmd)
 {
unsigned long flags;
+   bool aborted = false;
 
spin_lock_irqsave(>t_state_lock, flags);
-   cmd->transport_state |= CMD_T_ACTIVE;
+   if (cmd->transport_state & CMD_T_ABORTED) {
+   aborted = true;
+   } else {
+   cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
+   cmd->transport_state |= CMD_T_ACTIVE;
+   }
spin_unlock_irqrestore(>t_state_lock, flags);
 
+   if (aborted) {
+   pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d"
+   "ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function,
+   cmd->se_tmr_req->ref_task_tag, cmd->tag);
+   transport_cmd_check_stop_to_fabric(cmd);
+   return 0;
+   }
+
INIT_WORK(>work, target_tmr_work);
queue_work(cmd->se_dev->tmr_wq, >work);
return 0;
-- 
1.9.1

[PATCH 0/5] target: Miscellaneous bug-fixes for >= v4.10

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

Hi all,

This series contains a handful of bug-fixes that I've been validating
on v4.1.y code for production usage over the past couple of months.

All of these are long-standing issues that I don't think other
folks have been able to hit (or at least not reported), but have
been reproduced by Datera's QA team and/or have been reported by
customers.

Most of the patches are straight-forward fixes have been running
in Datera's nightly automation for weeks to months.

The one exception in patch #1 is a >= v4.2 RCU regression bug-fix
reported by Benjamin Estrabaud a while back, that re-instates
pre RCU conversion logic that kills a bogus BUG_ON() during
dynamic -> explicit se_node_acl conversion.

Please review.

--nab

Nicholas Bellinger (5):
  target: Don't BUG_ON during NodeACL dynamic -> explicit conversion
  target: Use correct SCSI status during EXTENDED_COPY exception
  target: Fix early transport_generic_handle_tmr abort scenario
  target: Fix multi-session dynamic se_node_acl double free OOPs
  target: Fix COMPARE_AND_WRITE ref leak for non GOOD status

 drivers/target/target_core_device.c| 16 +--
 drivers/target/target_core_sbc.c   |  8 +++-
 drivers/target/target_core_tpg.c   |  4 +-
 drivers/target/target_core_transport.c | 86 +++---
 drivers/target/target_core_xcopy.c |  2 +-
 include/target/target_core_base.h  |  1 +
 6 files changed, 80 insertions(+), 37 deletions(-)

-- 
1.9.1

[PATCH 5/5] target: Fix COMPARE_AND_WRITE ref leak for non GOOD status

[Nicholas A. Bellinger]

From: Nicholas Bellinger 

This patch addresses a long standing bug where the commit phase
of COMPARE_AND_WRITE would result in a se_cmd->cmd_kref reference
leak if se_cmd->scsi_status returned non SAM_STAT_GOOD.

This would manifest first as a lost SCSI response, and eventual
hung task during fabric driver logout or re-login, as existing
shutdown logic waited for the COMPARE_AND_WRITE se_cmd->cmd_kref
to reach zero.

To address this bug, compare_and_write_post() has been changed
to drop the incorrect !cmd->scsi_status conditional that was
preventing *post_ret = 1 for being set during non SAM_STAT_GOOD
status.

This patch has been tested with SAM_STAT_CHECK_CONDITION status
from normal target_complete_cmd() callback path, as well as the
incoming __target_execute_cmd() submission failure path when
se_cmd->execute_cmd() returns non zero status.

Reported-by: Donald White 
Cc: Donald White 
Tested-by: Gary Guo 
Cc: Gary Guo 
Cc:  # v3.12+
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_sbc.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 4879e70..df7b6e9 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -451,6 +451,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd 
*cmd, bool success,
 int *post_ret)
 {
struct se_device *dev = cmd->se_dev;
+   sense_reason_t ret = TCM_NO_SENSE;
 
/*
 * Only set SCF_COMPARE_AND_WRITE_POST to force a response fall-through
@@ -458,9 +459,12 @@ static sense_reason_t compare_and_write_post(struct se_cmd 
*cmd, bool success,
 * sent to the backend driver.
 */
spin_lock_irq(>t_state_lock);
-   if ((cmd->transport_state & CMD_T_SENT) && !cmd->scsi_status) {
+   if (cmd->transport_state & CMD_T_SENT) {
cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
*post_ret = 1;
+
+   if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION)
+   ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
spin_unlock_irq(>t_state_lock);
 
@@ -470,7 +474,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd 
*cmd, bool success,
 */
up(>caw_sem);
 
-   return TCM_NO_SENSE;
+   return ret;
 }
 
 static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool 
success,
-- 
1.9.1

Re: [PATCH 01/10] mpt3sas: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

Looks fine:

Reviewed-by: Christoph Hellwig 

Btw, I think the !smp_affinity_enable path should just go away
sooner or later.

Re: [PATCH 07/10] mpt3sas: use hi-priority queue for TMFs

[Christoph Hellwig]

On Tue, Jan 31, 2017 at 10:25:57AM +0100, Hannes Reinecke wrote:
> When sending a TMF via the ioctl interface we should be using
> the hi-priority queue instead of the scsi queue to be consistent
> with overall TMF usage.

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 00/10] mpt3sas: full mq support

[Christoph Hellwig]

Patch 1-7 look fine to me with minor fixups, and I'd love to see
them go into 4.11.  The last one looks really questionable,
and 8 and 9 will need some work so that the MPT passthrough ioctls
either go away or make use of struct request and the block layer
and SCSI infrastructure.

Re: [PATCH 03/10] mpt3sas: implement _dechain_st()

[Hannes Reinecke]

On 02/07/2017 02:15 PM, Christoph Hellwig wrote:
> On Tue, Jan 31, 2017 at 10:25:53AM +0100, Hannes Reinecke wrote:
>> Split off _dechain_st() as separate function.
>> No functional change.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c | 23 ++-
>>  1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
>> b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 09d0008..120b317 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -2365,6 +2365,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
>>  return smid;
>>  }
>>  
>> +static void
>> +_dechain_st(struct MPT3SAS_ADAPTER *ioc, struct scsiio_tracker *st)
>> +{
>> +struct chain_tracker *chain_req;
>> +
>> +while (!list_empty(>chain_list)) {
>> +chain_req = list_first_entry(>chain_list,
>> + struct chain_tracker,
>> + tracker_list);
>> +list_move(_req->tracker_list, >free_chain_list);
>> +}
> 
> Why not just use list_splice_init?
> 
Yep, can do.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 06/10] mpt3sas: Introduce mpt3sas_get_st_from_smid()

[Christoph Hellwig]

>  inline u8
>  mpt3sas_scsi_direct_io_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
>  {
> - return ioc->scsi_lookup[smid - 1].direct_io;
> + struct scsiio_tracker *st = mpt3sas_get_st_from_smid(ioc, smid);
> +
> + return st ? st->direct_io : 0;

This wrapper can go away and be merged into the only caller.

Otherwise looks fine:

Reviewed-by: Christoph Hellwig 

Re: [PATCH 05/10] mpt3sas: open-code _scsih_scsi_lookup_get()

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 03/10] mpt3sas: implement _dechain_st()

[Christoph Hellwig]

On Tue, Jan 31, 2017 at 10:25:53AM +0100, Hannes Reinecke wrote:
> Split off _dechain_st() as separate function.
> No functional change.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 09d0008..120b317 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2365,6 +2365,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
>   return smid;
>  }
>  
> +static void
> +_dechain_st(struct MPT3SAS_ADAPTER *ioc, struct scsiio_tracker *st)
> +{
> + struct chain_tracker *chain_req;
> +
> + while (!list_empty(>chain_list)) {
> + chain_req = list_first_entry(>chain_list,
> +  struct chain_tracker,
> +  tracker_list);
> + list_move(_req->tracker_list, >free_chain_list);
> + }

Why not just use list_splice_init?

Re: [PATCH 02/10] mpt3sas: set default value for cb_idx

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 04/10] mpt3sas: separate out _base_recovery_check()

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig 

[patch] scsi: qedi: silence sprintf() overflow warning

[Dan Carpenter]

The problem here is this:

sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no);

host_buf is 16 character so we only have 6 characters left for
->host_no.  But ->host_no is set in scsi_host_alloc():

index = ida_simple_get(_index_ida, 0, 0, GFP_KERNEL);

It could theoretically go up to 0x800 so we need space for 10
digits.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 5eda21d903e9..0dcf3b08230c 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -1735,7 +1735,7 @@ static int __qedi_probe(struct pci_dev *pdev, int mode)
u32 dp_module = 0;
u8 dp_level = 0;
bool is_vf = false;
-   char host_buf[16];
+   char host_buf[20];
struct qed_link_params link_params;
struct qed_slowpath_params sp_params;
struct qed_probe_params qed_params;

Re: [PATCH] scsi: aacraid: avoid open-coded upper_32_bits

[Johannes Thumshirn]

On 02/07/2017 01:59 PM, Arnd Bergmann wrote:
> Shifting a dma_addr_t right by 32 bits causes a compile-time warning when
> that type is only 32 bit wide:
> 
> drivers/scsi/aacraid/src.c: In function 'aac_src_start_adapter':
> drivers/scsi/aacraid/src.c:414:29: error: right shift count >= width of type 
> [-Werror=shift-count-overflow]
> 
> This changes the driver to use the predefined macros consistently, including
> one correct but open-coded upper_32_bits() instance.
> 
> Fixes: d1ef4da8487f ("scsi: aacraid: added support for init_struct_8")
> Fixes: 423400e64d37 ("scsi: aacraid: Include HBA direct interface")
> Signed-off-by: Arnd Bergmann 
> ---

The coolest thing with your patches is, one gets to know new cool
functions/macros.

Anyways looks good,
Reviewed-by: Johannes Thumshirn 


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH] scsi: aacraid: avoid open-coded upper_32_bits

[Arnd Bergmann]

Shifting a dma_addr_t right by 32 bits causes a compile-time warning when
that type is only 32 bit wide:

drivers/scsi/aacraid/src.c: In function 'aac_src_start_adapter':
drivers/scsi/aacraid/src.c:414:29: error: right shift count >= width of type 
[-Werror=shift-count-overflow]

This changes the driver to use the predefined macros consistently, including
one correct but open-coded upper_32_bits() instance.

Fixes: d1ef4da8487f ("scsi: aacraid: added support for init_struct_8")
Fixes: 423400e64d37 ("scsi: aacraid: Include HBA direct interface")
Signed-off-by: Arnd Bergmann 
---
 drivers/scsi/aacraid/src.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 46976a3b6952..8e4e2ddbafd7 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -410,8 +410,8 @@ static void aac_src_start_adapter(struct aac_dev *dev)
if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE3) {
init->r8.host_elapsed_seconds = cpu_to_le32(get_seconds());
src_sync_cmd(dev, INIT_STRUCT_BASE_ADDRESS,
-   (u32)(ulong)dev->init_pa,
-   (u32)((ulong)dev->init_pa>>32),
+   lower_32_bits(dev->init_pa),
+   upper_32_bits(dev->init_pa),
sizeof(struct _r8) +
(AAC_MAX_HRRQ - 1) * sizeof(struct _rrq),
0, 0, 0, NULL, NULL, NULL, NULL, NULL);
@@ -563,7 +563,7 @@ static int aac_src_deliver_message(struct fib *fib)
fib->hw_fib_va->header.SenderFibAddress =
cpu_to_le32((u32)address);
fib->hw_fib_va->header.u.TimeStamp = 0;
-   WARN_ON(((u32)(((address) >> 16) >> 16)) != 0L);
+   WARN_ON(upper_32_bits(address) != 0L);
} else {
/* Calculate the amount to the fibsize bits */
fibsize = (sizeof(struct aac_fib_xporthdr) +
-- 
2.9.0

[bug report] scsi_dh_rdac: switch to scsi_execute_req_flags()

[Dan Carpenter]

Hello Hannes Reinecke,

The patch 327825574132: "scsi_dh_rdac: switch to
scsi_execute_req_flags()" from Nov 3, 2016, leads to the following
static checker warning:

drivers/scsi/device_handler/scsi_dh_rdac.c:551 send_mode_select()
error: potential NULL dereference 'ctlr->ms_sdev'.

drivers/scsi/device_handler/scsi_dh_rdac.c
   529  static void send_mode_select(struct work_struct *work)
   530  {
   531  struct rdac_controller *ctlr =
   532  container_of(work, struct rdac_controller, ms_work);
   533  struct scsi_device *sdev = ctlr->ms_sdev;
   534  struct rdac_dh_data *h = sdev->handler_data;
   535  int err = SCSI_DH_OK, retry_cnt = RDAC_RETRY_COUNT;
   536  struct rdac_queue_data *tmp, *qdata;
   537  LIST_HEAD(list);
   538  unsigned char cdb[COMMAND_SIZE(MODE_SELECT_10)];
   539  struct scsi_sense_hdr sshdr;
   540  unsigned int data_size;
   541  u64 req_flags = REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
   542  REQ_FAILFAST_DRIVER;
   543  
   544  spin_lock(>ms_lock);
   545  list_splice_init(>ms_head, );
   546  ctlr->ms_queued = 0;
   547  ctlr->ms_sdev = NULL;

We set this to NULL.

   548  spin_unlock(>ms_lock);
   549  
   550   retry:
   551  data_size = rdac_failover_get(ctlr, , cdb);
  
Then dereference it here inside this function.

   552  
   553  RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d, "
   554  "%s MODE_SELECT command",
   555  (char *) h->ctlr->array_name, h->ctlr->index,
   556  (retry_cnt == RDAC_RETRY_COUNT) ? "queueing" : 
"retrying");
   557  

regards,
dan carpenter

RE: [PATCH 18/39] megaraid_sas: MR_TargetIdToLdGet u8 to u16 and avoid invalid raid-map access

[Shivasharan Srikanteshwara]

> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.com]
> Sent: Monday, February 06, 2017 4:51 PM
> To: Shivasharan S; linux-scsi@vger.kernel.org
> Cc: martin.peter...@oracle.com; the...@redhat.com;
> j...@linux.vnet.ibm.com; kashyap.de...@broadcom.com;
> sumit.sax...@broadcom.com
> Subject: Re: [PATCH 18/39] megaraid_sas: MR_TargetIdToLdGet u8 to u16
and
> avoid invalid raid-map access
>
> On 02/06/2017 10:59 AM, Shivasharan S wrote:
> > If MR_TargetIdToLdGet return >= 0xFF, it is invalid entry.
> > Consider that entry as invalid and do not access raid map for further
> operation.
> >
> > Signed-off-by: Shivasharan S 
> > Signed-off-by: Kashyap Desai 
> > ---
> >  drivers/scsi/megaraid/megaraid_sas.h|  2 +-
> >  drivers/scsi/megaraid/megaraid_sas_fp.c |  5 +++--
> >  drivers/scsi/megaraid/megaraid_sas_fusion.c | 25
> > ++---
> >  3 files changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> > b/drivers/scsi/megaraid/megaraid_sas.h
> > index f023e23..ec5f003 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -2448,7 +2448,7 @@ MR_BuildRaidContext(struct megasas_instance
> *instance,
> > struct IO_REQUEST_INFO *io_info,
> > struct RAID_CONTEXT *pRAID_Context,
> > struct MR_DRV_RAID_MAP_ALL *map, u8 **raidLUN);
> > -u8 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL *map);
> > +u16 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL
> *map);
> >  struct MR_LD_RAID *MR_LdRaidGet(u32 ld, struct MR_DRV_RAID_MAP_ALL
> > *map);
> >  u16 MR_ArPdGet(u32 ar, u32 arm, struct MR_DRV_RAID_MAP_ALL *map);
> >  u16 MR_LdSpanArrayGet(u32 ld, u32 span, struct MR_DRV_RAID_MAP_ALL
> > *map); diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c
> > b/drivers/scsi/megaraid/megaraid_sas_fp.c
> > index a0b0e68..9d5d485 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
> > @@ -165,7 +165,7 @@ u16 MR_GetLDTgtId(u32 ld, struct
> MR_DRV_RAID_MAP_ALL *map)
> > return le16_to_cpu(map->raidMap.ldSpanMap[ld].ldRaid.targetId);
> >  }
> >
> > -u8 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL *map)
> > +u16 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL
> *map)
> >  {
> > return map->raidMap.ldTgtIdToLd[ldTgtId];
> >  }
> > @@ -1151,7 +1151,7 @@ MR_BuildRaidContext(struct megasas_instance
> > *instance,  {
> > struct fusion_context *fusion;
> > struct MR_LD_RAID  *raid;
> > -   u32 ld, stripSize, stripe_mask;
> > +   u32 stripSize, stripe_mask;
> > u64 endLba, endStrip, endRow, start_row, start_strip;
> > u64 regStart;
> > u32 regSize;
> > @@ -1163,6 +1163,7 @@ MR_BuildRaidContext(struct megasas_instance
> *instance,
> > u8  retval = 0;
> > u8  startlba_span = SPAN_INVALID;
> > u64 *pdBlock = _info->pdBlock;
> > +   u16 ld;
> >
> > ldStartBlock = io_info->ldStartBlock;
> > numBlocks = io_info->numBlocks;
> > diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > index 2f523f2..3d4d4b8 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > @@ -1120,7 +1120,8 @@ megasas_sync_map_info(struct megasas_instance
> *instance)
> > int i;
> > struct megasas_cmd *cmd;
> > struct megasas_dcmd_frame *dcmd;
> > -   u32 size_sync_info, num_lds;
> > +   u16 num_lds;
> > +   u32 size_sync_info;
> > struct fusion_context *fusion;
> > struct MR_LD_TARGET_SYNC *ci = NULL;
> > struct MR_DRV_RAID_MAP_ALL *map;
> > @@ -1867,7 +1868,7 @@ megasas_set_pd_lba(struct
> MPI2_RAID_SCSI_IO_REQUEST *io_request, u8 cdb_len,
> >struct MR_DRV_RAID_MAP_ALL *local_map_ptr, u32 ref_tag)
> {
> > struct MR_LD_RAID *raid;
> > -   u32 ld;
> > +   u16 ld;
> > u64 start_blk = io_info->pdBlock;
> > u8 *cdb = io_request->CDB.CDB32;
> > u32 num_blocks = io_info->numBlocks; @@ -2300,10 +2301,11 @@
> > megasas_build_ldio_fusion(struct megasas_instance *instance,
> >
> > local_map_ptr = fusion->ld_drv_map[(instance->map_id & 1)];
> > ld = MR_TargetIdToLdGet(device_id, local_map_ptr);
> > -   raid = MR_LdRaidGet(ld, local_map_ptr);
> >
> > -   if ((MR_TargetIdToLdGet(device_id, local_map_ptr) >=
> > -   instance->fw_supported_vd_count) ||
(!fusion->fast_path_io)) {
> > +   if (ld < instance->fw_supported_vd_count)
> > +   raid = MR_LdRaidGet(ld, local_map_ptr);
> > +
> > +   if (!raid || (!fusion->fast_path_io)) {
> > io_request->RaidContext.raid_context.reg_lock_flags  = 0;
> > fp_possible = false;
> > } else {
> > @@ -2475,12 +2477,12 @@ static void
> > 

[PATCH][V2] scsi: aacraid: rcode is unsigned and should be signed int

[Colin King]

From: Colin Ian King 

aac_fib_send can return -ve error returns and hence rcode should
be signed. Currently the rcode >= 0 check is always true and -ve
errors are not being checked.

Thanks to Dan Carpenter for spotting my original broken fix to this
issue.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/aacraid/aachba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 3b5ddf4..907f1e8 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1798,7 +1798,7 @@ int aac_report_phys_luns(struct aac_dev *dev, struct fib 
*fibptr, int rescan)
struct sgmap64 *sg64;
dma_addr_t addr;
u32 vbus, vid;
-   u32 rcode = 0;
+   int rcode = 0;
 
/* Thor SA Firmware -> CISS_REPORT_PHYSICAL_LUNS */
fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry)
-- 
2.10.2

Re: [PATCH] scsi: aacraid: rcode is unsigned, so can never be less than zero

[Colin Ian King]

On 07/02/17 11:37, Dan Carpenter wrote:
> On Tue, Feb 07, 2017 at 11:27:38AM +, Colin King wrote:
>> From: Colin Ian King 
>>
>> The check on rcode >= 0 is always true because rcode is unsigned
>> and can never be less than zero.  Remove the redundant check.
>>
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/scsi/aacraid/aachba.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
>> index 3b5ddf4..ddfd726 100644
>> --- a/drivers/scsi/aacraid/aachba.c
>> +++ b/drivers/scsi/aacraid/aachba.c
>> @@ -1848,7 +1848,7 @@ int aac_report_phys_luns(struct aac_dev *dev, struct 
>> fib *fibptr, int rescan)
>>  FsaNormal, 1, 1, NULL, NULL);
>>  
>>  /* analyse data */
>> -if (rcode >= 0 && phys_luns->resp_flag == 2) {
> 
> The original code is buggy.  rcode should be an int.

Thanks for spotting that. Oops.

> 
> regards,
> dan carpenter
> 

Re: [PATCH] scsi: aacraid: rcode is unsigned, so can never be less than zero

[Dan Carpenter]

On Tue, Feb 07, 2017 at 11:27:38AM +, Colin King wrote:
> From: Colin Ian King 
> 
> The check on rcode >= 0 is always true because rcode is unsigned
> and can never be less than zero.  Remove the redundant check.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/aacraid/aachba.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 3b5ddf4..ddfd726 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -1848,7 +1848,7 @@ int aac_report_phys_luns(struct aac_dev *dev, struct 
> fib *fibptr, int rescan)
>   FsaNormal, 1, 1, NULL, NULL);
>  
>   /* analyse data */
> - if (rcode >= 0 && phys_luns->resp_flag == 2) {

The original code is buggy.  rcode should be an int.

regards,
dan carpenter

[PATCH] scsi: aacraid: rcode is unsigned, so can never be less than zero

[Colin King]

From: Colin Ian King 

The check on rcode >= 0 is always true because rcode is unsigned
and can never be less than zero.  Remove the redundant check.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/aacraid/aachba.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 3b5ddf4..ddfd726 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1848,7 +1848,7 @@ int aac_report_phys_luns(struct aac_dev *dev, struct fib 
*fibptr, int rescan)
FsaNormal, 1, 1, NULL, NULL);
 
/* analyse data */
-   if (rcode >= 0 && phys_luns->resp_flag == 2) {
+   if (phys_luns->resp_flag == 2) {
/* ok and extended reporting */
aac_update_hba_map(dev, phys_luns, rescan);
}
-- 
2.10.2

RE: [PATCH 13/39] megaraid_sas : set residual bytes count during IO compeltion

[Kashyap Desai]

> -Original Message-
> From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
> Sent: Tuesday, February 07, 2017 5:22 AM
> To: Shivasharan S
> Cc: linux-scsi@vger.kernel.org; martin.peter...@oracle.com;
> the...@redhat.com; j...@linux.vnet.ibm.com;
> kashyap.de...@broadcom.com; sumit.sax...@broadcom.com;
> h...@suse.com
> Subject: Re: [PATCH 13/39] megaraid_sas : set residual bytes count
during IO
> compeltion
>
> > "Shivasharan" == Shivasharan S
>  writes:
>
> Shivasharan> Fixing issue of not setting residual bytes correctly.
>
> @@ -1464,6 +1465,15 @@ map_cmd_status(struct fusion_context *fusion,
>  SCSI_SENSE_BUFFERSIZE);
>   scmd->result |= DRIVER_SENSE << 24;
>   }
> +
> + /*
> +  * If the  IO request is partially completed, then MR FW
will
> +  * update "io_request->DataLength" field with actual
number
> of
> +  * bytes transferred.Driver will set residual bytes count
in
> +  * SCSI command structure.
> +  */
> + resid = (scsi_bufflen(scmd) - data_length);
> + scsi_set_resid(scmd, resid);
>
> Is data_length guaranteed to be a multiple of the logical block size?
> Otherwise you need to tweak the residual like we just did for mpt3sas.

Martin, Data length will be always guaranteed to be a multiple of the
logical block size until and unless we have some firmware defect.
In past, We have seen  some partial/complete DMA data length return from
firmware was not aligned with logical block size. Eventually, root caused
+ fixed in firmware.

>
> --
> Martin K. PetersenOracle Linux Engineering

Re: [PATCH v2 07/18] lpfc: NVME Initiator: Base modifications Part E

[Johannes Thumshirn]

On 02/07/2017 12:08 AM, James Smart wrote:
> 
> NVME Initiator: Base modifications
> 
> This is part E of parts A..F.
> 
> Part E is limited to lpfc_sli.c. This is the location of most of changes
> for the following:
> - sli3 ring vs sli4 wq splits
> - io abort interfaces
> - actual queuing routines and use of dma and sgl pools
> - service routines to create/delete queues
> 
> This patch by itself will not result in a compilable driver. All parts
> A..F must be applied to create a compilable driver.
> 
> *
> 
> Refer to Part A for a description of base modifications
> 
> Signed-off-by: Dick Kennedy 
> Signed-off-by: James Smart 
> 
> ---
> Modifications in V2:
> Note: this was patch 6 in the V1 patches
>  Address review items:
>   Reverted lpfc_sli_hbq_count() spurious change. Thus more files than
>  just lpfc_sli.c

Yes but patch  03/18 'lpfc: NVME Initiator: Base modifications Part A'
still has calls to lpfc_sli_hbq_count(phba) (and in fact introduces this
change).

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

RE: [PATCH 33/39] megaraid_sas: call flush_scheduled_work during controller shutdown/detach

[Kashyap Desai]

> -Original Message-
> From: Kashyap Desai [mailto:kashyap.de...@broadcom.com]
> Sent: Monday, February 06, 2017 10:48 PM
> To: 'Tomas Henzl'; Shivasharan Srikanteshwara;
'linux-scsi@vger.kernel.org'
> Cc: 'martin.peter...@oracle.com'; 'j...@linux.vnet.ibm.com'; Sumit
Saxena;
> 'h...@suse.com'
> Subject: RE: [PATCH 33/39] megaraid_sas: call flush_scheduled_work
during
> controller shutdown/detach
>
> > -Original Message-
> > From: Tomas Henzl [mailto:the...@redhat.com]
> > Sent: Monday, February 06, 2017 9:35 PM
> > To: Shivasharan S; linux-scsi@vger.kernel.org
> > Cc: martin.peter...@oracle.com; j...@linux.vnet.ibm.com;
> > kashyap.de...@broadcom.com; sumit.sax...@broadcom.com;
> h...@suse.com
> > Subject: Re: [PATCH 33/39] megaraid_sas: call flush_scheduled_work
> > during controller shutdown/detach
> >
> > On 6.2.2017 11:00, Shivasharan S wrote:
> > > Signed-off-by: Kashyap Desai 
> > > Signed-off-by: Shivasharan S
> > 
> > > ---
> > >  drivers/scsi/megaraid/megaraid_sas_base.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> > b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > index 04ef0a0..b29cfd3 100644
> > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > @@ -6393,6 +6393,7 @@ megasas_suspend(struct pci_dev *pdev,
> > pm_message_t state)
> > >   if (instance->ev != NULL) {
> > >   struct megasas_aen_event *ev = instance->ev;
> > >   cancel_delayed_work_sync(>hotplug_work);
> > > + flush_scheduled_work();
> > >   instance->ev = NULL;
> > >   }
> > >
> > > @@ -6619,6 +6620,7 @@ static void megasas_detach_one(struct pci_dev
> > *pdev)
> > >   if (instance->ev != NULL) {
> > >   struct megasas_aen_event *ev = instance->ev;
> > >   cancel_delayed_work_sync(>hotplug_work);
> > > + flush_scheduled_work();
> > >   instance->ev = NULL;
> > >   }
> > >
> >
> > Why is cancel_delayed_work_sync not good enough?
>
> Megaraid_sas driver use certain work on global work queue.
>
> Below are the listed one -
>
>   if (instance->ctrl_context) {
>   INIT_WORK(>work_init, megasas_fusion_ocr_wq);
>   INIT_WORK(>crash_init,
> megasas_fusion_crash_dump_wq);
>   }
>   else
>   INIT_WORK(>work_init,
> process_fw_state_change_wq)
>
> Cancel_delayed_work_sync() was mainly targeted for only hotplug AEN
work.
> Calling flush_scheduled_work() we want above listed work to be completed
> as well.

Tomas - Here is one more update. I agree with your assessment. We don't
need this patch.

In our local repo code was like below and as part of sync up activity, I
did not realize that upstream is using cancel_delayed_work_sync() which is
internally doing the same as below.

cancel_delayed_work(>hotplug_work);
flush_scheduled_work();

Just for info - Similar patch was posted for mpt2sas long time ago to
replace above combination with cancel_delayed_work_sync()

https://lkml.org/lkml/2010/12/21/127

We will accommodate removal of this patch in V2 submission.



>
> >
> > tomash

Re: [PATCH v2 02/18] lpfc: use pci_irq_alloc_vectors and pci_irq_free_vectors

[Johannes Thumshirn]

On 02/07/2017 12:08 AM, James Smart wrote:
> 
> I replaced the v1 patch with Christoph's original
> 
>   james
> 
> From: Christoph Hellwig 
> 
> This avoids having to store the msix_entries array and simpliefies the
> shutdown and cleanup path a lot.
> 
> Signed-off-by: Christoph Hellwig 
> Signed-off-by: James Smart 
> ---

Looks good,
Reviewed-by: Johannes Thumshirn 


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 01/18] lpfc: Correct WQ creation for pagesize

[Johannes Thumshirn]

On 02/07/2017 12:08 AM, James Smart wrote:
> 
> Correct WQ creation for pagesize
> 
> The driver was calculating the adapter command pagesize indicator from
> the system pagesize. However, the buffers the driver allocates are only
> one size (SLI4_PAGE_SIZE), so no calculation was necessary.
> 
> Signed-off-by: Dick Kennedy 
> Signed-off-by: James Smart 
> ---
> Note: this patch was not in v1
> 
>  drivers/scsi/lpfc/lpfc_hw4.h | 2 ++
>  drivers/scsi/lpfc/lpfc_sli.c | 9 +
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
> index 5646699..964a1fd 100644
> --- a/drivers/scsi/lpfc/lpfc_hw4.h
> +++ b/drivers/scsi/lpfc/lpfc_hw4.h
> @@ -1186,6 +1186,7 @@ struct lpfc_mbx_wq_create {
>  #define lpfc_mbx_wq_create_page_size_SHIFT   0
>  #define lpfc_mbx_wq_create_page_size_MASK0x00FF
>  #define lpfc_mbx_wq_create_page_size_WORDword1
> +#define LPFC_WQ_PAGE_SIZE_4096   0x1
>  #define lpfc_mbx_wq_create_wqe_size_SHIFT8
>  #define lpfc_mbx_wq_create_wqe_size_MASK 0x000F
>  #define lpfc_mbx_wq_create_wqe_size_WORD word1
> @@ -1257,6 +1258,7 @@ struct rq_context {
>  #define lpfc_rq_context_page_size_SHIFT  0   /* Version 1 
> Only */
>  #define lpfc_rq_context_page_size_MASK   0x00FF
>  #define lpfc_rq_context_page_size_WORD   word0
> +#define  LPFC_RQ_PAGE_SIZE_4096  0x1

I wonder why do we need the two LPFC_RQ_PAGE_SIZE_4096 and
LPFC_WQ_PAGE_SIZE_4096 both being 1? Wouldn't a more global
LPFC_PAGE_SIZE_4096 or LPFC_Q_PAGE_SIZE_4096 be sufficient?

But I don't have a strong opinion on this either.

Otherwise,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850