date:20140801

On Fri, 2014-08-01 at 08:02 +0200, Juergen Gross wrote:
 On 08/01/2014 07:41 AM, Juergen Gross wrote:
  During test of Xen pvSCSI frontend module I found the following issue:
 
  When unplugging a passed-through SCSI-device the SCSI Host is removed.
  When calling the final scsi_host_put() from the driver an Oops is
  happening:
 
  [  219.816292] (file=drivers/scsi/xen-scsifront.c, line=808)
  scsifront_remove: device/vscsi/1 removed
  [  219.816371] BUG: unable to handle kernel NULL pointer dereference at
  0010
  [  219.816380] IP: [805fdcf8] scsi_put_host_cmd_pool+0x38/0xb0
  [  219.816390] PGD 3bd60067 PUD 3d353067 PMD 0
  [  219.816396] Oops:  [#1] SMP
  [  219.816400] Modules linked in: nls_utf8 sr_mod cdrom xen_scsifront
  xt_pkttype xt_LOG xt_limit af_packet ip6t_REJECT xt_tcpudp
  nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT iptable_raw
  xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns
  nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables
  xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables
  x86_pkg_temp_thermal thermal_sys coretemp hwmon crc32_pclmul
  crc32c_intel ghash_clmulni_intel aesni_intel ablk_helper cryptd lrw
  gf128mul glue_helper aes_x86_64 microcode pcspkr sg dm_mod autofs4
  scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh_hp_sw scsi_dh xen_blkfront
  xen_netfront
  [  219.816458] CPU: 0 PID: 23 Comm: xenwatch Not tainted
  3.16.0-rc6-11-xen+ #66
  [  219.816463] task: 88003da985d0 ti: 88003da9c000 task.ti:
  88003da9c000
  [  219.816467] RIP: e030:[805fdcf8]  [805fdcf8]
  scsi_put_host_cmd_pool+0x38/0xb0
  [  219.816474] RSP: e02b:88003da9fc20  EFLAGS: 00010202
  [  219.816477] RAX: a01a50c0 RBX:  RCX:
  0003
  [  219.816481] RDX: 0240 RSI: 88003d242b80 RDI:
  80c708c0
  [  219.816485] RBP: 88003da9fc38 R08: 4f7e974a31ed0290 R09:
  
  [  219.816488] R10: 7ff0 R11: 0001 R12:
  8800038f8000
  [  219.816491] R13: a01a50c0 R14:  R15:
  
  [  219.816498] FS:  7fe2e2eeb700() GS:88003f80()
  knlGS:
  [  219.816502] CS:  e033 DS:  ES:  CR0: 80050033
  [  219.816505] CR2: 0010 CR3: 3d20c000 CR4:
  00042660
  [  219.816509] Stack:
  [  219.816511]  8800038f8000 8800038f8030 880003ae3400
  88003da9fc58
  [  219.816516]  805fe78b 8800038f8000 88003bb82c40
  88003da9fc80
  [  219.816521]  805ff587 8800038f81a0 8800038f8190
  880003ae3400
  [  219.816527] Call Trace:
  [  219.816533]  [805fe78b]
  scsi_destroy_command_freelist+0x5b/0x60
  [  219.816538]  [805ff587] scsi_host_dev_release+0x97/0xe0
  [  219.816543]  [805dd71d] device_release+0x2d/0xa0
  [  219.816548]  [804dbec7] kobject_cleanup+0x77/0x1b0
  [  219.816553]  [804dbd70] kobject_put+0x30/0x60
  [  219.816556]  [805dd9e2] put_device+0x12/0x20
  [  219.816560]  [805ff490] scsi_host_put+0x10/0x20
  [  219.816565]  [a01a2042] scsifront_free+0x42/0x90
  [xen_scsifront]
  [  219.816569]  [a01a20ad] scsifront_remove+0x1d/0x50
  [xen_scsifront]
  [  219.816576]  [805a4ee0] xenbus_dev_remove+0x50/0xa0
  [  219.816580]  [805e1a8a] __device_release_driver+0x7a/0xf0
  [  219.816584]  [805e1b1e] device_release_driver+0x1e/0x30
  [  219.816588]  [805e1440] bus_remove_device+0x100/0x180
  [  219.816593]  [805ddef1] device_del+0x121/0x1b0
  [  219.816596]  [805ddf99] device_unregister+0x19/0x60
  [  219.816601]  [805a56be] xenbus_dev_changed+0x9e/0x1e0
  [  219.816606]  [8079d695] ?
  _raw_spin_unlock_irqrestore+0x25/0x50
  [  219.816611]  [805a41d0] ? unregister_xenbus_watch+0x200/0x200
  [  219.816615]  [805a7420] frontend_changed+0x20/0x50
  [  219.816619]  [805a426f] xenwatch_thread+0x9f/0x160
  [  219.816624]  [802a50f0] ? prepare_to_wait_event+0xf0/0xf0
  [  219.816628]  [8028485d] kthread+0xcd/0xf0
  [  219.816633]  [80284790] ? kthread_create_on_node+0x170/0x170
  [  219.816638]  [8079dcbc] ret_from_fork+0x7c/0xb0
  [  219.816642]  [80284790] ? kthread_create_on_node+0x170/0x170
  [  219.816645] Code: 8b af c0 00 00 00 48 c7 c7 c0 08 c7 80 e8 d1 e0 19
  00 49 8b 84 24 c0 00 00 00 8b 90 48 01 00 00 85 d2 74 2f 48 8b 98 50 01
  00 00 8b 43 10 85 c0 74 68 83 e8 01 85 c0 89 43 10 74 37 48 c7 c7 c0
  [  219.816732] RIP  [805fdcf8] scsi_put_host_cmd_pool+0x38/0xb0
  [  219.816747]  RSP 88003da9fc20
  [  219.816750] CR2: 0010
  [  219.816753] ---[ end trace c6915ea21a3d05f7 ]---
 
  I should mention I've specified .cmd_len in the scsi_host_template. The
  only other driver doing this seems to be virtio_scsi.c, so I assume the
  same problem could

Re: Oops in scsi_put_host_cmd_pool

2014-08-01 Thread Juergen Gross


On 08/01/2014 09:05 AM, James Bottomley wrote:

On Fri, 2014-08-01 at 08:02 +0200, Juergen Gross wrote:

On 08/01/2014 07:41 AM, Juergen Gross wrote:

During test of Xen pvSCSI frontend module I found the following issue:

When unplugging a passed-through SCSI-device the SCSI Host is removed.
When calling the final scsi_host_put() from the driver an Oops is
happening:

[  219.816292] (file=drivers/scsi/xen-scsifront.c, line=808)
scsifront_remove: device/vscsi/1 removed
[  219.816371] BUG: unable to handle kernel NULL pointer dereference at
0010
[  219.816380] IP: [805fdcf8] scsi_put_host_cmd_pool+0x38/0xb0
[  219.816390] PGD 3bd60067 PUD 3d353067 PMD 0
[  219.816396] Oops:  [#1] SMP
[  219.816400] Modules linked in: nls_utf8 sr_mod cdrom xen_scsifront
xt_pkttype xt_LOG xt_limit af_packet ip6t_REJECT xt_tcpudp
nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT iptable_raw
xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns
nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables
xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables
x86_pkg_temp_thermal thermal_sys coretemp hwmon crc32_pclmul
crc32c_intel ghash_clmulni_intel aesni_intel ablk_helper cryptd lrw
gf128mul glue_helper aes_x86_64 microcode pcspkr sg dm_mod autofs4
scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh_hp_sw scsi_dh xen_blkfront
xen_netfront
[  219.816458] CPU: 0 PID: 23 Comm: xenwatch Not tainted
3.16.0-rc6-11-xen+ #66
[  219.816463] task: 88003da985d0 ti: 88003da9c000 task.ti:
88003da9c000
[  219.816467] RIP: e030:[805fdcf8]  [805fdcf8]
scsi_put_host_cmd_pool+0x38/0xb0
[  219.816474] RSP: e02b:88003da9fc20  EFLAGS: 00010202
[  219.816477] RAX: a01a50c0 RBX:  RCX:
0003
[  219.816481] RDX: 0240 RSI: 88003d242b80 RDI:
80c708c0
[  219.816485] RBP: 88003da9fc38 R08: 4f7e974a31ed0290 R09:

[  219.816488] R10: 7ff0 R11: 0001 R12:
8800038f8000
[  219.816491] R13: a01a50c0 R14:  R15:

[  219.816498] FS:  7fe2e2eeb700() GS:88003f80()
knlGS:
[  219.816502] CS:  e033 DS:  ES:  CR0: 80050033
[  219.816505] CR2: 0010 CR3: 3d20c000 CR4:
00042660
[  219.816509] Stack:
[  219.816511]  8800038f8000 8800038f8030 880003ae3400
88003da9fc58
[  219.816516]  805fe78b 8800038f8000 88003bb82c40
88003da9fc80
[  219.816521]  805ff587 8800038f81a0 8800038f8190
880003ae3400
[  219.816527] Call Trace:
[  219.816533]  [805fe78b]
scsi_destroy_command_freelist+0x5b/0x60
[  219.816538]  [805ff587] scsi_host_dev_release+0x97/0xe0
[  219.816543]  [805dd71d] device_release+0x2d/0xa0
[  219.816548]  [804dbec7] kobject_cleanup+0x77/0x1b0
[  219.816553]  [804dbd70] kobject_put+0x30/0x60
[  219.816556]  [805dd9e2] put_device+0x12/0x20
[  219.816560]  [805ff490] scsi_host_put+0x10/0x20
[  219.816565]  [a01a2042] scsifront_free+0x42/0x90
[xen_scsifront]
[  219.816569]  [a01a20ad] scsifront_remove+0x1d/0x50
[xen_scsifront]
[  219.816576]  [805a4ee0] xenbus_dev_remove+0x50/0xa0
[  219.816580]  [805e1a8a] __device_release_driver+0x7a/0xf0
[  219.816584]  [805e1b1e] device_release_driver+0x1e/0x30
[  219.816588]  [805e1440] bus_remove_device+0x100/0x180
[  219.816593]  [805ddef1] device_del+0x121/0x1b0
[  219.816596]  [805ddf99] device_unregister+0x19/0x60
[  219.816601]  [805a56be] xenbus_dev_changed+0x9e/0x1e0
[  219.816606]  [8079d695] ?
_raw_spin_unlock_irqrestore+0x25/0x50
[  219.816611]  [805a41d0] ? unregister_xenbus_watch+0x200/0x200
[  219.816615]  [805a7420] frontend_changed+0x20/0x50
[  219.816619]  [805a426f] xenwatch_thread+0x9f/0x160
[  219.816624]  [802a50f0] ? prepare_to_wait_event+0xf0/0xf0
[  219.816628]  [8028485d] kthread+0xcd/0xf0
[  219.816633]  [80284790] ? kthread_create_on_node+0x170/0x170
[  219.816638]  [8079dcbc] ret_from_fork+0x7c/0xb0
[  219.816642]  [80284790] ? kthread_create_on_node+0x170/0x170
[  219.816645] Code: 8b af c0 00 00 00 48 c7 c7 c0 08 c7 80 e8 d1 e0 19
00 49 8b 84 24 c0 00 00 00 8b 90 48 01 00 00 85 d2 74 2f 48 8b 98 50 01
00 00 8b 43 10 85 c0 74 68 83 e8 01 85 c0 89 43 10 74 37 48 c7 c7 c0
[  219.816732] RIP  [805fdcf8] scsi_put_host_cmd_pool+0x38/0xb0
[  219.816747]  RSP 88003da9fc20
[  219.816750] CR2: 0010
[  219.816753] ---[ end trace c6915ea21a3d05f7 ]---

I should mention I've specified .cmd_len in the scsi_host_template. The
only other driver doing this seems to be virtio_scsi.c, so I assume the
same problem could occur with passed-through SCSI devices under KVM...


After looking into the code the reason seems to be rather obvious:

[PATCH] scsi: ufs: fix Command Type issue according to UFS 2.0 spec

2014-08-01 Thread Chuanxiao Dong

UFS 2.0 spec defines that the command type in transfer descriptor
is always 0x1 instead of different values as shown in UFSHC1.0/1.1.

This patch will distingwish v1.0/v1.1 UFSHC and later UFSHC when
setting CT

Signed-off-by: Chuanxiao Dong chuanxiao.d...@intel.com
---
 drivers/scsi/ufs/ufshcd.c |   27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ba27215..8635d5d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -706,16 +706,19 @@ static void ufshcd_disable_intr(struct ufs_hba *hba, u32 
intrs)
 /**
  * ufshcd_prepare_req_desc_hdr() - Fills the requests header
  * descriptor according to request
+ * @hba: per adapter instance
  * @lrbp: pointer to local reference block
  * @upiu_flags: flags required in the header
  * @cmd_dir: requests data direction
  */
-static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp,
-   u32 *upiu_flags, enum dma_data_direction cmd_dir)
+static void ufshcd_prepare_req_desc_hdr(struct ufs_hba *hba,
+   struct ufshcd_lrb *lrbp, u32 *upiu_flags,
+   enum dma_data_direction cmd_dir)
 {
struct utp_transfer_req_desc *req_desc = lrbp-utr_descriptor_ptr;
u32 data_direction;
u32 dword_0;
+   u8 cmd_type;
 
if (cmd_dir == DMA_FROM_DEVICE) {
data_direction = UTP_DEVICE_TO_HOST;
@@ -728,8 +731,20 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb 
*lrbp,
*upiu_flags = UPIU_CMD_FLAGS_NONE;
}
 
-   dword_0 = data_direction | (lrbp-command_type
-UPIU_COMMAND_TYPE_OFFSET);
+   /*
+* Per UFSHCI spec v1.0/v1.1, Command Type in UTP transfer
+* request descriptor is not the same as v2.0 spec.
+* In v2.0 spec, Command Type is always 1, the other values
+* are reserved
+*/
+   if (hba-ufs_version == UFSHCI_VERSION_10 ||
+   hba-ufs_version == UFSHCI_VERSION_11)
+   cmd_type = lrbp-command_type;
+   else
+   cmd_type = UTP_CMD_TYPE_UFS;
+
+   dword_0 = data_direction | (cmd_type  UPIU_COMMAND_TYPE_OFFSET);
+
if (lrbp-intr_cmd)
dword_0 |= UTP_REQ_DESC_INT_CMD;
 
@@ -834,7 +849,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
switch (lrbp-command_type) {
case UTP_CMD_TYPE_SCSI:
if (likely(lrbp-cmd)) {
-   ufshcd_prepare_req_desc_hdr(lrbp, upiu_flags,
+   ufshcd_prepare_req_desc_hdr(hba, lrbp, upiu_flags,
lrbp-cmd-sc_data_direction);
ufshcd_prepare_utp_scsi_cmd_upiu(lrbp, upiu_flags);
} else {
@@ -842,7 +857,7 @@ static int ufshcd_compose_upiu(struct ufs_hba *hba, struct 
ufshcd_lrb *lrbp)
}
break;
case UTP_CMD_TYPE_DEV_MANAGE:
-   ufshcd_prepare_req_desc_hdr(lrbp, upiu_flags, DMA_NONE);
+   ufshcd_prepare_req_desc_hdr(hba, lrbp, upiu_flags, DMA_NONE);
if (hba-dev_cmd.type == DEV_CMD_TYPE_QUERY)
ufshcd_prepare_utp_query_req_upiu(
hba, lrbp, upiu_flags);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1.3 0/18] arcmsr: bugfix, new features and support new adapters

Hi Christoph,

This patches are made against the 
git://git.infradead.org/users/hch/scsi-queue.git/tree/drivers/scsi/arcmsr/

This patches series address following issues.

1. Bugfix for command timeout, abort and ioctl error.

2. Add new feature of support MSI-X interrupt and system hibernation.

3. Support new adapters ARC12x4 series.

4. Simplify and unify code for readability and consistency.

Regards,
Chingching2...@areca.com.tw

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1.3 1/18] arcmsr: Revised interrupt service routine relate function to fix bug


This patch rewrite the interrupt service routine relate function to fix command 
timeout when controller has very 
heavy loading.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h  2014-07-30 10:33:02.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h  2014-04-28 16:02:46.0 +0800
@@ -51,7 +51,7 @@ struct device_attribute;
 #else
#define ARCMSR_MAX_FREECCB_NUM  320
 #endif
-#define ARCMSR_DRIVER_VERSION   Driver Version 1.20.00.15 
2010/08/05
+#define ARCMSR_DRIVER_VERSION  v1.30.00.04-20140428
 #define ARCMSR_SCSI_INITIATOR_ID   
255
 #define ARCMSR_MAX_XFER_SECTORS
512
 #define ARCMSR_MAX_XFER_SECTORS_B  
4096
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-07-30 10:32:28.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:02:44.0 +0800
@@ -1441,14 +1441,15 @@ static void arcmsr_hba_doorbell_isr(stru
uint32_t outbound_doorbell;
struct MessageUnit_A __iomem *reg = acb-pmuA;
outbound_doorbell = readl(reg-outbound_doorbell);
-   writel(outbound_doorbell, reg-outbound_doorbell);
-   if (outbound_doorbell  ARCMSR_OUTBOUND_IOP331_DATA_WRITE_OK) {
-   arcmsr_iop2drv_data_wrote_handle(acb);
-   }
-
-   if (outbound_doorbell  ARCMSR_OUTBOUND_IOP331_DATA_READ_OK) {
-   arcmsr_iop2drv_data_read_handle(acb);
-   }
+   do {
+   writel(outbound_doorbell, reg-outbound_doorbell);
+   if (outbound_doorbell  ARCMSR_OUTBOUND_IOP331_DATA_WRITE_OK)
+   arcmsr_iop2drv_data_wrote_handle(acb);
+   if (outbound_doorbell  ARCMSR_OUTBOUND_IOP331_DATA_READ_OK)
+   arcmsr_iop2drv_data_read_handle(acb);
+   outbound_doorbell = readl(reg-outbound_doorbell);
+   } while (outbound_doorbell  (ARCMSR_OUTBOUND_IOP331_DATA_WRITE_OK
+   | ARCMSR_OUTBOUND_IOP331_DATA_READ_OK));
 }
 static void arcmsr_hbc_doorbell_isr(struct AdapterControlBlock *pACB)
 {
@@ -1462,17 +1463,19 @@ static void arcmsr_hbc_doorbell_isr(stru
***
*/
outbound_doorbell = readl(reg-outbound_doorbell);
-   writel(outbound_doorbell, reg-outbound_doorbell_clear);/*clear 
interrupt*/
-   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_DATA_WRITE_OK) {
-   arcmsr_iop2drv_data_wrote_handle(pACB);
-   }
-   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_DATA_READ_OK) {
-   arcmsr_iop2drv_data_read_handle(pACB);
-   }
-   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE) {
-   arcmsr_hbc_message_isr(pACB);/* messenger of driver to iop 
commands */
-   }
-   return;
+   do {
+   writel(outbound_doorbell, reg-outbound_doorbell_clear);
+   readl(reg-outbound_doorbell_clear);
+   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_DATA_WRITE_OK)
+   arcmsr_iop2drv_data_wrote_handle(pACB);
+   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_DATA_READ_OK)
+   arcmsr_iop2drv_data_read_handle(pACB);
+   if (outbound_doorbell  ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE)
+   arcmsr_hbc_message_isr(pACB);
+   outbound_doorbell = readl(reg-outbound_doorbell);
+   } while (outbound_doorbell  (ARCMSR_HBCMU_IOP2DRV_DATA_WRITE_OK
+   | ARCMSR_HBCMU_IOP2DRV_DATA_READ_OK
+   | ARCMSR_HBCMU_IOP2DRV_MESSAGE_CMD_DONE));
 }
 static void arcmsr_hba_postqueue_isr(struct AdapterControlBlock *acb)
 {
@@ -1521,21 +1524,22 @@ static void arcmsr_hbc_postqueue_isr(str
/* areca cdb command done */
/* Use correct offset and size for syncing */
 
-   while (readl(phbcmu-host_int_status) 
-   ARCMSR_HBCMU_OUTBOUND_POSTQUEUE_ISR){
-   /* check if command done with no error*/
-   flag_ccb = readl(phbcmu-outbound_queueport_low);
-   ccb_cdb_phy = (flag_ccb  0xFFF0);/*frame must be 32 bytes aligned*/
-   arcmsr_cdb = (struct ARCMSR_CDB *)(acb-vir2phy_offset + ccb_cdb_phy);
-   ccb = container_of(arcmsr_cdb, struct CommandControlBlock, arcmsr_cdb);
-   error = (flag_ccb  ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ? true : false;
-   /* check if command done with no error */
-   arcmsr_drain_donequeue(acb, ccb, error);
-   if (throttling == ARCMSR_HBC_ISR_THROTTLING_LEVEL) {
-   writel(ARCMSR_HBCMU_DRV2IOP_POSTQUEUE_THROTTLING, 
phbcmu-inbound_doorbell);
-   break;
-   }
-   throttling++;
+   while ((flag_ccb = readl(phbcmu-outbound_queueport_low)) !=

[PATCH v1.3 2/18] arcmsr: Add code to support MSI-X, MSI interrupt


This patch adds code to support MSI, MSI-X interrupt.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h  2014-04-28 16:02:46.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h  2014-05-06 15:24:36.0 +0800
@@ -64,6 +64,7 @@ struct device_attribute;
 #define ARCMSR_MAX_HBB_POSTQUEUE   
264
 #define ARCMSR_MAX_XFER_LEN
0x26000 /* 152K */
 #define ARCMSR_CDB_SG_PAGE_LENGTH  
256 
+#define ARCMST_NUM_MSIX_VECTORS4
 #ifndef PCI_DEVICE_ID_ARECA_1880
 #define PCI_DEVICE_ID_ARECA_1880 0x1880
  #endif
@@ -508,6 +509,7 @@ struct AdapterControlBlock
struct pci_dev *pdev;
struct Scsi_Host *  host;
unsigned long   vir2phy_offset;
+   struct msix_entry   entries[ARCMST_NUM_MSIX_VECTORS];
/* Offset is used in making arc cdb physical to virtual calculations */
uint32_toutbound_int_enable;
uint32_tcdb_phyaddr_hi32;
@@ -544,6 +546,8 @@ struct AdapterControlBlock
/* iop init */
#define ACB_F_ABORT 0x0200
#define ACB_F_FIRMWARE_TRAP 0x0400
+   #define ACB_F_MSI_ENABLED   0x1000
+   #define ACB_F_MSIX_ENABLED  0x2000
struct CommandControlBlock *
pccb_pool[ARCMSR_MAX_FREECCB_NUM];
/* used for memory free */
struct list_headccb_free_list;
@@ -594,6 +598,7 @@ struct AdapterControlBlock
#define FW_DEADLOCK 0x0010
atomic_trq_map_token;
atomic_tante_token_value;
+   int msix_vector_count;
 };/* HW_DEVICE_EXTENSION */
 /*
 ***
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:02:44.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:03:00.0 +0800
@@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
}
 }
 
+static int
+arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
+{
+   int i, j, r;
+   struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
+
+   if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
+   goto msi_int;
+   for (i = 0; i  ARCMST_NUM_MSIX_VECTORS; i++)
+   entries[i].entry = i;
+   r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
+   if (r  0)
+   goto msi_int;
+   acb-msix_vector_count = r;
+   for (i = 0; i  r; i++) {
+   if (request_irq(entries[i].vector,
+   arcmsr_do_interrupt, 0, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq =%d failed!\n,
+   acb-host-host_no, pdev-irq);
+   for (j = 0 ; j  i ; j++)
+   free_irq(entries[j].vector, acb);
+   pci_disable_msix(pdev);
+   goto msi_int;
+   }
+   acb-entries[i] = entries[i];
+   }
+   acb-acb_flags |= ACB_F_MSIX_ENABLED;
+   pr_info(arcmsr%d: msi-x enabled\n, acb-host-host_no);
+   return ARC_SUCCESS;
+msi_int:
+   if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
+   goto legacy_int;
+   if (pci_enable_msi_range(pdev, 1, 1)  0)
+   goto legacy_int;
+   if (request_irq(pdev-irq, arcmsr_do_interrupt,
+   IRQF_SHARED, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq =%d failed!\n,
+   acb-host-host_no, pdev-irq);
+   pci_disable_msi(pdev);
+   goto legacy_int;
+   }
+   acb-acb_flags |= ACB_F_MSI_ENABLED;
+   pr_info(arcmsr%d: msi enabled\n, acb-host-host_no);
+   return ARC_SUCCESS;
+legacy_int:
+   if (request_irq(pdev-irq, arcmsr_do_interrupt,
+   IRQF_SHARED, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq = %d failed!\n,
+   acb-host-host_no, pdev-irq);
+   return ARC_FAILURE;
+   }
+   return ARC_SUCCESS;
+}
+
 static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
struct Scsi_Host *host;
@@ -667,16 +721,13 @@ static int arcmsr_probe(struct pci_dev *
if(error){
goto free_hbb_mu;
}
-   arcmsr_iop_init(acb);
error = scsi_add_host(host, pdev-dev);
if(error){
goto RAID_controller_stop;
}
-   error = request_irq(pdev-irq, arcmsr_do_interrupt, IRQF_SHARED,

[PATCH v1.3 3/18] arcmsr: Add code to support system hibernation


This patch adds code to support system hibernation.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:03:00.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:04:28.0 +0800
@@ -89,11 +89,15 @@ static int arcmsr_bios_param(struct scsi
 static int arcmsr_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd);
 static int arcmsr_probe(struct pci_dev *pdev,
const struct pci_device_id *id);
+static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state);
+static int arcmsr_resume(struct pci_dev *pdev);
 static void arcmsr_remove(struct pci_dev *pdev);
 static void arcmsr_shutdown(struct pci_dev *pdev);
 static void arcmsr_iop_init(struct AdapterControlBlock *acb);
 static void arcmsr_free_ccb_pool(struct AdapterControlBlock *acb);
 static u32 arcmsr_disable_outbound_ints(struct AdapterControlBlock *acb);
+static void arcmsr_enable_outbound_ints(struct AdapterControlBlock *acb,
+   u32 intmask_org);
 static void arcmsr_stop_adapter_bgrb(struct AdapterControlBlock *acb);
 static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb);
 static void arcmsr_flush_hbb_cache(struct AdapterControlBlock *acb);
@@ -167,6 +171,8 @@ static struct pci_driver arcmsr_pci_driv
.id_table   = arcmsr_device_id_table,
.probe  = arcmsr_probe,
.remove = arcmsr_remove,
+   .suspend= arcmsr_suspend,
+   .resume = arcmsr_resume,
.shutdown   = arcmsr_shutdown,
 };
 /*
@@ -776,6 +782,77 @@ static void arcmsr_free_irq(struct pci_d
free_irq(pdev-irq, acb);
 }
 
+static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+   uint32_t intmask_org;
+   struct Scsi_Host *host = pci_get_drvdata(pdev);
+   struct AdapterControlBlock *acb =
+   (struct AdapterControlBlock *)host-hostdata;
+
+   intmask_org = arcmsr_disable_outbound_ints(acb);
+   arcmsr_free_irq(pdev, acb);
+   del_timer_sync(acb-eternal_timer);
+   flush_scheduled_work();
+   arcmsr_stop_adapter_bgrb(acb);
+   arcmsr_flush_adapter_cache(acb);
+   arcmsr_enable_outbound_ints(acb, intmask_org);
+   pci_set_drvdata(pdev, host);
+   pci_save_state(pdev);
+   pci_disable_device(pdev);
+   pci_set_power_state(pdev, pci_choose_state(pdev, state));
+   return 0;
+}
+
+static int arcmsr_resume(struct pci_dev *pdev)
+{
+   int error;
+   struct Scsi_Host *host = pci_get_drvdata(pdev);
+   struct AdapterControlBlock *acb =
+   (struct AdapterControlBlock *)host-hostdata;
+
+   pci_set_power_state(pdev, PCI_D0);
+   pci_enable_wake(pdev, PCI_D0, 0);
+   pci_restore_state(pdev);
+   if (pci_enable_device(pdev)) {
+   pr_warn(%s: pci_enable_device error\n, __func__);
+   return -ENODEV;
+   }
+   error = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+   if (error) {
+   error = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+   if (error) {
+   pr_warn(scsi%d: No suitable DMA mask available\n,
+  host-host_no);
+   goto controller_unregister;
+   }
+   }
+   pci_set_master(pdev);
+   if (arcmsr_request_irq(pdev, acb) == ARC_FAILURE)
+   goto controller_stop;
+   arcmsr_iop_init(acb);
+   INIT_WORK(acb-arcmsr_do_message_isr_bh, arcmsr_message_isr_bh_fn);
+   atomic_set(acb-rq_map_token, 16);
+   atomic_set(acb-ante_token_value, 16);
+   acb-fw_flag = FW_NORMAL;
+   init_timer(acb-eternal_timer);
+   acb-eternal_timer.expires = jiffies + msecs_to_jiffies(6 * HZ);
+   acb-eternal_timer.data = (unsigned long) acb;
+   acb-eternal_timer.function = arcmsr_request_device_map;
+   add_timer(acb-eternal_timer);
+   return 0;
+controller_stop:
+   arcmsr_stop_adapter_bgrb(acb);
+   arcmsr_flush_adapter_cache(acb);
+controller_unregister:
+   scsi_remove_host(host);
+   arcmsr_free_ccb_pool(acb);
+   arcmsr_unmap_pciregion(acb);
+   pci_release_regions(pdev);
+   scsi_host_put(host);
+   pci_disable_device(pdev);
+   return -ENODEV;
+}
+
 static uint8_t arcmsr_abort_hba_allcmd(struct AdapterControlBlock *acb)
 {
struct MessageUnit_A __iomem *reg = acb-pmuA;


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1.3 4/18] arcmsr: limit max. number of SCSI command request


This patch limits the max. number of SCSI command request to avoid command 
overflow.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h  2014-05-06 15:24:06.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h  2014-05-06 15:22:36.0 +0800
@@ -45,11 +45,12 @@
 #include linux/interrupt.h
 struct device_attribute;
 /*The limit of outstanding scsi command that firmware can handle*/
-#define ARCMSR_MAX_OUTSTANDING_CMD 
256
 #ifdef CONFIG_XEN
#define ARCMSR_MAX_FREECCB_NUM  160
+#define ARCMSR_MAX_OUTSTANDING_CMD 155
 #else
#define ARCMSR_MAX_FREECCB_NUM  320
+#define ARCMSR_MAX_OUTSTANDING_CMD 255
 #endif
 #define ARCMSR_DRIVER_VERSION  v1.30.00.04-20140428
 #define ARCMSR_SCSI_INITIATOR_ID   
255
@@ -598,6 +599,7 @@ struct AdapterControlBlock
#define FW_DEADLOCK 0x0010
atomic_trq_map_token;
atomic_tante_token_value;
+   uint32_tmaxOutstanding;
int msix_vector_count;
 };/* HW_DEVICE_EXTENSION */
 /*
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:04:28.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:05:44.0 +0800
@@ -134,7 +134,7 @@ static struct scsi_host_template arcmsr_
.eh_bus_reset_handler   = arcmsr_bus_reset,
.bios_param = arcmsr_bios_param,
.change_queue_depth = arcmsr_adjust_disk_queue_depth,
-   .can_queue  = ARCMSR_MAX_FREECCB_NUM,
+   .can_queue  = ARCMSR_MAX_OUTSTANDING_CMD,
.this_id= ARCMSR_SCSI_INITIATOR_ID,
.sg_tablesize   = ARCMSR_DEFAULT_SG_ENTRIES, 
.max_sectors= ARCMSR_MAX_XFER_SECTORS_C, 
@@ -697,7 +697,7 @@ static int arcmsr_probe(struct pci_dev *
host-max_lun = ARCMSR_MAX_TARGETLUN;
host-max_id = ARCMSR_MAX_TARGETID; /*16:8*/
host-max_cmd_len = 16; /*this is issue of 
64bit LBA ,over 2T byte*/
-   host-can_queue = ARCMSR_MAX_FREECCB_NUM;   /* max simultaneous 
cmds */ 
+   host-can_queue = ARCMSR_MAX_OUTSTANDING_CMD;   /* max simultaneous 
cmds */ 
host-cmd_per_lun = ARCMSR_MAX_CMD_PERLUN;  
host-this_id = ARCMSR_SCSI_INITIATOR_ID;
host-unique_id = (bus  8) | dev_fun;
@@ -2220,8 +2220,7 @@ static int arcmsr_queue_command_lck(stru
arcmsr_handle_virtual_command(acb, cmd);
return 0;
}
-   if (atomic_read(acb-ccboutstandingcount) =
-   ARCMSR_MAX_OUTSTANDING_CMD)
+   if (atomic_read(acb-ccboutstandingcount) = acb-maxOutstanding)
return SCSI_MLQUEUE_HOST_BUSY;
ccb = arcmsr_get_freeccb(acb);
if (!ccb)
@@ -2432,12 +2431,26 @@ static bool arcmsr_get_hbc_config(struct
 }
 static bool arcmsr_get_firmware_spec(struct AdapterControlBlock *acb)
 {
-   if (acb-adapter_type == ACB_ADAPTER_TYPE_A)
-   return arcmsr_get_hba_config(acb);
-   else if (acb-adapter_type == ACB_ADAPTER_TYPE_B)
-   return arcmsr_get_hbb_config(acb);
+   bool rtn = false;
+
+   switch (acb-adapter_type) {
+   case ACB_ADAPTER_TYPE_A:
+   rtn = arcmsr_get_hba_config(acb);
+   break;
+   case ACB_ADAPTER_TYPE_B:
+   rtn = arcmsr_get_hbb_config(acb);
+   break;
+   case ACB_ADAPTER_TYPE_C:
+   rtn = arcmsr_get_hbc_config(acb);
+   break;
+   default:
+   break;
+   }
+   if(acb-firm_numbers_queue  ARCMSR_MAX_OUTSTANDING_CMD)
+   acb-maxOutstanding = ARCMSR_MAX_OUTSTANDING_CMD;
else
-   return arcmsr_get_hbc_config(acb);
+   acb-maxOutstanding = acb-firm_numbers_queue - 1;
+   return rtn;
 }
 
 static int arcmsr_polling_hba_ccbdone(struct AdapterControlBlock *acb,


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1.3 5/18] arcmsr: bugfix - return status of abort command


This patch fixes the wrong return status of abort command.

Singed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:05:44.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:06:34.0 +0800
@@ -2482,7 +2482,7 @@ static int arcmsr_polling_hba_ccbdone(st
}
arcmsr_cdb = (struct ARCMSR_CDB *)(acb-vir2phy_offset + 
(flag_ccb  5));
ccb = container_of(arcmsr_cdb, struct CommandControlBlock, 
arcmsr_cdb);
-   poll_ccb_done = (ccb == poll_ccb) ? 1:0;
+   poll_ccb_done |= (ccb == poll_ccb) ? 1 : 0;
if ((ccb-acb != acb) || (ccb-startdone != ARCMSR_CCB_START)) {
if ((ccb-startdone == ARCMSR_CCB_ABORTED) || (ccb == 
poll_ccb)) {
printk(KERN_NOTICE arcmsr%d: scsi id = %d lun 
= %d ccb = '0x%p'
@@ -2546,7 +2546,7 @@ static int arcmsr_polling_hbb_ccbdone(st
/* check if command done with no error*/
arcmsr_cdb = (struct ARCMSR_CDB *)(acb-vir2phy_offset + 
(flag_ccb  5));
ccb = container_of(arcmsr_cdb, struct CommandControlBlock, 
arcmsr_cdb);
-   poll_ccb_done = (ccb == poll_ccb) ? 1:0;
+   poll_ccb_done |= (ccb == poll_ccb) ? 1 : 0;
if ((ccb-acb != acb) || (ccb-startdone != ARCMSR_CCB_START)) {
if ((ccb-startdone == ARCMSR_CCB_ABORTED) || (ccb == 
poll_ccb)) {
printk(KERN_NOTICE arcmsr%d: scsi id = %d lun 
= %d ccb = '0x%p'
@@ -2602,7 +2602,7 @@ polling_hbc_ccb_retry:
ccb_cdb_phy = (flag_ccb  0xFFF0);
arcmsr_cdb = (struct ARCMSR_CDB *)(acb-vir2phy_offset + 
ccb_cdb_phy);/*frame must be 32 bytes aligned*/
pCCB = container_of(arcmsr_cdb, struct CommandControlBlock, 
arcmsr_cdb);
-   poll_ccb_done = (pCCB == poll_ccb) ? 1 : 0;
+   poll_ccb_done |= (pCCB == poll_ccb) ? 1 : 0;
/* check ifcommand done with no error*/
if ((pCCB-acb != acb) || (pCCB-startdone != 
ARCMSR_CCB_START)) {
if (pCCB-startdone == ARCMSR_CCB_ABORTED) {
@@ -3204,6 +3204,8 @@ static int arcmsr_abort(struct scsi_cmnd
(struct AdapterControlBlock *)cmd-device-host-hostdata;
int i = 0;
int rtn = FAILED;
+   uint32_t intmask_org;
+
printk(KERN_NOTICE
arcmsr%d: abort device command of scsi id = %d lun = %d \n,
acb-host-host_no, cmd-device-id, (u32)cmd-device-lun);
@@ -3215,9 +3217,12 @@ static int arcmsr_abort(struct scsi_cmnd
** we need to handle it as soon as possible and exit

*/
-   if (!atomic_read(acb-ccboutstandingcount))
+   if (!atomic_read(acb-ccboutstandingcount)) {
+   acb-acb_flags = ~ACB_F_ABORT;
return rtn;
+   }
 
+   intmask_org = arcmsr_disable_outbound_ints(acb);
for (i = 0; i  ARCMSR_MAX_FREECCB_NUM; i++) {
struct CommandControlBlock *ccb = acb-pccb_pool[i];
if (ccb-startdone == ARCMSR_CCB_START  ccb-pcmd == cmd) {
@@ -3227,6 +3232,7 @@ static int arcmsr_abort(struct scsi_cmnd
}
}
acb-acb_flags = ~ACB_F_ABORT;
+   arcmsr_enable_outbound_ints(acb, intmask_org);
return rtn;
 }
 


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.3 2/18] arcmsr: Add code to support MSI-X, MSI interrupt

2014-08-01 Thread Alexander Gordeev

On Fri, Aug 01, 2014 at 04:33:09PM +0800, Ching Huang wrote:
 
 This patch adds code to support MSI, MSI-X interrupt.
 
 Signed-off-by: Chingching2...@areca.com.tw
 ---
 
 diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
 --- a/drivers/scsi/arcmsr/arcmsr.h2014-04-28 16:02:46.0 +0800
 +++ b/drivers/scsi/arcmsr/arcmsr.h2014-05-06 15:24:36.0 +0800
 @@ -64,6 +64,7 @@ struct device_attribute;
  #define ARCMSR_MAX_HBB_POSTQUEUE 
 264
  #define ARCMSR_MAX_XFER_LEN  
 0x26000 /* 152K */
  #define ARCMSR_CDB_SG_PAGE_LENGTH
 256 
 +#define ARCMST_NUM_MSIX_VECTORS  4
  #ifndef PCI_DEVICE_ID_ARECA_1880
  #define PCI_DEVICE_ID_ARECA_1880 0x1880
   #endif
 @@ -508,6 +509,7 @@ struct AdapterControlBlock
   struct pci_dev *pdev;
   struct Scsi_Host *  host;
   unsigned long   vir2phy_offset;
 + struct msix_entry   entries[ARCMST_NUM_MSIX_VECTORS];
   /* Offset is used in making arc cdb physical to virtual calculations */
   uint32_toutbound_int_enable;
   uint32_tcdb_phyaddr_hi32;
 @@ -544,6 +546,8 @@ struct AdapterControlBlock
   /* iop init */
   #define ACB_F_ABORT 0x0200
   #define ACB_F_FIRMWARE_TRAP 0x0400
 + #define ACB_F_MSI_ENABLED   0x1000
 + #define ACB_F_MSIX_ENABLED  0x2000
   struct CommandControlBlock *
 pccb_pool[ARCMSR_MAX_FREECCB_NUM];
   /* used for memory free */
   struct list_headccb_free_list;
 @@ -594,6 +598,7 @@ struct AdapterControlBlock
   #define FW_DEADLOCK 0x0010
   atomic_trq_map_token;
   atomic_tante_token_value;
 + int msix_vector_count;
  };/* HW_DEVICE_EXTENSION */
  /*
  
 ***
 diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c 
 b/drivers/scsi/arcmsr/arcmsr_hba.c
 --- a/drivers/scsi/arcmsr/arcmsr_hba.c2014-08-01 11:02:44.0 
 +0800
 +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2014-08-01 11:03:00.0 
 +0800
 @@ -603,6 +603,60 @@ static void arcmsr_message_isr_bh_fn(str
   }
  }
  
 +static int
 +arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
 +{
 + int i, j, r;
 + struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
 +
 + if (!pci_find_capability(pdev, PCI_CAP_ID_MSIX))
 + goto msi_int;

It is not on top of my head, but checking the capability is discouraged.
pci_enable_msix_range() will fail if MSI-X is not supported. Hence, it
is redundant.

 + for (i = 0; i  ARCMST_NUM_MSIX_VECTORS; i++)
 + entries[i].entry = i;
 + r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
 + if (r  0)
 + goto msi_int;
 + acb-msix_vector_count = r;

Not sure how msix_vector_count is used throughout the rest of the code,
but placing this assignment after successful initialization of MSI-X
is generally safer and clearer.

 + for (i = 0; i  r; i++) {
 + if (request_irq(entries[i].vector,
 + arcmsr_do_interrupt, 0, arcmsr, acb)) {
 + pr_warn(arcmsr%d: request_irq =%d failed!\n,
 + acb-host-host_no, pdev-irq);

Nah, pdev-irq is wrong here. Should be entries[i].vector.

 + for (j = 0 ; j  i ; j++)
 + free_irq(entries[j].vector, acb);
 + pci_disable_msix(pdev);
 + goto msi_int;
 + }
 + acb-entries[i] = entries[i];
 + }
 + acb-acb_flags |= ACB_F_MSIX_ENABLED;
 + pr_info(arcmsr%d: msi-x enabled\n, acb-host-host_no);
 + return ARC_SUCCESS;
 +msi_int:
 + if (!pci_find_capability(pdev, PCI_CAP_ID_MSI))
 + goto legacy_int;

The same as with MSI-X (above).

 + if (pci_enable_msi_range(pdev, 1, 1)  0)
 + goto legacy_int;

pci_enable_msi_exact() or pci_enable_msi() is better choice here.

 + if (request_irq(pdev-irq, arcmsr_do_interrupt,
 + IRQF_SHARED, arcmsr, acb)) {
 + pr_warn(arcmsr%d: request_irq =%d failed!\n,
 + acb-host-host_no, pdev-irq);
 + pci_disable_msi(pdev);
 + goto legacy_int;
 + }
 + acb-acb_flags |= ACB_F_MSI_ENABLED;
 + pr_info(arcmsr%d: msi enabled\n, acb-host-host_no);
 + return ARC_SUCCESS;
 +legacy_int:
 + if (request_irq(pdev-irq, arcmsr_do_interrupt,
 + IRQF_SHARED, arcmsr, acb)) {
 + pr_warn(arcmsr%d: request_irq = %d failed!\n,
 + acb-host-host_no, pdev-irq);
 +

[PATCH v1.3 6/18] arcmsr: precise checking adapter ID


This patch rewrites the arcmsr_define_adapter_type function to precisely check 
Areca adapter's ID.
This can prevent an unknown adapter being used as a default adapter type by 
driver.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:06:34.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:07:44.0 +0800
@@ -295,24 +295,43 @@ static int arcmsr_bios_param(struct scsi
return 0;
 }
 
-static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
+static bool arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
 {
struct pci_dev *pdev = acb-pdev;
u16 dev_id;
+
pci_read_config_word(pdev, PCI_DEVICE_ID, dev_id);
acb-dev_id = dev_id;
switch (dev_id) {
-   case 0x1880: {
+   case 0x1880:
acb-adapter_type = ACB_ADAPTER_TYPE_C;
-   }
break;
-   case 0x1201: {
+   case 0x1200:
+   case 0x1201:
+   case 0x1202:
acb-adapter_type = ACB_ADAPTER_TYPE_B;
-   }
break;
-
-   default: acb-adapter_type = ACB_ADAPTER_TYPE_A;
+   case 0x1110:
+   case 0x1120:
+   case 0x1130:
+   case 0x1160:
+   case 0x1170:
+   case 0x1210:
+   case 0x1220:
+   case 0x1230:
+   case 0x1260:
+   case 0x1270:
+   case 0x1280:
+   case 0x1380:
+   case 0x1381:
+   case 0x1680:
+   acb-adapter_type = ACB_ADAPTER_TYPE_A;
+   break;
+   default:
+   pr_notice(Unknown device ID = 0x%x\n, dev_id);
+   return false;
}
+   return true;
 }
 
 static uint8_t arcmsr_hba_wait_msgint_ready(struct AdapterControlBlock *acb)
@@ -714,7 +733,9 @@ static int arcmsr_probe(struct pci_dev *
ACB_F_MESSAGE_WQBUFFER_READED);
acb-acb_flags = ~ACB_F_SCSISTOPADAPTER;
INIT_LIST_HEAD(acb-ccb_free_list);
-   arcmsr_define_adapter_type(acb);
+   error = arcmsr_define_adapter_type(acb);
+   if (!error)
+   goto pci_release_regs;
error = arcmsr_remap_pciregion(acb);
if(!error){
goto pci_release_regs;


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] random32: do not feed jiffies as seed from lpfc driver

2014-08-01 Thread Daniel Borkmann

In prandom we have already reseeding mechanisms that trigger
periodically from a much better entropy source than just
feeding in jiffies through lpfc_mbx_cmpl_fcf_scan_read_fcf_rec()
[what a function name 8-)]. Therefore, just remove this.

Signed-off-by: Daniel Borkmann dbork...@redhat.com
Cc: James Bottomley jbottom...@parallels.com
Cc: James Smart james.sm...@emulex.com
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 2a17e31..5072bb2 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -2146,7 +2146,6 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
uint16_t fcf_index, next_fcf_index;
struct lpfc_fcf_rec *fcf_rec = NULL;
uint16_t vlan_id;
-   uint32_t seed;
bool select_new_fcf;
int rc;
 
@@ -2383,9 +2382,6 @@ lpfc_mbx_cmpl_fcf_scan_read_fcf_rec(struct lpfc_hba 
*phba, LPFC_MBOXQ_t *mboxq)
phba-fcf.fcf_flag |= FCF_AVAILABLE;
/* Setup initial running random FCF selection count */
phba-fcf.eligible_fcf_cnt = 1;
-   /* Seeding the random number generator for random selection */
-   seed = (uint32_t)(0x  jiffies);
-   prandom_seed(seed);
}
spin_unlock_irq(phba-hbalock);
goto read_next_fcf;
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.3 2/18] arcmsr: Add code to support MSI-X, MSI interrupt

Hi Alexander,

Thanks for your advice.
This patch was revised according to your comment.

Signed-off-by: Chingching2...@areca.com.tw
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h  2014-04-28 16:02:46.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h  2014-05-06 15:24:36.0 +0800
@@ -64,6 +64,7 @@ struct device_attribute;
 #define ARCMSR_MAX_HBB_POSTQUEUE   
264
 #define ARCMSR_MAX_XFER_LEN
0x26000 /* 152K */
 #define ARCMSR_CDB_SG_PAGE_LENGTH  
256 
+#define ARCMST_NUM_MSIX_VECTORS4
 #ifndef PCI_DEVICE_ID_ARECA_1880
 #define PCI_DEVICE_ID_ARECA_1880 0x1880
  #endif
@@ -508,6 +509,7 @@ struct AdapterControlBlock
struct pci_dev *pdev;
struct Scsi_Host *  host;
unsigned long   vir2phy_offset;
+   struct msix_entry   entries[ARCMST_NUM_MSIX_VECTORS];
/* Offset is used in making arc cdb physical to virtual calculations */
uint32_toutbound_int_enable;
uint32_tcdb_phyaddr_hi32;
@@ -544,6 +546,8 @@ struct AdapterControlBlock
/* iop init */
#define ACB_F_ABORT 0x0200
#define ACB_F_FIRMWARE_TRAP 0x0400
+   #define ACB_F_MSI_ENABLED   0x1000
+   #define ACB_F_MSIX_ENABLED  0x2000
struct CommandControlBlock *
pccb_pool[ARCMSR_MAX_FREECCB_NUM];
/* used for memory free */
struct list_headccb_free_list;
@@ -594,6 +598,7 @@ struct AdapterControlBlock
#define FW_DEADLOCK 0x0010
atomic_trq_map_token;
atomic_tante_token_value;
+   int msix_vector_count;
 };/* HW_DEVICE_EXTENSION */
 /*
 ***
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 11:02:44.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2014-08-01 17:54:46.0 +0800
@@ -603,6 +603,56 @@ static void arcmsr_message_isr_bh_fn(str
}
 }
 
+static int
+arcmsr_request_irq(struct pci_dev *pdev, struct AdapterControlBlock *acb)
+{
+   int i, j, r;
+   struct msix_entry entries[ARCMST_NUM_MSIX_VECTORS];
+
+   for (i = 0; i  ARCMST_NUM_MSIX_VECTORS; i++)
+   entries[i].entry = i;
+   r = pci_enable_msix_range(pdev, entries, 1, ARCMST_NUM_MSIX_VECTORS);
+   if (r  0)
+   goto msi_int;
+   acb-msix_vector_count = r;
+   for (i = 0; i  r; i++) {
+   if (request_irq(entries[i].vector,
+   arcmsr_do_interrupt, 0, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq =%d failed!\n,
+   acb-host-host_no, entries[i].vector);
+   for (j = 0 ; j  i ; j++)
+   free_irq(entries[j].vector, acb);
+   pci_disable_msix(pdev);
+   goto msi_int;
+   }
+   acb-entries[i] = entries[i];
+   }
+   acb-acb_flags |= ACB_F_MSIX_ENABLED;
+   pr_info(arcmsr%d: msi-x enabled\n, acb-host-host_no);
+   return SUCCESS;
+msi_int:
+   if (pci_enable_msi_exact(pdev, 1)  0)
+   goto legacy_int;
+   if (request_irq(pdev-irq, arcmsr_do_interrupt,
+   IRQF_SHARED, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq =%d failed!\n,
+   acb-host-host_no, pdev-irq);
+   pci_disable_msi(pdev);
+   goto legacy_int;
+   }
+   acb-acb_flags |= ACB_F_MSI_ENABLED;
+   pr_info(arcmsr%d: msi enabled\n, acb-host-host_no);
+   return SUCCESS;
+legacy_int:
+   if (request_irq(pdev-irq, arcmsr_do_interrupt,
+   IRQF_SHARED, arcmsr, acb)) {
+   pr_warn(arcmsr%d: request_irq = %d failed!\n,
+   acb-host-host_no, pdev-irq);
+   return FAILED;
+   }
+   return SUCCESS;
+}
+
 static int arcmsr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
struct Scsi_Host *host;
@@ -667,16 +717,13 @@ static int arcmsr_probe(struct pci_dev *
if(error){
goto free_hbb_mu;
}
-   arcmsr_iop_init(acb);
error = scsi_add_host(host, pdev-dev);
if(error){
goto RAID_controller_stop;
}
-   error = request_irq(pdev-irq, arcmsr_do_interrupt, IRQF_SHARED, 
arcmsr, acb);
-   if(error){
+   if (arcmsr_request_irq(pdev, acb) == FAILED)
goto scsi_host_remove;
-   }
-   host-irq =

Re: [PATCH v1.3 2/18] arcmsr: Add code to support MSI-X, MSI interrupt

2014-08-01 Thread Dan Carpenter

On Fri, Aug 01, 2014 at 06:38:48PM +0800, Ching Huang wrote:
 Hi Alexander,
 
 Thanks for your advice.
 This patch was revised according to your comment.
 
 Signed-off-by: Chingching2...@areca.com.tw

This patch is something that can't be applied at all.  There is no
changelog.  Apply the patch with `cat email.txt | git am` and review
the changelog using the `git log` command.

https://www.google.com/search?q=how+to+send+a+v2+patch

Also the Signed-off-by line is wrong.  It should have your full name.
There needs to be a space between the name and the email address.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Save command pool address of Scsi_Host

On Fri, Aug 01, 2014 at 08:27:05AM +0200, jgr...@suse.com wrote:
 From: Juergen Gross jgr...@suse.com
 
 If a scsi host driver specifies .cmd_len in it's scsi_host_template, a 
 driver's
 private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
 scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.
 
 This will result in an access error when the host is removed.
 
 Avoid the problem by saving the address of a new allocated command pool where
 it is expected.
 
 Signed-off-by: Juergen Gross jgr...@suse.com

Looks good, but minor nitpick below:

 + if (shost-hostt-cmd_size)
 + shost-hostt-cmd_pool = pool;
 +


We already have a local hostt variable for the host template in this
function, please use it.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] [PATCH V2 2/4] Introduce xen-scsifront module

On Wed, Jul 30, 2014 at 06:53:59AM +0200, Juergen Gross wrote:
 Hmm, I looked into scsi_add_device(). It seems as if the caller can't
 distinguish between a new created and an already existing device. Am I
 missing something?

That's right.  If you need that I still think it's better to add a
variant of scsi_add_device helping you with that.

 The race is not existing: scsi_add_device() (and scsi_remove_device()
 as well) for this scsi_host is called in scsifront_do_lun_hotplug()
 only, and this function is always called in the same thread (xenbus
 watch). A comment seems to be a good idea.

Do you disable scanning through procfs and sysfs as well?

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] scsi patch queue tree updated

I've pushed out updates to both the core-for-3.17 and drivers-for-3.17
branches.  I think we're in a good shape for the merge window, but
I'd still like to get reviewers attention for a few driver updates that
I'd love to get in still:

 - my eata patch to remove the driver_lock
 - the partially reviewed megaraid series
 - the arcmsr series

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.3 6/18] arcmsr: precise checking adapter ID

 -static void arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
 +static bool arcmsr_define_adapter_type(struct AdapterControlBlock *acb)
  {
   struct pci_dev *pdev = acb-pdev;
   u16 dev_id;
 +
   pci_read_config_word(pdev, PCI_DEVICE_ID, dev_id);
   acb-dev_id = dev_id;

This is already available through pdev-device.


   acb-adapter_type = ACB_ADAPTER_TYPE_C;

Just store the adapter type in the pci_device_id private data field,
that way you enumerate the type in the same place the ids are added
and you'll never miss adding them to a switch value.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1.3 4/18] arcmsr: limit max. number of SCSI command request

 @@ -2220,8 +2220,7 @@ static int arcmsr_queue_command_lck(stru
   arcmsr_handle_virtual_command(acb, cmd);
   return 0;
   }
 - if (atomic_read(acb-ccboutstandingcount) =
 - ARCMSR_MAX_OUTSTANDING_CMD)
 + if (atomic_read(acb-ccboutstandingcount) = acb-maxOutstanding)
   return SCSI_MLQUEUE_HOST_BUSY;

The scsi midlayer already takes care of this check for you.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[V2 PATCH] Save command pool address of Scsi_Host

2014-08-01 Thread jgross

From: Juergen Gross jgr...@suse.com

If a scsi host driver specifies .cmd_len in it's scsi_host_template, a driver's
private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.

This will result in an access error when the host is removed.

Avoid the problem by saving the address of a new allocated command pool where
it is expected.

Signed-off-by: Juergen Gross jgr...@suse.com
---
 drivers/scsi/scsi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe..e8e8428 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -380,6 +380,10 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
pool-slab_flags |= SLAB_CACHE_DMA;
pool-gfp_mask = __GFP_DMA;
}
+
+   if (hostt-cmd_size)
+   hostt-cmd_pool = pool;
+
return pool;
 }
 
-- 
1.8.4.5

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] [PATCH V2 2/4] Introduce xen-scsifront module

2014-08-01 Thread Juergen Gross


On 08/01/2014 02:08 PM, Christoph Hellwig wrote:

On Wed, Jul 30, 2014 at 06:53:59AM +0200, Juergen Gross wrote:

Hmm, I looked into scsi_add_device(). It seems as if the caller can't
distinguish between a new created and an already existing device. Am I
missing something?


That's right.  If you need that I still think it's better to add a
variant of scsi_add_device helping you with that.


I'm open to that solution.

Do you have preferences how to do it (IOW: can you give me a hint)?


The race is not existing: scsi_add_device() (and scsi_remove_device()
as well) for this scsi_host is called in scsifront_do_lun_hotplug()
only, and this function is always called in the same thread (xenbus
watch). A comment seems to be a good idea.


Do you disable scanning through procfs and sysfs as well?


No, I don't.

OTOH I don't think I see the problem. What could go wrong? Either
scsi_device_lookup() does find an existing device, then I refuse to add
it again. If I don't find it, it will be added. I can't see how any harm
would be done in either case when the device is added/removed between
the check and the action. What am I missing?

Juergen
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] iscsi class: fix get_host_stats error handling

2014-08-01 Thread Hannes Reinecke


On 08/01/2014 12:32 AM, Mike Christie wrote:

On 07/30/2014 07:50 AM, Hannes Reinecke wrote:

On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote:

From: Mike Christie micha...@cs.wisc.edu

iscsi_get_host_stats was dropping the error code returned
by drivers like qla4xxx.

Signed-off-by: Mike Christie micha...@cs.wisc.edu
---
   drivers/scsi/scsi_transport_iscsi.c |4 
   1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index b481e62..14bfa53 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3467,6 +3467,10 @@ iscsi_get_host_stats(struct iscsi_transport
*transport, struct nlmsghdr *nlh)
   memset(buf, 0, host_stats_size);

   err = transport-get_host_stats(shost, buf, host_stats_size);
+if (err) {
+kfree(skbhost_stats);
+goto exit_host_stats;
+}

   actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size);
   skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));


What happens with the skbhost_stats allocated earlier? Shouldn't it be
freed here, too?



You mean for this success path right. It is not freed here by the iscsi
code on purpose. For the code path here where we have successfully
called into the driver then a couple lines below we will do

iscsi_multicast_skb() - nlmsg_multicast()

which will pass the skbhost_stats skb to the netlink layer. The
netlink/socket/skb code then frees it when it is done with it.


No, I meant the failure path.

You do an alloc_skb above, then get_host_stats failed, and you exit 
the function without freeing the skb.

Or do I miss something?


Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [scsi/net-next] Pulling in net-next changes

Hi Anish,

Linus plans to do the 3.16 release this weekend, so unless you have
really urgent fixes that require changes from the scsi and net tree I'd
rather avoid the whole issue but waiting for the next merge window.

Can you wait another week or two for these updates?

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Xen-devel] [PATCH V2 2/4] Introduce xen-scsifront module

On Fri, Aug 01, 2014 at 03:06:04PM +0200, Juergen Gross wrote:
 That's right.  If you need that I still think it's better to add a
 variant of scsi_add_device helping you with that.
 
 I'm open to that solution.
 
 Do you have preferences how to do it (IOW: can you give me a hint)?

I thought about it a bit more and came to the conclusion that we
shouldn't bother.  Why do you care if the scsi_add_device actually
added the device?  Any per-device setup should be done in
-slave_configure anyway.

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] iscsi class: fix get_host_stats error handling

2014-08-01 Thread Mike Christie

On 08/01/2014 08:31 AM, Hannes Reinecke wrote:
 On 08/01/2014 12:32 AM, Mike Christie wrote:
 On 07/30/2014 07:50 AM, Hannes Reinecke wrote:
 On 07/12/2014 10:51 PM, micha...@cs.wisc.edu wrote:
 From: Mike Christie micha...@cs.wisc.edu

 iscsi_get_host_stats was dropping the error code returned
 by drivers like qla4xxx.

 Signed-off-by: Mike Christie micha...@cs.wisc.edu
 ---
drivers/scsi/scsi_transport_iscsi.c |4 
1 files changed, 4 insertions(+), 0 deletions(-)

 diff --git a/drivers/scsi/scsi_transport_iscsi.c
 b/drivers/scsi/scsi_transport_iscsi.c
 index b481e62..14bfa53 100644
 --- a/drivers/scsi/scsi_transport_iscsi.c
 +++ b/drivers/scsi/scsi_transport_iscsi.c
 @@ -3467,6 +3467,10 @@ iscsi_get_host_stats(struct iscsi_transport
 *transport, struct nlmsghdr *nlh)
memset(buf, 0, host_stats_size);

err = transport-get_host_stats(shost, buf,
 host_stats_size);
 +if (err) {
 +kfree(skbhost_stats);
 +goto exit_host_stats;
 +}

actual_size = nlmsg_total_size(sizeof(*ev) +
 host_stats_size);
skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));

 What happens with the skbhost_stats allocated earlier? Shouldn't it be
 freed here, too?


 You mean for this success path right. It is not freed here by the iscsi
 code on purpose. For the code path here where we have successfully
 called into the driver then a couple lines below we will do

 iscsi_multicast_skb() - nlmsg_multicast()

 which will pass the skbhost_stats skb to the netlink layer. The
 netlink/socket/skb code then frees it when it is done with it.

 No, I meant the failure path.
 
 You do an alloc_skb above, then get_host_stats failed, and you exit the
 function without freeing the skb.

Ah, actually I am freeing it, but I now realize with the wrong function,
so I think that is why I was confused by your comment.  Above I added a
kfree() of the skb. I should be using kfree_skb. Will fix it up. Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] hpsa: work in progress lockless monster patches

2014-08-01 Thread Webb Scales

On 7/31/14 9:56 AM, Tomas Henzl wrote:

In cmd_tagged_alloc Thus, there should never be a collision here between
two requests if this is true you don't need the refcount and just a simple
flag were sufficient for your other needs. (And maybe you get to ~971k IOPS..)

:-) The code previously had the reference count in there to close
certain race conditions' asynchronous accesses to the command block
(e.g., between an abort and a completing command). We hope that, using
the block layer tags, those races no longer occur, but there didn't seem
to be any point in removing the reference count until we'd had more
experience with the code...and, in a fit of healthy paranoia I added the
check to which you refer. Unfortunately, in some of Rob Elliott's
recent tests, he managed to drive a case where the check triggers. So,
until we reconcile that, I'm inclined to leave the check in place
(hopefully it's not costing a full 1k IOPS :-) ).

Let us assume that the block layer never sends duplicate tags to the driver,
I think there are some weaknesses in the way how it's implemented,
for example here - from fail_all_outstanding_cmds:
refcount = atomic_inc_return(c-refcount);
if (refcount 1) {
...
finish_cmd(c); - this finishes the command
and the tag might be now reused,
when that happens you'll see a race
atomic_dec(h-commands_outstanding);
failcount++;
}
else {what happens when right now comes a call from block
layer?}
The next call from the block layer should find that the controller has
been marked as locked-up and return immediately with a no-connect error
(even before trying to allocate an HPSA command block), so I don't think
there is a race or conflict.

When references are used it usually means, that when refcount==0 that
a release function frees the resources, in this case it is the tag number.
In your implementation you should ensure that when you signal
to the upper layer that the tag is free, that nothing else holds the reference.
Actually, in this case, the resource is the HPSA command block which
corresponds to the tag number. But, you are correct that trying to
manage them separately is apt to cause problems. Having the
cmd_[tagged_]free() routine make the call to scsi_done() -- only when
the reference count is dropping to zero -- is an interesting
suggestion...but I'll have to do considerable investigation before I can
proceed with that.

If this is true the conclusion is that you don't need this kind of references
and a simple flag just to debug not yet fixed races is enough.
I'm maybe wrong because I don't understand what you want to protect
in the above example, so that makes me to have some doubts if I understand
the code properly.
Absent the block layer support, the HPSA code has to be able to defend
against various races where there may be more than two interested
parties looking at the command block -- so a simple flag will not
suffice. On the other hand, with the block layer support in place,
we're hoping that we can get rid of the reference count altogether, but
we're not there, yet.

Webb

On 07/25/2014 09:28 PM, scame...@beardog.cce.hp.com wrote:

hpsa: Work In Progress: lockless monster patches

To be clear, I am not suggesting that these patches be merged at this time.

This patchset is vs. Christoph Hellwig's scsi-mq.4 branch which
may be found here: git://git.infradead.org/users/hch/scsi.git

We've been working for a long time on a patchset for hpsa to remove
all the locks from the main i/o path in pursuit of high IOPS. Some
of that work is already upstream, but a lot more of it is not quite
yet ready to be merged. However, I think we've gone dark for a bit
too long on this, and even though the patches aren't really ready to
be merged just yet, I thought I should let other people who might be
interested have a look anyway, as things are starting to be at least
more stable than unstable. Be warned though, there are still some
problems, esp. around error recovery.

That being said, with the right hardware (fast SAS SSDs, recent Smart
Arrays e.g. P430, with up-to-date firmware, attached to recent disk enclosures)
with these patches and the scsi-mq patches, it is possible to get around
~970k IOPS from a single logical drive on a single controller.

There are about 150 patches in this set. Rather than bomb the list
with that, here is a link to a tarball of the patches in the form of
an stgit patch series:
https://github.com/smcameron/hpsa-lockless-patches-work-in-progress/blob/master/hpsa-lockless-vs-hch-scsi-mq.4-2014-07-25-1415CDT.tar.bz2?raw=true

In some cases, I have probably erred on the side of having too many too
small patches, on the theory that it is easier to bake a cake than to
unbake a cake. Before these are submitted for

Re: [PATCH 4/5] [SCSI] Do not use platform_bus as a parent

2014-08-01 Thread Pawel Moll

On Sun, 2014-07-27 at 16:07 +0100, Greg Kroah-Hartman wrote:
 Ah, ok, it's a scsi core thing, not a driver core thing, that's less
 confusing now.  For a fallback of a platform device, if you switch the
 lines around you should be fine, something like this patch perhaps:
 
 diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
 index 3cbb57a8b846..d8d3b294f5bc 100644
 --- a/drivers/scsi/hosts.c
 +++ b/drivers/scsi/hosts.c
 @@ -218,16 +218,16 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
 struct device *dev,
   goto fail;
  
   if (!shost-shost_gendev.parent)
 - shost-shost_gendev.parent = dev ? dev : platform_bus;
 - if (!dma_dev)
 - dma_dev = shost-shost_gendev.parent;
 -
 - shost-dma_dev = dma_dev;
 + shost-shost_gendev.parent = dev;
  
   error = device_add(shost-shost_gendev);
   if (error)
   goto out;
  
 + if (!dma_dev)
 + dma_dev = shost-shost_gendev.parent;
 + shost-dma_dev = dma_dev;
 +
   pm_runtime_set_active(shost-shost_gendev);
   pm_runtime_enable(shost-shost_gendev);
   device_enable_async_suspend(shost-shost_gendev);

But this will still make shost-dma_dev == NULL in the cases James
quoted:

On Fri, 2014-07-25 at 15:46 +0100, James Bottomley wrote:
 Yes, for DMA purposes, the parent cannot now be NULL; we'll get a panic
 in the DMA transfers if it is.  A lot of the legacy ISA device on x86
 and I thought some ARM SOC devices don't pass in the parent device, so
 we hang them off a known parent.
 
 You can grep for it; these are the devices that will begin to panic if
 you apply this patch:
 
 arch/ia64/hp/sim/simscsi.c: error = scsi_add_host(host, NULL);
 drivers/scsi/a2091.c:   error = scsi_add_host(instance, NULL);
 drivers/scsi/a3000.c:   error = scsi_add_host(instance, NULL);
 drivers/scsi/aha152x.c: if( scsi_add_host(shpnt, NULL) ) {
 drivers/scsi/gdth.c:error = scsi_add_host(shp, NULL);
 drivers/scsi/gdth.c:error = scsi_add_host(shp, NULL);
 drivers/scsi/gvp11.c:   error = scsi_add_host(instance, NULL);
 drivers/scsi/imm.c: err = scsi_add_host(host, NULL);
 drivers/scsi/pcmcia/fdomain_stub.c:if (scsi_add_host(host, NULL))
 drivers/scsi/pcmcia/nsp_cs.c:   ret = scsi_add_host (host, NULL);
 drivers/scsi/pcmcia/qlogic_stub.c:  if (scsi_add_host(shost, NULL))
 drivers/scsi/pcmcia/sym53c500_cs.c: if (scsi_add_host(host, NULL))
 drivers/scsi/ppa.c: err = scsi_add_host(host, NULL);
 drivers/scsi/qlogicfas.c:   if (scsi_add_host(hreg, NULL))
 drivers/scsi/scsi_module.c: error = scsi_add_host(shost, NULL);
 drivers/scsi/sgiwd93.c: err = scsi_add_host(host, NULL);

Maybe the DMA API should be coping with NULL device? It seems to handle
it in a half of the methods and fails in the other half...

Pawel

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/3] ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver.

This patch removes the NCQ support from the APM X-Gene SoC AHCI
Host Controller driver as it doesn't support it.

Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---
 drivers/ata/ahci_xgene.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 1cfbdca..f416495 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -344,7 +344,7 @@ static struct ata_port_operations xgene_ahci_ops = {
 };

 static const struct ata_port_info xgene_ahci_port_info = {
-   .flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
+   .flags = AHCI_FLAG_COMMON,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = xgene_ahci_ops,
@@ -481,7 +481,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
/* Configure the host controller */
xgene_ahci_hw_init(hpriv);

-   hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ;
+   hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ;

rc = ahci_platform_init_host(pdev, hpriv, xgene_ahci_port_info,
 hflags, 0, 0);
--
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/3] arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS node.

The value of the csr-mask of the SATA PHY clock DTS node has a
wrong value resulting a kernel panic as the clock/reset is not
proper for the PHY of the SATA host controller 1. This patch
fixes the correct csr-mask value of the SATA PHY clock DTS node
for the SATA Host controller 1.

As the 'ok' is the default status of a device tree node, this patch
removes the status of the PHY clock node of SATA Host Controller 1.
The status of the clock node is handled from the firmware based on
the controller enabled/disabled by the user.

Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---
 arch/arm64/boot/dts/apm-storm.dtsi | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm-storm.dtsi
index 40aa96c..1bdaeda 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -184,9 +184,8 @@
reg = 0x0 0x1f21c000 0x0 0x1000;
reg-names = csr-reg;
clock-output-names = sataphy1clk;
-   status = disabled;
csr-offset = 0x4;
-   csr-mask = 0x00;
+   csr-mask = 0x3a;
enable-offset = 0x0;
enable-mask = 0x06;
};
@@ -198,7 +197,6 @@
reg = 0x0 0x1f22c000 0x0 0x1000;
reg-names = csr-reg;
clock-output-names = sataphy2clk;
-   status = ok;
csr-offset = 0x4;
csr-mask = 0x3a;
enable-offset = 0x0;
@@ -212,7 +210,6 @@
reg = 0x0 0x1f23c000 0x0 0x1000;
reg-names = csr-reg;
clock-output-names = sataphy3clk;
-   status = ok;
csr-offset = 0x4;
csr-mask = 0x3a;
enable-offset = 0x0;
--
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 3/3] ahci_xgene: Skip the PHY and clock initialization if already configured by the firmware.

This patch implements the feature to skip the PHY and clock
initialization if it is already configured by the firmware.

Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---
 drivers/ata/ahci_xgene.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index f416495..0a87f2e 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -145,6 +145,16 @@ static unsigned int xgene_ahci_qc_issue(struct 
ata_queued_cmd *qc)
return rc;
 }

+static int xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
+{
+   void __iomem *diagcsr = ctx-csr_diag;
+
+   if (readl(diagcsr + CFG_MEM_RAM_SHUTDOWN) == 0 
+   readl(diagcsr + BLOCK_MEM_RDY) == 0x)
+   return 1;
+   return 0;
+}
+
 /**
  * xgene_ahci_read_id - Read ID data from the specified device
  * @dev: device
@@ -468,6 +478,11 @@ static int xgene_ahci_probe(struct platform_device *pdev)
return -ENODEV;
}

+   if (xgene_ahci_is_memram_inited(ctx)) {
+   dev_info(dev, skip clock and PHY initialization\n);
+   goto skip_clk_phy;
+   }
+
/* Due to errata, HW requires full toggle transition */
rc = ahci_platform_enable_clks(hpriv);
if (rc)
@@ -481,6 +496,8 @@ static int xgene_ahci_probe(struct platform_device *pdev)
/* Configure the host controller */
xgene_ahci_hw_init(hpriv);

+skip_clk_phy:
+
hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ;

rc = ahci_platform_init_host(pdev, hpriv, xgene_ahci_port_info,
--
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/3] ahci_xgene: Fixes related to APM X-Gene SATA host controller driver.

This patch set contains a couple of fixes related to APM X-Gene SATA
controller driver.

v2 Change:
   1. Drop the Link down retry patch from this patch set.

v4 Change:
   1. Drop the patch to fix the csr-mask in dts for PHY clock
 node of SATA Host Controller 1.
   2. Add the patch to correct the OOB tunning parameters for
 the COMINIT/COMWAKE parameters.
   3. Add the patch to remove the NCQ support from the APM
 X-Gene AHCI SATA Host controller driver.
   4. Add the patch to remove the clock and PHY reference nodes
 from the APM X-Gene Host controller dts node.

v5 Change :
   1. All the patches are based on 3.16.0-rc6/for-3.17 kernel.
   2. Drop the patch to remove the clock and PHY reference nodes
 from the APM X-Gene Host controller dts node as it breaks
 with old firmware.
   3. Add the patch to skip phy and clock initialisation if
 already done in the firmware.
   4. Add the patch to fix the csr-mask in dts for PHY clock
 node of SATA Host Controller 1.
   5. Add the patch to remove the NCQ support from the APM
 X-Gene AHCI SATA Host controller driver based on 3.16.0-rc6/
 for-3.17 kernel.
   6. Drop the patch to correct the OOB tunning parameters for
 the COMINIT/COMWAKE parameters as it is already applied to
 3.16/for-3.17 branch by the maintainer.
   7. Drop the patch to fix the watermark threshold as it is
 already applied to 3.16/for-3.17 branch by the maintainer.

Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---

Suman Tripathi (3):
  ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA
Host Controller driver.
  arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS
node.
  ahci_xgene: Skip the PHY and clock initialization if already
configured by the firmware.

 arch/arm64/boot/dts/apm-storm.dtsi |  5 +
 drivers/ata/ahci_xgene.c   | 21 +++--
 2 files changed, 20 insertions(+), 6 deletions(-)

--
1.8.2.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags

2014-08-01 Thread Sitsofe Wheeler

On Thu, Jul 24, 2014 at 07:40:36AM +0200, Hannes Reinecke wrote:
 On 07/22/2014 01:06 AM, K. Y. Srinivasan wrote:
 Add blist flags to permit the reading of the VPD pages even when
 the target may claim SPC-2 compliance. MSFT targets currently
 claim SPC-2 compliance while they implement post SPC-2 features.
 With this patch we can correctly handle WRITE_SAME_16 issues.
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 
 Reviewed-by: Hannes Reinecke h...@suse.de

On Wed, Jul 23, 2014 at 09:13:41PM +0100, Sitsofe Wheeler wrote:
 On Wed, Jul 23, 2014 at 07:15:58AM -0700, Christoph Hellwig wrote:
  On Wed, Jul 23, 2014 at 03:10:28PM +0100, Sitsofe Wheeler wrote:
   I'm not sure this alone will work - won't sdev_bflags/bflags have
   already been built at this point?
  
  They've been built up, but we can still or new values into it.  It looks
  fine to me from review, but if you can test it on an actualy hypverv
  setup that would be valueable feedback.
 
 The previous patches didn't work for me with a Windows 2012 R2 host with a
 3.16.0-rc6.x86_64-00076-g2f7d2ec-dirty guest. After applying
 https://patchwork.kernel.org/patch/4541201 (which needed a small fixup) and
 https://patchwork.kernel.org/patch/4598601 and the attached debugging output

I've tested f3cfabce7a2e92564d380de3aad4b43901fb7ae6 (Drivers: add blist
flags) from the drivers-for-3.17 branch of scsi-queue
(http://git.infradead.org/users/hch/scsi-queue.git/commit/f3cfabce7a2e92564d380de3aad4b43901fb7ae6
) and this patch still doesn't appear to work (thin provisioning on
Hyper-V's Virtual Disks is not enabled):

# sg_inq  /dev/sda
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  version=0x04  [SPC-2]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=2
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=1
length=36 (0x24)   Peripheral device type: disk
 Vendor identification: Msft
 Product identification: Virtual Disk
 Product revision level: 1.0 
# sg_vpd -p lbpv /dev/sda
Logical block provisioning VPD page (SBC):
  Unmap command supported (LBPU): 1
  Write same (16) with unmap bit supported (LBWS): 0
  Write same (10) with unmap bit supported (LBWS10): 0
  Logical block provisioning read zeros (LBPRZ): 0
  Anchored LBAs supported (ANC_SUP): 0
  Threshold exponent: 1
  Descriptor present (DP): 0
  Provisioning type: 2

grep . /sys/block/sd*/device/scsi_disk/*/{thin,prov,rotat,device/model}* | sort
/sys/block/sda/device/scsi_disk/0:0:0:0/device/model:Virtual Disk
/sys/block/sda/device/scsi_disk/0:0:0:0/provisioning_mode:full
/sys/block/sda/device/scsi_disk/0:0:0:0/thin_provisioning:0
/sys/block/sdb/device/scsi_disk/1:0:0:0/device/model:Virtual Disk
/sys/block/sdb/device/scsi_disk/1:0:0:0/provisioning_mode:full
/sys/block/sdb/device/scsi_disk/1:0:0:0/thin_provisioning:0
/sys/block/sdc/device/scsi_disk/1:0:0:2/device/model:00AAKS-00TMA0   
/sys/block/sdc/device/scsi_disk/1:0:0:2/provisioning_mode:full
/sys/block/sdc/device/scsi_disk/1:0:0:2/thin_provisioning:0
/sys/block/sdd/device/scsi_disk/1:0:0:1/device/model:SSD S510 120GB  
/sys/block/sdd/device/scsi_disk/1:0:0:1/provisioning_mode:full
/sys/block/sdd/device/scsi_disk/1:0:0:1/thin_provisioning:0
/sys/block/sde/device/scsi_disk/1:0:0:3/device/model:Virtual Disk
/sys/block/sde/device/scsi_disk/1:0:0:3/provisioning_mode:full
/sys/block/sde/device/scsi_disk/1:0:0:3/thin_provisioning:0
/sys/block/sdf/device/scsi_disk/1:0:0:4/device/model:ST1000DM003-9YN1
/sys/block/sdf/device/scsi_disk/1:0:0:4/provisioning_mode:full
/sys/block/sdf/device/scsi_disk/1:0:0:4/thin_provisioning:0

At least the virtual disks should have thin_provisioning... I was also
expecting the passthrough SSD to be reported as non-rotational with this patch:

# sg_inq /dev/sdd
standard INQUIRY:
  PQual=0  Device_type=0  RMB=0  version=0x00  [no conformance claimed]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=0  Resp_data_format=2
  SCCS=0  ACC=0  TPGS=0  3PC=0  Protect=0  [BQue=0]
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=1
  [SPI: Clocking=0x0  QAS=0  IUS=0]
length=60 (0x3c)   Peripheral device type: disk
 Vendor identification: ADATA   
 Product identification: SSD S510 120GB
 Product revision level: 5.2.
 Unit serial number: 0320511550032076
# sg_vpd -p bdc /dev/sdd
Block device characteristics VPD page (SBC):
  Non-rotating medium (e.g. solid state)
  Product type: Not specified
  WABEREQ=0
  WACEREQ=0
  Nominal form factor not reported
  FUAB=0
  VBULS=0
# grep -H . /sys/block/sdd/queue/rot*
/sys/block/sdd/queue/rotational:1

-- 
Sitsofe | http://sucs.org/~sits/
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Save command pool address of Scsi_Host

On Fri, 2014-08-01 at 05:03 -0700, Christoph Hellwig wrote:
 On Fri, Aug 01, 2014 at 08:27:05AM +0200, jgr...@suse.com wrote:
  From: Juergen Gross jgr...@suse.com
  
  If a scsi host driver specifies .cmd_len in it's scsi_host_template, a 
  driver's
  private command pool is needed. scsi_find_host_cmd_pool() will locate it, 
  but
  scsi_alloc_host_cmd_pool() isn't saving the pool address in the host 
  template.
  
  This will result in an access error when the host is removed.
  
  Avoid the problem by saving the address of a new allocated command pool 
  where
  it is expected.
  
  Signed-off-by: Juergen Gross jgr...@suse.com
 
 Looks good, but minor nitpick below:
 
  +   if (shost-hostt-cmd_size)
  +   shost-hostt-cmd_pool = pool;
  +
 
 
 We already have a local hostt variable for the host template in this
 function, please use it.

Wait, that's not right at all.  There looks to be a thinko in the
command pool handling code.  We have both a cmd_pool in the host
structure and in the host template structure, but there's confusion
about which one we're supposed to be using.

The origin of confusion seems to be the reference counting in the pool
itself ... you want the same pool for all hosts, since they can only
have one cmd_size, but you want it created on first host use and
destroyed again on the last one.

If you take this patch, a host that attached, detaches and then attaches
a host will panic because it will use a freed pool structure.

This whole mess is created by the attempt to refcount the pools.  What's
wrong with simply creating the pool at init time and deleting it again
at module removal ... that way no refcounting and no bogus problems like
this (and we can delete the cmd_pool from the host).  The restriction
this would give is that cmd_size can only be set in the template, but
that seems to be the only safe use anyway, since any driver trying to
vary this in its host add routines will get unexpected results.

James


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] scsi patch queue tree updated

On Fri, 2014-08-01 at 05:20 -0700, Christoph Hellwig wrote:
 I've pushed out updates to both the core-for-3.17 and drivers-for-3.17
 branches.

So I'm afraid we missed the last -next build on these, so they can't go
in with the early SCSI pull.  I'm open to doing one mid merge window,
but Linus tends not to like that.

James


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Interrupt strangeness in scsi_request_fn()

2014-08-01 Thread Jan Kara

  Hello,

  when debugging one bug, I've noticed one strangeness in
scsi_request_fn(). We enter it with interrupts disabled and queue_lock
held. In the function we do stuff like:
spin_unlock_irq(shost-host_lock);
/*
 * Finally, initialize any error handling parameters, and
 * set up
 * the timers for timeouts.
 */
scsi_init_cmd_errh(cmd);

/*
 * Dispatch the command to the low-level driver.
 */
rtn = scsi_dispatch_cmd(cmd);
spin_lock_irq(q-queue_lock);

Why do we enable interrupts there? The thing is that scsi_request_fn() can
be called from IO completion (thus softirq context) and if we enable
interrupts there, another HW interrupt can interrupt softirq processing
which seems wrong to me.

Now to admit my motivation I have reports from one customer (SLES kernel so
too old to be really interesting upstream but still of some value I
believe) where he's seeing softlockup reports when fully loading his SAN
and from the crash dumps it seems the machine simply spends too long in IO
completion because HW interrupts keep interrupting it plus there's
contention on shost-host_lock. If I change scsi_request_fn() to never
enable interrupts problems go away.

So what would people say to something like the attached patch?

Honza
-- 
Jan Kara j...@suse.cz
SUSE Labs, CR
From a7c74ea9375cf0b04a11ac3f00bcd835a8499422 Mon Sep 17 00:00:00 2001
From: Jan Kara j...@suse.cz
Date: Fri, 1 Aug 2014 22:45:32 +0200
Subject: [PATCH] scsi: Keep interrupts disabled while submitting requests

scsi_request_fn() can be called from softirq context during IO
completion. If it enables interrupts there, HW interrupts can interrupt
softirq processing and queue more IO completion work which can
eventually lead to softlockup reports because IO completion softirq runs
for too long. Keep interrupts disabled in scsi_request_fn().

Signed-off-by: Jan Kara j...@suse.cz
---
 drivers/scsi/scsi_lib.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f7e316368c99..44b867e9adc9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1481,7 +1481,8 @@ static void scsi_softirq_done(struct request *rq)
  *
  * Returns: Nothing
  *
- * Lock status: IO request lock assumed to be held when called.
+ * Lock status: IO request lock assumed to be held when called, interrupts
+ * must be disabled.
  */
 static void scsi_request_fn(struct request_queue *q)
 	__releases(q-queue_lock)
@@ -1563,7 +1564,7 @@ static void scsi_request_fn(struct request_queue *q)
 		 * XXX(hch): This is rather suboptimal, scsi_dispatch_cmd will
 		 *		take the lock again.
 		 */
-		spin_unlock_irq(shost-host_lock);
+		spin_unlock(shost-host_lock);
 
 		/*
 		 * Finally, initialize any error handling parameters, and set up
@@ -1575,7 +1576,7 @@ static void scsi_request_fn(struct request_queue *q)
 		 * Dispatch the command to the low-level driver.
 		 */
 		rtn = scsi_dispatch_cmd(cmd);
-		spin_lock_irq(q-queue_lock);
+		spin_lock(q-queue_lock);
 		if (rtn)
 			goto out_delay;
 	}
@@ -1583,7 +1584,7 @@ static void scsi_request_fn(struct request_queue *q)
 	return;
 
  not_ready:
-	spin_unlock_irq(shost-host_lock);
+	spin_unlock(shost-host_lock);
 
 	/*
 	 * lock q, handle tag, requeue req, and decrement device_busy. We
@@ -1593,7 +1594,7 @@ static void scsi_request_fn(struct request_queue *q)
 	 * cases (host limits or settings) should run the queue at some
 	 * later time.
 	 */
-	spin_lock_irq(q-queue_lock);
+	spin_lock(q-queue_lock);
 	blk_requeue_request(q, req);
 	sdev-device_busy--;
 out_delay:
-- 
1.8.1.4

Re: Interrupt strangeness in scsi_request_fn()