diff mbox

[v2,01/12] blk-mq: Use all available hardware queues

Message ID 5433E473.4020102@acm.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Bart Van Assche Oct. 7, 2014, 1:02 p.m. UTC
Suppose that a system has two CPU sockets, three cores per socket,
that it does not support hyperthreading and that four hardware
queues are provided by a block driver. With the current algorithm
this will lead to the following assignment of CPU cores to hardware
queues:

  HWQ 0: 0 1
  HWQ 1: 2 3
  HWQ 2: 4 5
  HWQ 3: (none)

This patch changes the queue assignment into:

  HWQ 0: 0 1
  HWQ 1: 2
  HWQ 2: 3 4
  HWQ 3: 5

In other words, this patch has the following three effects:
- All four hardware queues are used instead of only three.
- CPU cores are spread more evenly over hardware queues. For the
  above example the range of the number of CPU cores associated
  with a single HWQ is reduced from [0..2] to [1..2].
- If the number of HWQ's is a multiple of the number of CPU sockets
  it is now guaranteed that all CPU cores associated with a single
  HWQ reside on the same CPU socket.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
Cc: Jens Axboe <axboe@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@canonical.com>
---
 block/blk-mq-cpumap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jens Axboe Oct. 7, 2014, 2:37 p.m. UTC | #1
On 10/07/2014 07:02 AM, Bart Van Assche wrote:
> Suppose that a system has two CPU sockets, three cores per socket,
> that it does not support hyperthreading and that four hardware
> queues are provided by a block driver. With the current algorithm
> this will lead to the following assignment of CPU cores to hardware
> queues:
> 
>   HWQ 0: 0 1
>   HWQ 1: 2 3
>   HWQ 2: 4 5
>   HWQ 3: (none)
> 
> This patch changes the queue assignment into:
> 
>   HWQ 0: 0 1
>   HWQ 1: 2
>   HWQ 2: 3 4
>   HWQ 3: 5
> 
> In other words, this patch has the following three effects:
> - All four hardware queues are used instead of only three.
> - CPU cores are spread more evenly over hardware queues. For the
>   above example the range of the number of CPU cores associated
>   with a single HWQ is reduced from [0..2] to [1..2].
> - If the number of HWQ's is a multiple of the number of CPU sockets
>   it is now guaranteed that all CPU cores associated with a single
>   HWQ reside on the same CPU socket.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
> Cc: Jens Axboe <axboe@fb.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ming Lei <ming.lei@canonical.com>
> ---
>  block/blk-mq-cpumap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
> index 1065d7c..8e56455 100644
> --- a/block/blk-mq-cpumap.c
> +++ b/block/blk-mq-cpumap.c
> @@ -17,7 +17,7 @@
>  static int cpu_to_queue_index(unsigned int nr_cpus, unsigned int nr_queues,
>  			      const int cpu)
>  {
> -	return cpu / ((nr_cpus + nr_queues - 1) / nr_queues);
> +	return cpu * nr_queues / nr_cpus;
>  }
>  
>  static int get_first_sibling(unsigned int cpu)

Lets do this separate, as explained last time, it needs to be evaluated
on its own and doesn't really belong in this series of patches.
Bart Van Assche Oct. 8, 2014, 1:21 p.m. UTC | #2
On 10/07/14 16:37, Jens Axboe wrote:
> Lets do this separate, as explained last time, it needs to be evaluated
> on its own and doesn't really belong in this series of patches.

Hello Jens,

A few minutes ago I have resent this patch to you with the LKML in CC. I 
hope that Christoph agrees with leaving out this patch from this series 
without me having to resend this series. BTW, Sagi promised me off-list 
to review the other patches in this series.

Bart.


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Oct. 11, 2014, 11:11 a.m. UTC | #3
On Wed, Oct 08, 2014 at 03:21:56PM +0200, Bart Van Assche wrote:
> On 10/07/14 16:37, Jens Axboe wrote:
> >Lets do this separate, as explained last time, it needs to be evaluated
> >on its own and doesn't really belong in this series of patches.
> 
> Hello Jens,
> 
> A few minutes ago I have resent this patch to you with the LKML in CC. I
> hope that Christoph agrees with leaving out this patch from this series
> without me having to resend this series. BTW, Sagi promised me off-list to
> review the other patches in this series.

Ignoring patches is one of the easier tasks, np.  Do you want me to
merge all the srp updates?  They normally go through the IB tree, but
I can pick them up this time so that we don't need to synchronize the
two trees.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bart Van Assche Oct. 13, 2014, 9:45 a.m. UTC | #4
On 10/11/14 13:11, Christoph Hellwig wrote:
> On Wed, Oct 08, 2014 at 03:21:56PM +0200, Bart Van Assche wrote:
>> On 10/07/14 16:37, Jens Axboe wrote:
>>> Lets do this separate, as explained last time, it needs to be evaluated
>>> on its own and doesn't really belong in this series of patches.
>>
>> Hello Jens,
>>
>> A few minutes ago I have resent this patch to you with the LKML in CC. I
>> hope that Christoph agrees with leaving out this patch from this series
>> without me having to resend this series. BTW, Sagi promised me off-list to
>> review the other patches in this series.
>
> Ignoring patches is one of the easier tasks, np.  Do you want me to
> merge all the srp updates?  They normally go through the IB tree, but
> I can pick them up this time so that we don't need to synchronize the
> two trees.

Hello Christoph,

Since patch 1/12 already has been sent separately to Jens patches 
2/12..12/12 remain. The SRP initiator changes in this series depend on 
the blk-mq and scsi-mq features added in patches 2/12..4/12. I think we 
should avoid splitting these patches over multiple kernel trees to avoid 
having to deal with dependencies between kernel trees.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Oct. 17, 2014, 1:20 p.m. UTC | #5
On Mon, Oct 13, 2014 at 11:45:43AM +0200, Bart Van Assche wrote:
> Since patch 1/12 already has been sent separately to Jens patches
> 2/12..12/12 remain. The SRP initiator changes in this series depend on the
> blk-mq and scsi-mq features added in patches 2/12..4/12. I think we should
> avoid splitting these patches over multiple kernel trees to avoid having to
> deal with dependencies between kernel trees.

I'd like to pull it in and Roland already did indicate that he's fine
with it.

Sagi, can I get a review from you for the remaining SRP patches?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sagi Grimberg Oct. 17, 2014, 2:11 p.m. UTC | #6
>> On Mon, Oct 13, 2014 at 11:45:43AM +0200, Bart Van Assche wrote:
>> Since patch 1/12 already has been sent separately to Jens patches
>> 2/12..12/12 remain. The SRP initiator changes in this series depend on the
>> blk-mq and scsi-mq features added in patches 2/12..4/12. I think we should
>> avoid splitting these patches over multiple kernel trees to avoid having to
>> deal with dependencies between kernel trees.
> 
> I'd like to pull it in and Roland already did indicate that he's fine
> with it.
> 
> Sagi, can I get a review from you for the remaining SRP patches?

As I promised Bart, it's on my todo list. Unfortunately this week I was violently pulled to another project...
It's a holiday here in Israel, so I'm planning to review the set by mid next week.

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

Patch

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 1065d7c..8e56455 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -17,7 +17,7 @@ 
 static int cpu_to_queue_index(unsigned int nr_cpus, unsigned int nr_queues,
 			      const int cpu)
 {
-	return cpu / ((nr_cpus + nr_queues - 1) / nr_queues);
+	return cpu * nr_queues / nr_cpus;
 }
 
 static int get_first_sibling(unsigned int cpu)