Re: [ofa-general][PATCH 3/4] SRP fail-over faster

2009-10-14 Thread David Dillow
On Wed, 2009-10-14 at 15:47 -0700, Roland Dreier wrote: First it does not make sense for user to set it below 60; therefore, it is forced to have 60 and above Why not? A minute seems to be a really long time given the point of these patches is supposed to be failing over

Re: [ofa-general][PATCH 3/4] SRP fail-over faster

2009-10-22 Thread David Dillow
On Thu, 2009-10-22 at 20:24 -0400, Vu Pham wrote: David Dillow wrote: On Thu, 2009-10-22 at 20:04 -0400, Vu Pham wrote: Yes and you can not disable intirely. I'm still looking at benefits/advantages to disable it entirely To me, the advantage is I have a perfectly viable backup path

Re: [ofa-general][PATCH 3/4] SRP fail-over faster

2009-10-28 Thread David Dillow
On Sat, 2009-10-24 at 03:35 -0400, Vu Pham wrote: It's a big improvement from 3-5 minutes cutting down to 1s and now you talk about device_loss_timeout=0. I'll look at the trade-off to have it; however, to receive and process the async event (port error) already cost you a fair amount of

Re: [PATCH] IB/srp: Fix initiator lockup

2010-01-03 Thread David Dillow
On Sat, 2010-01-02 at 13:19 +0100, Bart Van Assche wrote: This patch fixes this issue by adding support for SRP_CRED_REQ information units in the SRP initiator. Additionally, this patch makes the per-session variable req_lim visible through sysfs, which makes observing the initiator state

[RFC PATCH 2/3] IB/srp: minimal support for SRP_CRED_REQ and SRP_AER_REQ

2010-01-03 Thread David Dillow
This patch adds support for SRP_CRED_REQ to avoid a lockup by targets that use that mechanism to return credits to the initiator. This prevents a lockup observed in the field where we would never add the credits from the SRP_CRED_REQ to our current count, and would therefore never send another

[RFC PATCH 3/3] IB/srp: export req_limit via sysfs

2010-01-03 Thread David Dillow
Export the target's current request limit, in case it is useful for debugging. --- I don't feel strongly that this should go in, only that it should be a separate patch if it does. drivers/infiniband/ulp/srp/ib_srp.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff

[RFC PATCH 1/3] IB/srp: differentiate the uses of SRP_SQ_SIZE

2010-01-03 Thread David Dillow
SRP_SQ_SIZE was used in many places that needed to know about the real size of the data structures, and needed to use a magic offset. Additionally, the meaning of the value was different depending on where it was used. Prepare for future work by naming the use appropriately for the site. Also,

Re: [PATCH] IB/srp: Fix initiator lockup

2010-01-04 Thread David Dillow
On Mon, 2010-01-04 at 08:13 +0100, Bart Van Assche wrote: On Mon, Jan 4, 2010 at 2:34 AM, David Dillow d...@thedillows.org wrote: I agree that we should add support for SRP_CRED_REQ, but I'm not thrilled with the mix of changes in the patch, as well as the general aesthetics of the result

Re: [RFC PATCH 2/3] IB/srp: minimal support for SRP_CRED_REQ and SRP_AER_REQ

2010-01-06 Thread David Dillow
On Wed, 2010-01-06 at 13:48 -0800, Roland Dreier wrote: Looks pretty good; a few minor comments: +static int srp_response_common(struct srp_target_port *target, s32 req_delta, + void *rsp, int len); Didn't check -- can we reorder things to avoid this forward

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-06 Thread David Dillow
On Wed, 2010-01-06 at 17:16 -0700, Chris Worley wrote: 1) I'm seeing small block random writes (32KB and smaller) get better performance over SRP than they do as a local drive. I'm guessing this is async behavior: once the written data is on the wire, it's deemed complete, and setting a sync

Re: [PATCH] IB/srp: Fix initiator lockup

