Re: [PATCH 8/8] IB/srp: Add multichannel support

2014-09-19 Thread Ming Lei
On Fri, Sep 19, 2014 at 9:00 PM, Bart Van Assche  wrote:
> Improve performance by using multiple RDMA/RC channels per SCSI host
> for communicating with an SRP target.
>
> Signed-off-by: Bart Van Assche 
> ---
>  Documentation/ABI/stable/sysfs-driver-ib_srp |  25 +-
>  drivers/infiniband/ulp/srp/ib_srp.c  | 337 
> ---
>  drivers/infiniband/ulp/srp/ib_srp.h  |  20 +-
>  3 files changed, 287 insertions(+), 95 deletions(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-driver-ib_srp 
> b/Documentation/ABI/stable/sysfs-driver-ib_srp
> index b9688de..d5a459e 100644
> --- a/Documentation/ABI/stable/sysfs-driver-ib_srp
> +++ b/Documentation/ABI/stable/sysfs-driver-ib_srp
> @@ -55,12 +55,12 @@ Description:Interface for making ib_srp connect 
> to a new target.
>   only safe with partial memory descriptor list support 
> enabled
>   (allow_ext_sg=1).
> * comp_vector, a number in the range 0..n-1 specifying the
> - MSI-X completion vector. Some HCA's allocate multiple (n)
> - MSI-X vectors per HCA port. If the IRQ affinity masks of
> - these interrupts have been configured such that each MSI-X
> - interrupt is handled by a different CPU then the comp_vector
> - parameter can be used to spread the SRP completion workload
> - over multiple CPU's.
> + MSI-X completion vector of the first RDMA channel. Some
> + HCA's allocate multiple (n) MSI-X vectors per HCA port. If
> + the IRQ affinity masks of these interrupts have been
> + configured such that each MSI-X interrupt is handled by a
> + different CPU then the comp_vector parameter can be used to
> + spread the SRP completion workload over multiple CPU's.
> * tl_retry_count, a number in the range 2..7 specifying the
>   IB RC retry count.
> * queue_size, the maximum number of commands that the
> @@ -88,6 +88,13 @@ Description: Whether ib_srp is allowed to include a 
> partial memory
> descriptor list in an SRP_CMD when communicating with an SRP
> target.
>
> +What:  /sys/class/scsi_host/host/ch_count
> +Date:  November 1, 2014
> +KernelVersion: 3.18
> +Contact:   linux-rdma@vger.kernel.org
> +Description:   Number of RDMA channels used for communication with the SRP
> +   target.
> +
>  What:  /sys/class/scsi_host/host/cmd_sg_entries
>  Date:  May 19, 2011
>  KernelVersion: 2.6.39
> @@ -95,6 +102,12 @@ Contact:linux-rdma@vger.kernel.org
>  Description:   Maximum number of data buffer descriptors that may be sent to
> the target in a single SRP_CMD request.
>
> +What:  /sys/class/scsi_host/host/comp_vector
> +Date:  September 2, 2013
> +KernelVersion: 3.11
> +Contact:   linux-rdma@vger.kernel.org
> +Description:   Completion vector used for the first RDMA channel.
> +
>  What:  /sys/class/scsi_host/host/dgid
>  Date:  June 17, 2006
>  KernelVersion: 2.6.17
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
> b/drivers/infiniband/ulp/srp/ib_srp.c
> index 9feeea1..58ca618 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -123,6 +123,16 @@ MODULE_PARM_DESC(dev_loss_tmo,
>  " if fast_io_fail_tmo has not been set. \"off\" means that"
>  " this functionality is disabled.");
>
> +static unsigned ch_count;
> +module_param(ch_count, uint, 0444);
> +MODULE_PARM_DESC(ch_count,
> +"Number of RDMA channels to use for communication with an 
> SRP"
> +" target. Using more than one channel improves performance"
> +" if the HCA supports multiple completion vectors. The"
> +" default value is the minimum of four times the number of"
> +" online CPU sockets and the number of completion vectors"
> +" supported by the HCA.");
> +
>  static void srp_add_one(struct ib_device *device);
>  static void srp_remove_one(struct ib_device *device);
>  static void srp_recv_completion(struct ib_cq *cq, void *ch_ptr);
> @@ -556,17 +566,32 @@ err:
>   * Note: this function may be called without srp_alloc_iu_bufs() having been
>   * invoked. Hence the ch->[rt]x_ring checks.
>   */
> -static void srp_free_ch_ib(struct srp_rdma_ch *ch)
> +static void srp_free_ch_ib(struct srp_target_port *target,
> +  struct srp_rdma_ch *ch)
>  {
> -   struct srp_target_port *target = ch->target;
> struct srp_device *dev = target->srp_host->srp_dev;
> int i;
>
> +   if (!ch->target)
> +   return;
> +
> +   /*
> +* Avoid that the SCSI error handler tries to use this channel after
> +* it has been freed. Th

Re: [PATCH 8/8] IB/srp: Add multichannel support

2014-09-19 Thread Ming Lei
On Fri, Sep 19, 2014 at 11:21 PM, Bart Van Assche  wrote:
> On 09/19/14 16:28, Ming Lei wrote:
>>
>> On Fri, Sep 19, 2014 at 9:00 PM, Bart Van Assche 
>> wrote:
>>>
>>> @@ -2643,7 +2754,8 @@ static struct scsi_host_template srp_template = {
>>>  .proc_name  = DRV_NAME,
>>>  .slave_configure= srp_slave_configure,
>>>  .info   = srp_target_info,
>>> -   .queuecommand   = srp_queuecommand,
>>> +   .queuecommand   = srp_sq_queuecommand,
>>> +   .mq_queuecommand= srp_mq_queuecommand,
>>
>>
>> Another choice is to obtain hctx from request directly, then mq can
>> reuse the .queuecommand interface too.
>
>
> Hello Ming,
>
> Is the hctx information already available in the request data structure ? I
> have found a mq_ctx member but no hctx member. Did I perhaps overlook
> something ?

You are right, but the mq_ctx can be mapped to hctx like below way:

ctx = rq->mq_ctx;
hctx = q->mq_ops->map_queue(q, ctx->cpu);

Thanks,
--
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 2/8] scsi-mq: Add support for multiple hardware queues

2014-09-26 Thread Ming Lei
On Sat, Sep 20, 2014 at 2:05 AM, Sagi Grimberg  wrote:
> On 9/19/2014 3:57 PM, Bart Van Assche wrote:
>> Allow a SCSI LLD to declare how many hardware queues it supports
>> by setting Scsi_Host.nr_hw_queues before calling scsi_add_host().
>>
>> Note: it is assumed that each hardware queue has a queue depth of
>> shost->can_queue. In other words, the total queue depth per host
>> is (number of hardware queues) * (shost->can_queue).
>>
>> Signed-off-by: Bart Van Assche 
>> Cc: Christoph Hellwig 
>> ---
>>   drivers/scsi/scsi_lib.c  | 2 +-
>>   include/scsi/scsi_host.h | 4 
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index d837dc1..b0b6117 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -2071,7 +2071,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
>>
>>   memset(&shost->tag_set, 0, sizeof(shost->tag_set));
>>   shost->tag_set.ops = &scsi_mq_ops;
>> - shost->tag_set.nr_hw_queues = 1;
>> + shost->tag_set.nr_hw_queues = shost->nr_hw_queues ? : 1;
>>   shost->tag_set.queue_depth = shost->can_queue;
>>   shost->tag_set.cmd_size = cmd_size;
>>   shost->tag_set.numa_node = NUMA_NO_NODE;
>> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
>> index ba20347..0a867d9 100644
>> --- a/include/scsi/scsi_host.h
>> +++ b/include/scsi/scsi_host.h
>> @@ -638,6 +638,10 @@ struct Scsi_Host {
>>   short unsigned int sg_prot_tablesize;
>>   unsigned int max_sectors;
>>   unsigned long dma_boundary;
>> + /*
>> + * In scsi-mq mode, the number of hardware queues supported by the LLD.
>> + */
>> + unsigned nr_hw_queues;
>>   /*
>>   * Used to assign serial numbers to the cmds.
>>   * Protected by the host lock.
>>
>
> I think this patch should be squashed with passing LLD hctx patch (in
> whatever form it ends up).

I suggest to apply this patch and related scsi-mq multi hw-queue
enablement patches first, so that any scsi drivers capable of
multi hw-queue can go without waiting for passing hctx
patches which may take a bit long since lots of drivers are involved,
as Jens said, the mapping from req to hctx is quite cheap.


Thanks,
--
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