Re: [patch,v3 00/10] make I/O path allocations more numa-friendly
On 11/09/12 20:17, Jeff Moyer wrote: This patch set makes memory allocations for data structures used in the I/O path more numa friendly by allocating them from the same numa node as the storage device. I've only converted a handful of drivers at this point. My testing showed that, for workloads where the I/O processes were not tied to the numa node housing the device, a speedup of around 6% was observed. When the I/O processes were tied to the numa node of the device, there was no measurable difference in my test setup. Given my relatively low-end setup[1], I wouldn't be surprised if others could show a more significant performance advantage. Comments would be greatly appreciated. Sorry but I'm not familiar with any of the SCSI LLDs modified via this patch series. But I'm fine with the SCSI core patches in this series. So if you want you can add the following to the first five patches in this series: Reviewed-by: Bart Van Assche bvanass...@acm.org Bart. -- 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 RESEND v4 0/5] Migrate SCSI drivers to use dev_pm_ops
Hi James, Care to comment? Thanks, Aaron On 11/09/2012 03:27 PM, Aaron Lu wrote: This patchset has been quiet for a while, so resend them. v4: Only Patch 4 is modified: Fixed a line over 80 characters warning by checkpatch.pl; Update the changelog so that it is no more a try :-) v3: Only patch 4 is modified: Remove the special case for system freeze in scsi_bus_suspend_common as pointed out by Alan Stern; Updated some comments; Removed the use of typedef (*pm_callback_t)(struct device *). v2: Change the runtime suspend behaviour of sd driver by putting the device into stopped power state. Revert 2 patches which are no longer needed as pointed out by Alan Stern. Find out device callbacks in bus callbacks as suggested by Alan Stern. Due to these changes, patch number grows from 2 - 5. v1: The 2 patches will migrate SCSI drivers to use the pm callbacks defined in dev_pm_ops as pm_message is deprecated and should not be used by driver. Bus level callback is changed to use callbacks defined in dev_pm_ops when needed and sd's pm callback is updated to use what are defined in dev_pm_ops. Aaron Lu (5): sd: put to stopped power state when runtime suspend Revert [SCSI] scsi_pm: set device runtime state before parent suspended Revert [SCSI] runtime resume parent for child's system-resume pm: use callbacks from dev_pm_ops for scsi devices sd: update sd to use the new pm callbacks drivers/scsi/scsi_pm.c | 98 +++--- drivers/scsi/sd.c | 18 +++--- 2 files changed, 67 insertions(+), 49 deletions(-) -- 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] scsi: prevent stack buffer overflow in host_reset
Am Donnerstag 15 November 2012, 15:51:46 schrieb Sasha Levin: store_host_reset() has tried to re-invent the wheel to compare sysfs strings. Unfortunately it did so poorly and never bothered to check the input from userspace before overwriting stack with it, so something simple as: echo WoopsieWoopsie /sys/devices/pseudo_0/adapter0/host0/scsi_host/host0/host_reset would result in: [ 316.310101] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: 81f5bac7 [ 316.310101] [ 316.320051] Pid: 6655, comm: sh Tainted: GW 3.7.0-rc5-next-20121114-sasha-00016-g5c9d68d-dirty #129 [ 316.320051] Call Trace: [ 316.340058] pps pps0: PPS event at 1352918752.620355751 [ 316.340062] pps pps0: capture assert seq #303 [ 316.320051] [83b3856b] panic+0xcd/0x1f4 [ 316.320051] [81f5bac7] ? store_host_reset+0xd7/0x100 [ 316.320051] [8110b996] __stack_chk_fail+0x16/0x20 [ 316.320051] [81f5bac7] store_host_reset+0xd7/0x100 [ 316.320051] [81e55bb3] dev_attr_store+0x13/0x30 [ 316.320051] [812f7db1] sysfs_write_file+0x101/0x170 [ 316.320051] [8127acc8] vfs_write+0xb8/0x180 [ 316.320051] [8127ae80] sys_write+0x50/0xa0 [ 316.320051] [83c03418] tracesys+0xe1/0xe6 Fix this by uninventing whatever was going on there and just use sysfs_streq. Bug introduced by 29443691 ([SCSI] scsi: Added support for adapter and firmware reset). Signed-off-by: Sasha Levin sasha.le...@oracle.com That revision is in 3.2 and all following, so I think this needs to go into stable, too. Eike signature.asc Description: This is a digitally signed message part.
Re: [scsi:misc 36/36] drivers/scsi/csiostor/csio_attr.c:92:41: sparse: incorrect type in assignment (different base types)
On 11/15/2012 11:59 PM, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git misc head: 0779fa137a73170b6c796fdb476f1455ba67b9aa commit: 0779fa137a73170b6c796fdb476f1455ba67b9aa [36/36] [SCSI] csiostor: Chelsio FCoE offload driver sparse warnings: Thanks for your report. My run of 'make C=2' to run sparse never reported these. Is there someway I can run your tool locally so I can clean up my next set of patches before sending them? With respect to the warnings, will correct type-casting help me get rid of most of them? Regards, Naresh. -- 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 3/5] arcmsr: Support MSI and MSI-X
From: Nick Cheng nick.ch...@areca.com.tw Support MSI or MSI-X for whole series of RAID controllers. Meanwhile correct the register access as iowrite32/ioread32 Signed-off-by: Nick Cheng nick.ch...@areca.com.tw --- patch3 Description: Binary data
[PATCH 2/5] arcmsr: Support Hibernation
From: Nick Cheng nick.ch...@areca.com.tw Support hibernation for whole series of RAID controllers Signed-off-by: Nick Cheng nick.ch...@areca.com.tw --- patch2 Description: Binary data
[PATCH 5/5] arcmsr: Modify ARC-1214 IO Behavior
From: Nick Cheng nick.ch...@areca.com.tw Modify ARC-1214 IO behavior to make up for HW seldom malfunction. Signed-off-by: Nick Cheng nick.ch...@areca.com.tw --- patch5 Description: Binary data
Re: [PATCH 2/3] target: Add max_write_same_len device attribute
Il 15/11/2012 20:23, Nicholas A. Bellinger ha scritto: This patch adds a new max_write_same_len device attribute for use with WRITE_SAME w/ UNMAP=0 backend emulation. Also, update block limits VPD emulation code in spc_emulate_evpd_b0() to set the default MAXIMUM WRITE SAME LENGTH value of zero. why do we need an exposed attribute for this? This is useful for userspace to reduce the allowed maximum from the default 0x set by IBLOCK. Allowing huge WRITE_SAMEs can very much effect performance (esp. with spinning media), so being able to reduce the max we accept via a userspace tunable is helpful. Unfortunately this doesn't really help. Linux will submit the smaller WRITE SAMEs in parallel, and this could easily bring the target to its knees. (This was reported to me with a Linux virtual machine sending WRITE SAME commands to a Nexenta iSCSI target running OpenSolaris. QEMU can be easily replaced with LIO, with the same effect). Paolo -- 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
UAPI disintegration for SCSI, FCoE and RDMA
Hi James, Robert, David, I have patches still outstanding for the UAPI header disintegration of linux/scsi/, linux/scsi/fs/ and linux/rdma. I have updated them from the original pull requests and they can be found at tags: disintegrate-scsi-20121116 disintegrate-fc-20121116 disintegrate-rdma-20121116 in: git://git.infradead.org/users/dhowells/linux-headers.git http://git.infradead.org/users/dhowells/linux-headers.git Do you accept git pull requests or only patches through the mailing lists? Should all three go via the SCSI tree? Note that the MAINTAINERS' record for RDMA points to a defunct git tree. Thanks, David -- 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:misc 36/36] drivers/scsi/csiostor/csio_attr.c:92:41: sparse: incorrect type in assignment (different base types)
Hi Naresh, On Fri, Nov 16, 2012 at 03:41:53PM +0530, Naresh Kumar Inna wrote: On 11/15/2012 11:59 PM, kbuild test robot wrote: tree: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git misc head: 0779fa137a73170b6c796fdb476f1455ba67b9aa commit: 0779fa137a73170b6c796fdb476f1455ba67b9aa [36/36] [SCSI] csiostor: Chelsio FCoE offload driver sparse warnings: Thanks for your report. My pleasure! My run of 'make C=2' to run sparse never reported these. Is there someway I can run your tool locally so I can clean up my next set of patches before sending them? This will enable more sparse type checks: make C=1 CF=-D__CHECK_ENDIAN__ With respect to the warnings, will correct type-casting help me get rid of most of them? As long as the type-casting is *correct* ;-) Thanks, Fengguang -- 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 0/4] Make aborts work on tcm_qla2xxx, other cleanups
From: Roland Dreier rol...@purestorage.com Hi Nic, Here's a series that makes aborts actually work on qla2xxx. Stopping and releasing commands is quite convoluted so I'm not sure the first patch is totally correct, but without it I can easily reproduce task hangs or list corruption by having an initiator flood a tcm_qla2xxx target with aborts. With those fixes, Steve's patch is pretty straightforward. The last two patches are just cleanups I noticed while debugging this. Just to be clear: to the extent that this is copyrightable work, it is released exclusively under the GPL. No permission is granted to redistribute this under any other terms. - R. Roland Dreier (3): target: Fix handling of aborted commands target: Clean up logic in transport_put_cmd() target: Clean up flow in transport_check_aborted_status() Steve Hodgson (1): qla2xxx: Look up LUN for abort requests drivers/scsi/qla2xxx/qla_target.c | 19 ++- drivers/target/target_core_transport.c | 40 ++-- 2 files changed, 36 insertions(+), 23 deletions(-) -- 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 1/4] target: Fix handling of aborted commands
From: Roland Dreier rol...@purestorage.com - If we stop processing an already-aborted command in target_execute_cmd(), then we need to complete t_transport_stop_comp to wake up the the TMR handling thread, or else it will end up waiting forever. - If we've a already sent an aborted status for a command in transport_check_aborted_status() then we should bail out of transport_send_task_abort() to avoid freeing the command twice. Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/target/target_core_transport.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 9097155..dcecbfb 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1819,8 +1819,10 @@ void target_execute_cmd(struct se_cmd *cmd) /* * If the received CDB has aleady been aborted stop processing it here. */ - if (transport_check_aborted_status(cmd, 1)) + if (transport_check_aborted_status(cmd, 1)) { + complete(cmd-t_transport_stop_comp); return; + } /* * Determine if IOCTL context caller in requesting the stopping of this @@ -3067,7 +3069,7 @@ void transport_send_task_abort(struct se_cmd *cmd) unsigned long flags; spin_lock_irqsave(cmd-t_state_lock, flags); - if (cmd-se_cmd_flags SCF_SENT_CHECK_CONDITION) { + if (cmd-se_cmd_flags (SCF_SENT_CHECK_CONDITION | SCF_SENT_DELAYED_TAS)) { spin_unlock_irqrestore(cmd-t_state_lock, flags); return; } -- 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 2/4] qla2xxx: Look up LUN for abort requests
From: Steve Hodgson st...@purestorage.com Search through the list of pending commands on the session list to find the command the initiator is actually aborting, so that we can pass the correct LUN to the core TMR handling code. Signed-off-by: Steve Hodgson st...@purestorage.com Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/scsi/qla2xxx/qla_target.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 62aa558..79fbdd7 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1264,9 +1264,26 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, struct abts_recv_from_24xx *abts, struct qla_tgt_sess *sess) { struct qla_hw_data *ha = vha-hw; + struct se_session *se_sess = sess-se_sess; struct qla_tgt_mgmt_cmd *mcmd; + struct se_cmd *se_cmd; + u32 lun = 0; int rc; + spin_lock(se_sess-sess_cmd_lock); + list_for_each_entry(se_cmd, se_sess-sess_cmd_list, se_cmd_list) { + struct qla_tgt_cmd *cmd = + container_of(se_cmd, struct qla_tgt_cmd, se_cmd); + if (cmd-tag == abts-exchange_addr_to_abort) { + lun = cmd-unpacked_lun; + break; + } + } + spin_unlock(se_sess-sess_cmd_lock); + + if (!lun) + return -ENOENT; + ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00f, qla_target(%d): task abort (tag=%d)\n, vha-vp_idx, abts-exchange_addr_to_abort); @@ -1283,7 +1300,7 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, mcmd-sess = sess; memcpy(mcmd-orig_iocb.abts, abts, sizeof(mcmd-orig_iocb.abts)); - rc = ha-tgt.tgt_ops-handle_tmr(mcmd, 0, TMR_ABORT_TASK, + rc = ha-tgt.tgt_ops-handle_tmr(mcmd, lun, TMR_ABORT_TASK, abts-exchange_addr_to_abort); if (rc != 0) { ql_dbg(ql_dbg_tgt_mgt, vha, 0xf052, -- 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 3/4] target: Clean up logic in transport_put_cmd()
From: Roland Dreier rol...@purestorage.com No need to have a goto where a return is clearer. Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/target/target_core_transport.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index dcecbfb..0f29d70 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2183,9 +2183,10 @@ static void transport_put_cmd(struct se_cmd *cmd) unsigned long flags; spin_lock_irqsave(cmd-t_state_lock, flags); - if (atomic_read(cmd-t_fe_count)) { - if (!atomic_dec_and_test(cmd-t_fe_count)) - goto out_busy; + if (atomic_read(cmd-t_fe_count) + !atomic_dec_and_test(cmd-t_fe_count)) { + spin_unlock_irqrestore(cmd-t_state_lock, flags); + return; } if (cmd-transport_state CMD_T_DEV_ACTIVE) { @@ -2196,9 +2197,6 @@ static void transport_put_cmd(struct se_cmd *cmd) transport_free_pages(cmd); transport_release_cmd(cmd); - return; -out_busy: - spin_unlock_irqrestore(cmd-t_state_lock, 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 4/4] target: Clean up flow in transport_check_aborted_status()
From: Roland Dreier rol...@purestorage.com Signed-off-by: Roland Dreier rol...@purestorage.com --- drivers/target/target_core_transport.c | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 0f29d70..f225f90 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3042,23 +3042,19 @@ EXPORT_SYMBOL(transport_send_check_condition_and_sense); int transport_check_aborted_status(struct se_cmd *cmd, int send_status) { - int ret = 0; + if (!(cmd-transport_state CMD_T_ABORTED)) + return 0; - if (cmd-transport_state CMD_T_ABORTED) { - if (!send_status || -(cmd-se_cmd_flags SCF_SENT_DELAYED_TAS)) - return 1; + if (!send_status || (cmd-se_cmd_flags SCF_SENT_DELAYED_TAS)) + return 1; - pr_debug(Sending delayed SAM_STAT_TASK_ABORTED -status for CDB: 0x%02x ITT: 0x%08x\n, - cmd-t_task_cdb[0], - cmd-se_tfo-get_task_tag(cmd)); + pr_debug(Sending delayed SAM_STAT_TASK_ABORTED status for CDB: 0x%02x ITT: 0x%08x\n, +cmd-t_task_cdb[0], cmd-se_tfo-get_task_tag(cmd)); - cmd-se_cmd_flags |= SCF_SENT_DELAYED_TAS; - cmd-se_tfo-queue_status(cmd); - ret = 1; - } - return ret; + cmd-se_cmd_flags |= SCF_SENT_DELAYED_TAS; + cmd-se_tfo-queue_status(cmd); + + return 1; } EXPORT_SYMBOL(transport_check_aborted_status); -- 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
Re: [PATCH] target/iblock: Fix double iblock_complete_cmd callback bug
On Thu, Nov 15, 2012 at 02:36:30PM -0800, Nicholas A. Bellinger wrote: However, setting the default ibr-pending=2 value before dispatch, and making a extra iblock_complete_cmd() call from iblock_execute_rw(), while doing the normal iblock_complete_cmd() calls from iblock_bio_done() is AFAICT a pointless extra atomic_dec_and_test() call per I/O. Was there a reason why you changed -pending from 1 - 2 during the se_task removal in commit 5787cacd0bd5ee01..? It is to avoid having the request completed before even submitting all bios. As soon as one is submitted it could on a very fast device complete before we've incremented the count for the next bio. It's a very common scheme used all over the kernel when submitting I/O in smaller subdivisions. So the case this patch tries to avoid is where iblock_submit_bio() is called multiple times before the final ibr-pending value is set, this could potentially cause bios completion calls to decrement the value + complete to core before iblock_execute_rw() is done incrementing ibr-pending. That's exacltly what we try to avoid here. How about returning an exception here instead when IBLOCK_MAX_BIOS in reached..? Why? As soon as we kicked the first batch off we're guaranteed to make progress allocating more as sson as the first of the submitted ones completes. Btw, where did the default of 32 for this come from..? It's a random number, with the important property that it's considerably smaller than IBLOCK_BIO_POOL_SIZE. -- 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 v2] [SCSI] mvsas: fix undefined bit shift
The macro bit(n) is defined as ((u32)1 n), and thus it doesn't work with n = 32, such as in mvs_94xx_assign_reg_set(): if (i = 32) { mvi-sata_reg_set |= bit(i); ... } The shift ((u32)1 n) with n = 32 also leads to undefined behavior. The result varies depending on the architecture. This patch changes bit(n) to do a 64-bit shift. It also simplifies mv_ffc64() using __ffs64(), since invoking ffz() with ~0 is undefined. Signed-off-by: Xi Wang xi.w...@gmail.com Cc: Xiangliang Yu yuxia...@marvell.com Cc: James Bottomley jbottom...@parallels.com Cc: sta...@vger.kernel.org --- As suggested by James, v2 is a single patch with a stable tag. --- drivers/scsi/mvsas/mv_94xx.h | 14 ++ drivers/scsi/mvsas/mv_sas.h |2 +- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/mvsas/mv_94xx.h b/drivers/scsi/mvsas/mv_94xx.h index 8f7eb4f..487aa6f 100644 --- a/drivers/scsi/mvsas/mv_94xx.h +++ b/drivers/scsi/mvsas/mv_94xx.h @@ -258,21 +258,11 @@ enum sas_sata_phy_regs { #define SPI_ADDR_VLD_94XX (1U 1) #define SPI_CTRL_SpiStart_94XX (1U 0) -#define mv_ffc(x) ffz(x) - static inline int mv_ffc64(u64 v) { - int i; - i = mv_ffc((u32)v); - if (i = 0) - return i; - i = mv_ffc((u32)(v32)); - - if (i != 0) - return 32 + i; - - return -1; + u64 x = ~v; + return x ? __ffs64(x) : -1; } #define r_reg_set_enable(i) \ diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h index c04a4f5..da24955 100644 --- a/drivers/scsi/mvsas/mv_sas.h +++ b/drivers/scsi/mvsas/mv_sas.h @@ -69,7 +69,7 @@ extern struct kmem_cache *mvs_task_list_cache; #define DEV_IS_EXPANDER(type) \ ((type == EDGE_DEV) || (type == FANOUT_DEV)) -#define bit(n) ((u32)1 n) +#define bit(n) ((u64)1 n) #define for_each_phy(__lseq_mask, __mc, __lseq)\ for ((__mc) = (__lseq_mask), (__lseq) = 0; \ -- 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