2010-01-07 Thread David Dillow
. You are welcome to have a look at the descriptions of the three patches posted by David Dillow. I was not credited in any way in these descriptions. Not for analyzing the problem, not for the design of a solution and not for the work I did in implementing a solution. That is why I wrote that David

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-08 Thread David Dillow
On Fri, 2010-01-08 at 14:40 -0700, Chris Worley wrote: On Wed, Jan 6, 2010 at 6:57 PM, David Dillow d...@thedillows.org wrote: On Wed, 2010-01-06 at 17:16 -0700, Chris Worley wrote: 1) I'm seeing small block random writes (32KB and smaller) get better performance over SRP than they do

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-08 Thread David Dillow
On Fri, 2010-01-08 at 15:39 -0700, Chris Worley wrote: On Fri, Jan 8, 2010 at 3:17 PM, David Dillow d...@thedillows.org wrote: On Fri, 2010-01-08 at 14:40 -0700, Chris Worley wrote: I do set the ib_srp initiator srp_sg_tablesize to its maximum of 58. The max is 255, which will guarantee

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-08 Thread David Dillow
On Fri, 2010-01-08 at 18:07 -0500, David Dillow wrote: But this still isn't hurting you at the small request sizes we seem to be talking about. Or do you mean 58 KB, which is believable -- the default is 12, which guarantees a 48 KB request size is possible, and you'd only need a few pages

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-09 Thread David Dillow
On Sat, 2010-01-09 at 14:05 +0100, Bart Van Assche wrote: The SRP spec says that the target must specify the maximum message size in the SRP_LOGIN_RSP information unit. The largest value one can set the srp_sg_tablesize initiator parameter to is (max. SRP message size defined by the target -

Re: SRP Q's: 1) When is asynchronous I/O complete, 2) Is sequential I/O coalesced, and 3) why is iSCSI faster than SRP in some instances

2010-01-09 Thread David Dillow
On Sat, 2010-01-09 at 18:49 +0100, Bart Van Assche wrote: On Sat, Jan 9, 2010 at 6:16 PM, David Dillow d...@thedillows.org wrote: Does SRPT support the RDMA'ing the indirect buffer descriptors from the Initiator such that it isn't constrained by the partial memory descriptor list

Re: [PATCH] IB/srp: fix race condition on srp_target_port.req_lim

2010-07-25 Thread David Dillow
On Sun, 2010-07-25 at 18:12 +0200, Bart Van Assche wrote: In the current implementation of ib_srp the req_lim field of struct srp_target_port can be manipulated in a non-atomic way by more than one CPU at a time: one CPU can be modifying req_lim in function srp_process_rsp() while another CPU

Re: [PATCH] IB/srp: introduce print_hex_dump()

2010-07-29 Thread David Dillow
On Thu, 2010-07-29 at 17:56 +0200, Bart Van Assche wrote: Reduces the number of source code lines in ib_srp.c by replacing an explicit loop by a call to print_hex_dump(). I like the idea, minor nit in that we loose the SCSI prefix that tells use which host device printed it. Then again it's a

Re: [PATCH 1/4] IB/srp: rename some symbolic constants

2010-08-02 Thread David Dillow
On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: The patch below realizes the following transformations on the symbolic constants defined in ib_srp.h and used in ib_srp.h and ib_srp.c: * Added the constants SRP_RQ_MASK and SRP_SQ_MASK. * Renamed SRP_SQ_SIZE into SRP_REQ_SQ_SIZE. *

Re: [PATCH 2/4] IB/srp: implement SRP_CRED_REQ

2010-08-02 Thread David Dillow
On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: This patch enables allocation of request and response information units on the send queue instead of only requests, and implements processing of SRP_CRED_REQ information units. Also, declarations have been added to include/scsi/srp.h

Re: [PATCH 3/4] IB/srp: adjust can_queue

2010-08-02 Thread David Dillow
On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: Makes sure that the SCSI mid-layer never tries to queue more than (SRP request limit) - 1 SCSI commands. Improves performance for targets whose request limit is less than or equal to SRP_SQ_REQ_SIZE (63) by reducing the number of BUSY

Re: [PATCH 4/4] IB/srp: export req_lim via sysfs

2010-08-02 Thread David Dillow
On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: Exports req_lim via sysfs, which is convenient for debugging purposes. Signed-off-by: Bart Van Assche bart.vanass...@gmail.com Cc: Roland Dreier rola...@cisco.com Cc: David Dillow d...@thedillows.org Acked-by: David Dillow d

Re: [PATCH 2/4] IB/srp: implement SRP_CRED_REQ

2010-08-03 Thread David Dillow
On Tue, 2010-08-03 at 13:40 +0200, Bart Van Assche wrote: On Mon, Aug 2, 2010 at 11:44 PM, David Dillow d...@thedillows.org wrote: On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: [ ... ] There seems to be an accounting change from using tx_head/tx_tail to using tx_req

Re: [PATCH 1/4] IB/srp: rename some symbolic constants

2010-08-03 Thread David Dillow
On Tue, 2010-08-03 at 13:33 +0200, Bart Van Assche wrote: On Mon, Aug 2, 2010 at 11:23 PM, David Dillow d...@thedillows.org wrote: On Mon, 2010-08-02 at 17:32 +0200, Bart Van Assche wrote: [ ... ] @@ -1051,7 +1052,7 @@ static struct srp_iu *__srp_get_tx_iu(struct srp_target_port

Re: [PATCH 3/4 v3] IB/srp: adjust can_queue

2010-08-03 Thread David Dillow
On Tue, 2010-08-03 at 16:08 +0200, Bart Van Assche wrote: The SRP (draft) standard specifies that an SRP initiator must never queue more than (SRP request limit) - 1 unanswered SRP_CMD information units. This patch makes sure that the SCSI mid-layer never tries to queue more than (SRP request

Re: [PATCH 2/4] IB/srp: implement SRP_CRED_REQ

2010-08-03 Thread David Dillow
On Tue, 2010-08-03 at 17:26 +0200, Bart Van Assche wrote: On Tue, Aug 3, 2010 at 5:05 PM, David Dillow d...@thedillows.org wrote: I understood that you were allocating from the same queue of buffers, I'm just not sure you need to keep track separately. We already hold SRP_RSP_SQ_SIZE

Re: About a shortcoming of the verbs API

2010-08-09 Thread David Dillow
On Sun, 2010-08-08 at 20:19 +0200, Bart Van Assche wrote: On Sat, Aug 7, 2010 at 6:32 PM, Roland Dreier rdre...@cisco.com wrote: Not sure that I follow the problem you're worried about. A given tasklet can only be running on one CPU at any one time -- if an interrupt occurs and reschedules

Re: About a shortcoming of the verbs API

2010-08-09 Thread David Dillow
On Mon, 2010-08-09 at 22:45 +0400, Vladislav Bolkhovitin wrote: David Dillow, on 08/09/2010 06:49 PM wrote: I'm not sure it makes sense to enable/disable this at runtime -- we don't do it for NAPI, why do it for block devices? I'm not even sure I'd want to see a config option

Re: [PATCH 2/4] IB/srp: implement SRP_CRED_REQ

2010-08-13 Thread David Dillow
On Tue, 2010-08-10 at 09:55 +0200, Bart Van Assche wrote: On Tue, Aug 3, 2010 at 5:44 PM, David Dillow d...@thedillows.org wrote: On Tue, 2010-08-03 at 17:26 +0200, Bart Van Assche wrote: [ ... ] I'm not sure it is a good idea to allow that all transmit buffers get allocated

Re: [PATCH 1/3 v4] IB/srp: Preparation for transmit ring response allocation

2010-08-13 Thread David Dillow
On Tue, 2010-08-10 at 20:33 +0200, Bart Van Assche wrote: @@ -820,9 +820,11 @@ static int srp_post_recv(struct srp_target_port *target) unsigned int next; int ret; + BUILD_BUG_ON_NOT_POWER_OF_2(SRP_RQ_MASK + 1); + Please put this together with the other build bug in the

Re: [PATCH 2/3 v4] IB/srp: Implement SRP_CRED_REQ

2010-08-13 Thread David Dillow
On Tue, 2010-08-10 at 20:33 +0200, Bart Van Assche wrote: Implements SRP_CRED_REQ, which is an information unit defined in the SRP (draft) standard and that allows an SRP target to inform an SRP initiator that more requests may be sent by the initiator. Adds declarations for the SRP_CRED_REQ

Re: [PATCH 2/3 v4] IB/srp: Implement SRP_CRED_REQ

2010-08-13 Thread David Dillow
On Fri, 2010-08-13 at 15:55 -0400, David Dillow wrote: On Tue, 2010-08-10 at 20:33 +0200, Bart Van Assche wrote: Implements SRP_CRED_REQ, which is an information unit defined in the SRP (draft) standard and that allows an SRP target to inform an SRP initiator that more requests may

Re: [PATCH 1/3 v4] IB/srp: Preparation for transmit ring response allocation

2010-08-14 Thread David Dillow
On Sat, 2010-08-14 at 09:35 +0200, Bart Van Assche wrote: On Fri, Aug 13, 2010 at 8:24 PM, David Dillow d...@thedillows.org wrote: + + rsv = (req_type == SRP_REQ_NORMAL) ? SRP_TSK_MGMT_RSV : 0; srp_send_completion(target-send_cq, target); - if (target-tx_head

Re: [PATCH 2/4] IB/srp: implement SRP_CRED_REQ

2010-08-14 Thread David Dillow
On Sat, 2010-08-14 at 10:21 +0200, Bart Van Assche wrote: On Fri, Aug 13, 2010 at 8:12 PM, David Dillow d...@thedillows.org wrote: On Tue, 2010-08-10 at 09:55 +0200, Bart Van Assche wrote: On Tue, Aug 3, 2010 at 5:44 PM, David Dillow d...@thedillows.org wrote: On Tue, 2010-08-03 at 17

Re: [PATCH 1/3 v5] IB/srp: Preparation for transmit ring response allocation

2010-08-18 Thread David Dillow
On Mon, 2010-08-16 at 20:54 +0200, Bart Van Assche wrote: @@ -989,19 +989,21 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr) static struct srp_iu *__srp_get_tx_iu(struct srp_target_port *target, enum srp_request_type req_type) {

Re: [PATCH 3/3 v5] IB/srp: Reduce number of BUSY conditions

2010-08-18 Thread David Dillow
On Mon, 2010-08-16 at 20:55 +0200, Bart Van Assche wrote: - target-scsi_host-can_queue = min(target-req_lim, - target-scsi_host-can_queue); + /* + * Set can_queue such that the

Re: [PATCH 2/3 v5] IB/srp: Implement SRP_CRED_REQ and SRP_AER_REQ

2010-08-18 Thread David Dillow
On Mon, 2010-08-16 at 20:55 +0200, Bart Van Assche wrote: Implements SRP_CRED_REQ and SRP_AER_REQ, which are information units defined in the SRP (draft) standard. Adds declarations for the SRP_CRED_REQ, SRP_CRED_RSP, SRP_AER_REQ and SRP_AER_RSP information units to include/scsi/srp.h.

Re: [PATCH 1/3 v5] IB/srp: Preparation for transmit ring response allocation

2010-08-19 Thread David Dillow
On Thu, 2010-08-19 at 08:04 +0200, Bart Van Assche wrote: On Thu, Aug 19, 2010 at 1:44 AM, David Dillow d...@thedillows.org wrote: On Mon, 2010-08-16 at 20:54 +0200, Bart Van Assche wrote: @@ -989,19 +989,21 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr

Re: [PATCH 2/3 v5] IB/srp: Implement SRP_CRED_REQ and SRP_AER_REQ

2010-08-19 Thread David Dillow
On Thu, 2010-08-19 at 12:10 +0200, Bart Van Assche wrote: On Thu, Aug 19, 2010 at 2:48 AM, David Dillow d...@thedillows.org wrote: On Mon, 2010-08-16 at 20:55 +0200, Bart Van Assche wrote: Implements SRP_CRED_REQ and SRP_AER_REQ, which are information units defined in the SRP (draft

Re: [PATCH 1/3 v5] IB/srp: Preparation for transmit ring response allocation

2010-08-19 Thread David Dillow
On Thu, 2010-08-19 at 18:53 +0200, Bart Van Assche wrote: On Thu, Aug 19, 2010 at 12:44 PM, David Dillow d...@thedillows.org wrote: Put the assignment on the same line as the declaration and I'll deal with the name. Putting the rsv assignment on the same line as its declaration would make

Re: srp sg_tablesize

2010-08-20 Thread David Dillow
On Fri, 2010-08-20 at 09:49 +0200, Bernd Schubert wrote: In ib_srp.c sg_tablesize is defined as 255. With that value we see lots of IO requests of size 1020. As I already wrote on linux-scsi, that is really sub- optimal for DDN storage, as lots of IO requests of size 1020 come up. Now the

Re: srp sg_tablesize

2010-08-21 Thread David Dillow
On Sat, 2010-08-21 at 13:14 +0200, Bart Van Assche wrote: On Fri, Aug 20, 2010 at 9:49 AM, Bernd Schubert bs_li...@aakef.fastmail.fm wrote: In ib_srp.c sg_tablesize is defined as 255. With that value we see lots of IO requests of size 1020. As I already wrote on linux-scsi, that is

Re: srp sg_tablesize

2010-08-21 Thread David Dillow
On Sat, 2010-08-21 at 19:28 +0200, Bart Van Assche wrote: On Sat, Aug 21, 2010 at 6:27 PM, David Dillow dillo...@ornl.gov wrote: On Sat, 2010-08-21 at 13:14 +0200, Bart Van Assche wrote: The request size of 1020 indicates that there are less than 60 data buffer descriptors

Re: srp sg_tablesize

2010-08-21 Thread David Dillow
On Sat, 2010-08-21 at 20:20 +0200, Bernd Schubert wrote: On Saturday, August 21, 2010, Bart Van Assche wrote: What might help - depending on how the target is implemented - is using an I/O depth larger than one. ib_srp sends all SRP_CMDs with the It depends if we enable write-back cache

Re: srp sg_tablesize

2010-08-24 Thread David Dillow
On Tue, 2010-08-24 at 15:47 -0400, Bernd Schubert wrote: On Friday, August 20, 2010, David Dillow wrote: Currently, we limit sg_tablesize to 255 because we can only cache 255 indirect memory descriptors in the SRP_CMD message to the target. That's due to the count being in an 8 bit field

Re: [PATCH 5/5 v6] IB/srp: Introduce list_first_entry()

2010-08-24 Thread David Dillow
: Roland Dreier rola...@cisco.com Acked-by: David Dillow d...@thedillows.org -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SIW: iWARP Protocol headers

2010-10-06 Thread David Dillow
On Wed, 2010-10-06 at 11:25 -0600, Jason Gunthorpe wrote: On Wed, Oct 06, 2010 at 02:52:46PM +0200, Bernard Metzler wrote: It is actually a little more complicated than just this. I assume you are casting the structures over packet payloads? In this case you have to guarentee alignment (or

[PATCH 3/5] IB/srp: eliminate two forward declarations

2010-10-16 Thread David Dillow
Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 142 +-- 1 files changed, 69 insertions(+), 73 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index a54eee9..d4c08d6

[PATCH 1/5] IB/srp: Preparation for transmit ring response allocation

2010-10-16 Thread David Dillow
of SRP_SQ_SIZE, increases the size of the IB send completion queue by one element and reserves one transmit ring slot for SRP_TSK_MGMT requests. Signed-off-by: Bart Van Assche bvanass...@acm.org Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 27

[PATCH 4/5] IB/srp: Reduce number of BUSY conditions

2010-10-16 Thread David Dillow
for targets whose request limit is less than or equal to SRP_NORMAL_REQ_SQ_SIZE by reducing the number of BUSY responses reported by ib_srp to the SCSI mid-layer. Signed-off-by: Bart Van Assche bvanass...@acm.org Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c

[PATCH 5/5] IB/srp: Introduce list_first_entry()

2010-10-16 Thread David Dillow
From: Bart Van Assche bvanass...@acm.org Introduces the list_first_entry() macro in ib_srp, which makes the source code slightly more descriptive. The list_first_entry() macro itself was introduced in kernel 2.6.22. Signed-off-by: Bart Van Assche bvanass...@acm.org Signed-off-by: David Dillow

Re: [PATCH 0/5] IB/srp: Add SRP_CRED_REQ support

2010-10-17 Thread David Dillow
On Sun, 2010-10-17 at 10:51 +0200, Bart Van Assche wrote: Information about why part of the implementation has been reworked is missing. Information about whether or not the reworked implementation has been tested is missing too. Bart, This series has been tested as thoroughly as I possibly

Re: [PATCH 2/5] IB/srp: implement SRP_CRED_REQ and SRP_AER_REQ

2010-10-18 Thread David Dillow
On Mon, 2010-10-18 at 09:10 -0400, David Dillow wrote: On Mon, 2010-10-18 at 06:11 -0400, Bart Van Assche wrote: As far as I can see the above sync call applies to a buffer in the tx_ring[]. Data in that buffer is only modified by the CPU and never by the HCA. So why is the above sync call

Re: DMA sync question

2010-10-18 Thread David Dillow
On Mon, 2010-10-18 at 06:11 -0400, Bart Van Assche wrote: On Sun, Oct 17, 2010 at 5:25 AM, David Dillow dillo...@ornl.gov wrote: [ ... ] +static int srp_response_common(struct srp_target_port *target, s32 req_delta, + void *rsp, int len

Re: [PATCH 2/5] IB/srp: implement SRP_CRED_REQ and SRP_AER_REQ

2010-10-18 Thread David Dillow
at this, I noticed that we didn't do any syncs in srp_send_tsk_mgmt(), so I've pushed this patch for now. We can remove the *_sync_for_cpu() calls later if it turns out we really don't need them. commit 155d75eeb12a09d574688c0ad98c6a24fed5bbcd Author: David Dillow dillo...@ornl.gov Date: Mon Oct 18 08:54

Re: [PATCH 2/5] IB/srp: implement SRP_CRED_REQ and SRP_AER_REQ

2010-10-23 Thread David Dillow
On Sat, 2010-10-23 at 12:47 +0200, Bart Van Assche wrote: On Mon, Oct 18, 2010 at 3:10 PM, David Dillow dillo...@ornl.gov wrote: [ ... ] While looking at this, I noticed that we didn't do any syncs in srp_send_tsk_mgmt(), so I've pushed this patch for now. We can remove

Re: Possible bug on SRP device delete

2010-12-15 Thread David Dillow
On Mon, 2010-12-13 at 17:32 +0100, torn5 wrote: I have been suggested to the RDMA mailing list as it might be an ib_srp bug: 5) If I echo one line of ibsrpdm -c to /sys/class/infiniband_srp/srp-mthca0-1/add_target so to add a srp disk to my system, what

[RFC 2/8] IB/srp: consolidate state change code

2010-12-23 Thread David Dillow
This is a break-out and slight cleanup of Bart Van Assche's work. --- drivers/infiniband/ulp/srp/ib_srp.c | 45 ++ 1 files changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index

[RFC 3/8] IB/srp: allow lockless work posting

2010-12-23 Thread David Dillow
Only one CPU at a time will own an RX IU, so using the address of the IU as the work request cookie allows us to avoid taking a lock. We can similarly prepare the TX path for lockless posting by moving the free TX IUs to a list. This also removes the requirement that the queue sizes be a power of

[RFC 1/8] IB/srp: allow task management without a previous request

2010-12-23 Thread David Dillow
it is. This fixes a crash when we issue a bus reset using sg_reset. Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=13893 Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 90 -- drivers/infiniband/ulp/srp/ib_srp.h | 10 ++-- 2

[RFC 7/8] IB/srp: stop sharing the host lock with SCSI

2010-12-23 Thread David Dillow
We don't need protection against the SCSI stack, so use our own lock to allow parallel progress on separate CPUs. This is a break-out of Bart Van Assche's work. --- drivers/infiniband/ulp/srp/ib_srp.c | 42 +- drivers/infiniband/ulp/srp/ib_srp.h |2 + 2

[RFC 5/8] IB/srp: reduce local coverage for command submission and EH

2010-12-23 Thread David Dillow
We only need locks to protect our lists and number of credits available. By pre-consuming the credit for the request, we can reduce our lock coverage to just those areas. If we don't actually send the request, we'll need to put the credit back into the pool. This is a break-out and slight cleanup

[RFC 8/8] IB/srp: consolidate hot-path variables into cache lines

2010-12-23 Thread David Dillow
Put the variables accessed together in the hot-path into common cachelines, and separate them by RW vs RO to avoid false dirtying. We keep a local copy of the lkey and rkey in the target to avoid traversing pointers (and associated cache lines) to find them. ---

Re: [RFC 1/8] IB/srp: allow task management without a previous request

2010-12-27 Thread David Dillow
On Mon, 2010-12-27 at 16:01 +0100, Bart Van Assche wrote: On Thu, Dec 23, 2010 at 10:31 PM, David Dillow dillo...@ornl.gov wrote: We can only have one task management comment outstanding, so move the completion and status to the target port. This allows us to handle resets of a LUN

Re: [RFC 2/8] IB/srp: consolidate state change code

2010-12-27 Thread David Dillow
On Mon, 2010-12-27 at 16:12 +0100, Bart Van Assche wrote: On Thu, Dec 23, 2010 at 10:31 PM, David Dillow dillo...@ornl.gov wrote: This is a break-out and slight cleanup of Bart Van Assche's work. +static bool srp_change_state(struct srp_target_port *target, + enum

Re: [RFC 3/8] IB/srp: allow lockless work posting

2010-12-27 Thread David Dillow
On Mon, 2010-12-27 at 16:37 +0100, Bart Van Assche wrote: On Thu, Dec 23, 2010 at 10:31 PM, David Dillow dillo...@ornl.gov wrote: [ ... ] @@ -876,7 +876,7 @@ static int __srp_post_send(struct srp_target_port *target, ret = ib_post_send(target-qp, wr, bad_wr); if (!ret

Re: [RFC 4/8] IB/srp: don't move active requests to their own list

2010-12-27 Thread David Dillow
On Mon, 2010-12-27 at 16:50 +0100, Bart Van Assche wrote: On Thu, Dec 23, 2010 at 10:31 PM, David Dillow dillo...@ornl.gov wrote: We use req-scmnd != NULL to indicate an active request, so there's no need to keep a separate list for them. We can afford the array iteration during error

Re: [RFC 7/8] IB/srp: stop sharing the host lock with SCSI

2010-12-27 Thread David Dillow
On Thu, 2010-12-23 at 16:31 -0500, David Dillow wrote: We don't need protection against the SCSI stack, so use our own lock to allow parallel progress on separate CPUs. @@ -1126,7 +1125,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd) return 0

Re: [RFC 7/8] IB/srp: stop sharing the host lock with SCSI

2010-12-27 Thread David Dillow
On Mon, 2010-12-27 at 19:56 +0200, Boaz Harrosh wrote: On 12/27/2010 07:49 PM, David Dillow wrote: On Thu, 2010-12-23 at 16:31 -0500, David Dillow wrote: We don't need protection against the SCSI stack, so use our own lock to allow parallel progress on separate CPUs. @@ -1126,7 +1125,7

Re: [RFC 6/8] IB/srp: reduce lock coverage of command completion

2011-01-03 Thread David Dillow
On Sun, 2011-01-02 at 18:27 +0100, Bart Van Assche wrote: Before this patch target-req_lim was updated both when receiving a response for a regular SRP command and when receiving a response for an SRP task management command. This patch modifies that in only updating target-req_lim in the

Re: [RFC 5/8] IB/srp: reduce local coverage for command submission and EH

2011-01-03 Thread David Dillow
On Sun, 2011-01-02 at 18:16 +0100, Bart Van Assche wrote: On Thu, Dec 23, 2010 at 10:31 PM, David Dillow dillo...@ornl.gov wrote: We only need locks to protect our lists and number of credits available. By pre-consuming the credit for the request, we can reduce our lock coverage to just

Re: [RFC 0/8] IB/srp: scaling and bug fixes for 2.6.38

2011-01-05 Thread David Dillow
On Wed, 2011-01-05 at 10:09 -0800, Roland Dreier wrote: I don't have anything to add beyond Bart's feedback -- this all appears quite well-tested and broken down into reasonable steps. So I'd like to get your updated (and signed-off) patch queue for the 2.6.38 merge window if you get a

[PATCH v2 6/8] IB/srp: reduce lock coverage of command completion

2011-01-05 Thread David Dillow
before we've released our request and added any credits returned by the target. This prevents us from returning HOST_BUSY unneccesarily. Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out, small cleanups, and modified to avoid potential extraneous HOST_BUSY returns by David Dillow

[PATCH v2 5/8] IB/srp: reduce local coverage for command submission and EH

2011-01-05 Thread David Dillow
into the pool. Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out and small cleanups by David Dillow ] Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 124 +++ drivers/infiniband/ulp/srp/ib_srp.h |1 - 2 files changed, 67

[PATCH v2 2/8] IB/srp: consolidate state change code

2011-01-05 Thread David Dillow
From: Bart Van Assche bvanass...@acm.org Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out and small cleanups by David Dillow ] Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 45 ++ 1 files changed, 24

[PATCH v2 3/8] IB/srp: allow lockless work posting

2011-01-05 Thread David Dillow
the requirement that the queue sizes be a power of 2. Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out, small cleanups, and modified to avoid needing an extra field in the IU by David Dillow] Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 65

[PATCH v2 0/8] scaling and bug fixes for 2.6.38

2011-01-05 Thread David Dillow
the host lock with SCSI David Dillow (8): IB/srp: allow task management without a previous request IB/srp: consolidate hot-path variables into cache lines drivers/infiniband/ulp/srp/ib_srp.c | 392 --- drivers/infiniband/ulp/srp/ib_srp.h | 46 +++-- 2 files

[PATCH v2 1/8] IB/srp: allow task management without a previous request

2011-01-05 Thread David Dillow
it is. This fixes a crash when we issue a bus reset using sg_reset. Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=13893 Reported-by: Bart Van Assche bvanass...@acm.org Reviewed-by: Bart Van Assche bvanass...@acm.org Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c

[PATCH v2 4/8] IB/srp: don't move active requests to their own list

2011-01-05 Thread David Dillow
Van Assche bvanass...@acm.org [ broken out and small cleanups by David Dillow ] Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 21 - drivers/infiniband/ulp/srp/ib_srp.h |1 - 2 files changed, 12 insertions(+), 10 deletions(-) diff

[PATCH v2 8/8] IB/srp: consolidate hot-path variables into cache lines

2011-01-05 Thread David Dillow
...@acm.org Signed-off-by: David Dillow dillo...@ornl.gov --- drivers/infiniband/ulp/srp/ib_srp.c | 12 +++- drivers/infiniband/ulp/srp/ib_srp.h | 31 +++ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b

[PATCH v2 7/8] IB/srp: stop sharing the host lock with SCSI

2011-01-05 Thread David Dillow
From: Bart Van Assche bvanass...@acm.org We don't need protection against the SCSI stack, so use our own lock to allow parallel progress on separate CPUs. Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out and small cleanups by David Dillow ] Signed-off-by: David Dillow dillo

Mellanox target workaround in SRP

2011-01-07 Thread David Dillow
I have question regarding workaround introduced in commit 559ce8f1 of the mainline tree: IB/srp: Work around data corruption bug on Mellanox targets Data corruption has been seen with Mellanox SRP targets when FMRs create a memory region with I/O virtual address != 0. Add a

Re: Mellanox target workaround in SRP

2011-01-08 Thread David Dillow
On Fri, 2011-01-07 at 20:05 -0800, Roland Dreier wrote: I'm sure this was tested and shown to fix the problem; I'm just confused as to what the problem really was and if this is still relevant. Can someone please enlighten me? At this point I'm afraid it's all lost in the mists of time,

Re: [PATCH v2 3/8] IB/srp: allow lockless work posting

2011-01-09 Thread David Dillow
On Sun, 2011-01-09 at 16:59 +0100, Bart Van Assche wrote: On Wed, Jan 5, 2011 at 9:35 PM, David Dillow dillo...@ornl.gov wrote: From: Bart Van Assche bvanass...@acm.org + list_del_init(target-free_tx); + for (i = 0; i SRP_SQ_SIZE; ++i) + list_move(target

Re: [PATCH v2 2/8] IB/srp: consolidate state change code

2011-01-09 Thread David Dillow
On Sun, 2011-01-09 at 16:43 +0100, Bart Van Assche wrote: On Wed, Jan 5, 2011 at 9:35 PM, David Dillow dillo...@ornl.gov wrote: From: Bart Van Assche bvanass...@acm.org Signed-off-by: Bart Van Assche bvanass...@acm.org [ broken out and small cleanups by David Dillow ] Signed-off

Re: [PATCH v2 0/8] scaling and bug fixes for 2.6.38

2011-01-09 Thread David Dillow
On Sun, 2011-01-09 at 17:01 +0100, Bart Van Assche wrote: On Wed, Jan 5, 2011 at 9:35 PM, David Dillow dillo...@ornl.gov wrote: Updated patches to reflect the list's review, and better reflect Bart's role in preparing them. Once he's had a chance to look over them again, I'll push

Re: [ewg] Mellanox target workaround in SRP

2011-01-10 Thread David Dillow
On Mon, 2011-01-10 at 10:21 -0800, Vu Pham wrote: David Dillow wrote: On Fri, 2011-01-07 at 20:05 -0800, Roland Dreier wrote: looking at the patch, I would guess that the corruption occurred when the target got an IO request that started at a non-page-aligned address but that spanned more

Re: [ewg] Mellanox target workaround in SRP

2011-01-10 Thread David Dillow
On Mon, 2011-01-10 at 10:49 -0800, Roland Dreier wrote: I think that the patch is specific for srp initiator using Mellanox FMR. It tried to avoid indirect desc with Mellanox FMR having first-byte-offset != 0. Since the low level implementation of mlx4/mthca_map_phys_fmr() did not

Re: [PATCH v2 3/8] IB/srp: allow lockless work posting

2011-01-10 Thread David Dillow
On Sun, 2011-01-09 at 11:35 -0500, David Dillow wrote: On Sun, 2011-01-09 at 16:59 +0100, Bart Van Assche wrote: On Wed, Jan 5, 2011 at 9:35 PM, David Dillow dillo...@ornl.gov wrote: From: Bart Van Assche bvanass...@acm.org + list_del_init(target-free_tx); + for (i = 0; i

[GIT] lock scaling and bug fixes for 2.6.38

2011-01-10 Thread David Dillow
submission and EH IB/srp: reduce lock coverage of command completion IB/srp: stop sharing the host lock with SCSI David Dillow (8): IB/srp: allow task management without a previous request IB/srp: consolidate hot-path variables into cache lines drivers/infiniband/ulp/srp/ib_srp.c | 392

Re: srp warning

2011-01-12 Thread David Dillow
On Wed, 2011-01-12 at 12:43 -0500, Roland Dreier wrote: Just noticed this in my latest build: drivers/infiniband/ulp/srp/ib_srp.c: In function ‘srp_queuecommand’: drivers/infiniband/ulp/srp/ib_srp.c:1116: warning: ‘req’ may be used uninitialized in this function [snip] and my gcc

Re: srp warning

2011-01-12 Thread David Dillow
On Wed, 2011-01-12 at 13:02 -0500, Roland Dreier wrote: Out of curiosity, what version are you using? I've been using 4.4.5 and 4.5.1. $ gcc --version gcc (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5 (on x86-64). Not sure what changed because I don't think I saw it before either. Could you

Re: srp warning

2011-01-13 Thread David Dillow
On Thu, 2011-01-13 at 14:10 -0500, Bart Van Assche wrote: On Thu, Jan 13, 2011 at 4:21 PM, David Dillow dillo...@ornl.gov wrote: Please drop the unlikely -- it isn't at all unlikely on the hardware and loads I run. [snip] But it surprises me that on the hardware and loads you run

Re: [PATCH] ib_srp: Test once whether iu allocation succeeded

2011-01-13 Thread David Dillow
On Thu, 2011-01-13 at 20:02 +0100, Bart Van Assche wrote: Merge the two tests whether information unit allocation succeeded into one. An intended side effect of this change is that gcc 4.4.4 does no longer complain that the variable 'req' might be used uninitialized in the function

IB: increase DMA max_segment_size on Mellanox hardware

2011-01-16 Thread David Dillow
GB, though there is no HW limit. Signed-off-by: David Dillow dillo...@ornl.gov -- It could be 3 GB or even (4 GB - 1), since it's an unsigned int, but we should rarely see a 2 GB request anyways. I think it is probably safe to do something similar for the qib and ipath drivers, and perhaps

Re: [ewg] Mellanox target workaround in SRP

2011-01-16 Thread David Dillow
On Mon, 2011-01-10 at 11:58 -0800, Vu Pham wrote: David Dillow wrote: either. The SRP FMR mapping code is careful to mask the SG address with the FMR page mask, so we should never ask the HCA to map a page with the first_byte_offset != 0. Instead, we tell the target to request an IO

Re: IB: increase DMA max_segment_size on Mellanox hardware

2011-01-18 Thread David Dillow
On Tue, 2011-01-18 at 09:16 +0200, Or Gerlitz wrote: David Dillow wrote: We're talking about different things -- max_segments(sg_tablesize)/max_sectors are the limits as we're adding pages to the bio via bio_add_page(). blk_rq_map_sg() uses max_segment_size as a bound on the largest

Re: [ewg] Mellanox target workaround in SRP

2011-01-18 Thread David Dillow
On Tue, 2011-01-18 at 11:53 -0800, Vu Pham wrote: Our hw/fw guys confirm that there is no problem, my suspect is wrong. To explain clearly how hw translate from remote rdma address to physical address in fmr's MTT X = requested/rdma_va - MPT.start + MPT.fbo MTT index = X / MPT.blocksize

[RFC 6/8] IB/srp: add support for indirect tables that don't fit in SRP_CMD

2011-01-18 Thread David Dillow
This allows us to guarantee the ability to submit up to 8 MB requests based on the current value of SCSI_MAX_SG_CHAIN_SEGMENTS. While FMR will usually condense the requests into 8 SG entries, it is imperative that the target support external tables in case the FMR mapping fails or is not

[RFC 0/8] Reliably generate large request from SRP

2011-01-18 Thread David Dillow
. This is an unexpected improvement and needs further examination. I've only played with performance testing; I need to test data integrity as well. David Dillow (8): IB/srp: always avoid non-zero offsets into an FMR IB/srp: move IB CM setup completion into its own function IB/srp: allow sg_tablesize

  1   2   3   >