All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Gordeev <agordeev@redhat.com>
To: Omar Sandoval <osandov@osandov.com>
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org
Subject: Re: [PATCH 04/14] blk-mq: Do not limit number of queues to 'nr_cpu_ids' in allocations
Date: Tue, 20 Sep 2016 13:44:36 +0200	[thread overview]
Message-ID: <20160920114435.GC26808@agordeev.lab.eng.brq.redhat.com> (raw)
In-Reply-To: <20160919174849.GA21803@vader>

On Mon, Sep 19, 2016 at 10:48:49AM -0700, Omar Sandoval wrote:
> On Sun, Sep 18, 2016 at 09:37:14AM +0200, Alexander Gordeev wrote:
> > Currently maximum number of used hardware queues is limited to
> > number of CPUs in the system. However, using 'nr_cpu_ids' as
> > the limit for (de-)allocations of data structures instead of
> > existing data structures' counters (a) worsens readability and
> > (b) leads to unused memory when number of hardware queues is
> > less than number of CPUs.
> > 
> > CC: linux-block@vger.kernel.org
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > ---
> >  block/blk-mq.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 276ec7b..2c77b68 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -2054,8 +2054,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> >  	if (!q->queue_ctx)
> >  		goto err_exit;
> >  
> > -	q->queue_hw_ctx = kzalloc_node(nr_cpu_ids * sizeof(*(q->queue_hw_ctx)),
> > -						GFP_KERNEL, set->numa_node);
> > +	q->queue_hw_ctx = kzalloc_node(set->nr_hw_queues *
> > +			sizeof(*(q->queue_hw_ctx)), GFP_KERNEL, set->numa_node);
> >  	if (!q->queue_hw_ctx)
> >  		goto err_percpu;
> >  
> > @@ -2319,7 +2319,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
> >  	if (set->nr_hw_queues > nr_cpu_ids)
> >  		set->nr_hw_queues = nr_cpu_ids;
> >  
> > -	set->tags = kzalloc_node(nr_cpu_ids * sizeof(struct blk_mq_tags *),
> > +	set->tags = kzalloc_node(set->nr_hw_queues * sizeof(*set->tags),
> >  				 GFP_KERNEL, set->numa_node);
> >  	if (!set->tags)
> >  		return -ENOMEM;
> > @@ -2360,7 +2360,7 @@ void blk_mq_free_tag_set(struct blk_mq_tag_set *set)
> >  {
> >  	int i;
> >  
> > -	for (i = 0; i < nr_cpu_ids; i++) {
> > +	for (i = 0; i < set->nr_hw_queues; i++) {
> >  		if (set->tags[i])
> >  			blk_mq_free_rq_map(set, set->tags[i], i);
> >  	}
> 
> I don't think this is safe since we might increase the number of
> hardware queues (blk_mq_update_nr_hw_queues()).

It is safe, because blk_mq_update_nr_hw_queues() limits number of
hardware queues to nr_cpu_ids. But still using nr_cpu_ids is wrong,
because (a) set->nr_hw_queues could be less than nr_cpu_ids and
(b) it is set->nr_hw_queues counter that tracks size of the array.

> -- 
> Omar

  reply	other threads:[~2016-09-20 11:38 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-18  7:37 [PATCH 00/14] blk-mq: Minor fixes and cleanups Alexander Gordeev
2016-09-18  7:37 ` [PATCH 01/14] blk-mq: Fix memory leaks on queue cleanup Alexander Gordeev
2016-09-19 13:51   ` Christoph Hellwig
2016-09-18  7:37 ` [PATCH 02/14] blk-mq: Fix a potential NULL pointer assignment to hctx tags Alexander Gordeev
2016-09-19 18:34   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 03/14] block: Get rid of unused request_queue::nr_queues member Alexander Gordeev
2016-09-19 18:37   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 04/14] blk-mq: Do not limit number of queues to 'nr_cpu_ids' in allocations Alexander Gordeev
2016-09-19 17:48   ` Omar Sandoval
2016-09-20 11:44     ` Alexander Gordeev [this message]
2016-09-20 17:26       ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 05/14] blk-mq: Remove a redundant assignment Alexander Gordeev
2016-09-19 18:38   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 06/14] blk-mq: Fix hardware context data node selection Alexander Gordeev
2016-09-19 18:54   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 07/14] blk-mq: Cleanup a loop exit condition Alexander Gordeev
2016-09-19 19:00   ` Omar Sandoval
2016-09-20 11:33     ` Alexander Gordeev
2016-09-18  7:37 ` [PATCH 08/14] blk-mq: Get rid of unnecessary blk_mq_free_hw_queues() Alexander Gordeev
2016-09-18  7:37 ` [PATCH 09/14] blk-mq: Move duplicating code to blk_mq_exit_hctx() Alexander Gordeev
2016-09-19 17:57   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 10/14] blk-mq: Uninit hardware context in order reverse to init Alexander Gordeev
2016-09-19 17:59   ` Omar Sandoval
2016-09-18  7:37 ` [PATCH 11/14] blk-mq: Move hardware context init code into single location Alexander Gordeev
2016-09-18  7:37 ` [PATCH 12/14] blk-mq: Rework blk_mq_init_hctx() function Alexander Gordeev
2016-09-18  7:37 ` [PATCH 13/14] blk-mq: Pair blk_mq_hctx_kobj_init() with blk_mq_hctx_kobj_put() Alexander Gordeev
2016-09-18  7:37 ` [PATCH 14/14] blk-mq: Set flush_start_tag to BLK_MQ_MAX_DEPTH Alexander Gordeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160920114435.GC26808@agordeev.lab.eng.brq.redhat.com \
    --to=agordeev@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=osandov@osandov.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.