| Message ID | 20191202153914.84722-4-hare@suse.de (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | blk-mq/scsi: Provide hostwide shared tags for SCSI HBAs | expand |
On 02/12/2019 15:39, Hannes Reinecke wrote: > The function does not set the depth, but rather transitions from > shared to non-shared queues and vice versa. > So rename it to blk_mq_update_tag_set_shared() to better reflect > its purpose. We will probably need to split this patch into two, as we're doing more than just renaming. The prep patches could be picked up earlier if we're lucky. Thanks, John > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > block/blk-mq-tag.c | 18 ++++++++++-------- > block/blk-mq.c | 8 ++++---- > 2 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c > index d7aa23c82dbf..f5009587e1b5 100644 > --- a/block/blk-mq-tag.c > +++ b/block/blk-mq-tag.c > @@ -440,24 +440,22 @@ static int bt_alloc(struct sbitmap_queue *bt, unsigned int depth, > node); > } > > -static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct blk_mq_tags *tags, > - int node, int alloc_policy) > +static int blk_mq_init_bitmap_tags(struct blk_mq_tags *tags, > + int node, int alloc_policy) > { > unsigned int depth = tags->nr_tags - tags->nr_reserved_tags; > bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR; > > if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node)) > - goto free_tags; > + return -ENOMEM; > if (bt_alloc(&tags->breserved_tags, tags->nr_reserved_tags, round_robin, > node)) > goto free_bitmap_tags; > > - return tags; > + return 0; > free_bitmap_tags: > sbitmap_queue_free(&tags->bitmap_tags); > -free_tags: > - kfree(tags); > - return NULL; > + return -ENOMEM; > } > > struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags, > @@ -478,7 +476,11 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags, > tags->nr_tags = total_tags; > tags->nr_reserved_tags = reserved_tags; > > - return blk_mq_init_bitmap_tags(tags, node, alloc_policy); > + if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) { > + kfree(tags); > + tags = NULL; > + } > + return tags; > } > > void blk_mq_free_tags(struct blk_mq_tags *tags) > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 6b39cf0efdcd..91950d3e436a 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -2581,8 +2581,8 @@ static void queue_set_hctx_shared(struct request_queue *q, bool shared) > } > } > > -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, > - bool shared) > +static void blk_mq_update_tag_set_shared(struct blk_mq_tag_set *set, > + bool shared) > { > struct request_queue *q; > > @@ -2605,7 +2605,7 @@ static void blk_mq_del_queue_tag_set(struct request_queue *q) > /* just transitioned to unshared */ > set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED; > /* update existing queue */ > - blk_mq_update_tag_set_depth(set, false); > + blk_mq_update_tag_set_shared(set, false); > } > mutex_unlock(&set->tag_list_lock); > INIT_LIST_HEAD(&q->tag_set_list); > @@ -2623,7 +2623,7 @@ static void blk_mq_add_queue_tag_set(struct blk_mq_tag_set *set, > !(set->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { > set->flags |= BLK_MQ_F_TAG_QUEUE_SHARED; > /* update existing queue */ > - blk_mq_update_tag_set_depth(set, true); > + blk_mq_update_tag_set_shared(set, true); > } > if (set->flags & BLK_MQ_F_TAG_QUEUE_SHARED) > queue_set_hctx_shared(q, true); >
On 12/3/19 3:30 PM, John Garry wrote: > On 02/12/2019 15:39, Hannes Reinecke wrote: >> The function does not set the depth, but rather transitions from >> shared to non-shared queues and vice versa. >> So rename it to blk_mq_update_tag_set_shared() to better reflect >> its purpose. > > We will probably need to split this patch into two, as we're doing more > than just renaming. > > The prep patches could be picked up earlier if we're lucky. > > Thanks, > John > >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> block/blk-mq-tag.c | 18 ++++++++++-------- >> block/blk-mq.c | 8 ++++---- >> 2 files changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index d7aa23c82dbf..f5009587e1b5 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -440,24 +440,22 @@ static int bt_alloc(struct sbitmap_queue *bt, >> unsigned int depth, >> node); >> } >> -static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct >> blk_mq_tags *tags, >> - int node, int alloc_policy) >> +static int blk_mq_init_bitmap_tags(struct blk_mq_tags *tags, >> + int node, int alloc_policy) >> { >> unsigned int depth = tags->nr_tags - tags->nr_reserved_tags; >> bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR; >> if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node)) >> - goto free_tags; >> + return -ENOMEM; >> if (bt_alloc(&tags->breserved_tags, tags->nr_reserved_tags, >> round_robin, >> node)) >> goto free_bitmap_tags; >> - return tags; >> + return 0; >> free_bitmap_tags: >> sbitmap_queue_free(&tags->bitmap_tags); >> -free_tags: >> - kfree(tags); >> - return NULL; >> + return -ENOMEM; >> } >> struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags, >> @@ -478,7 +476,11 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int >> total_tags, >> tags->nr_tags = total_tags; >> tags->nr_reserved_tags = reserved_tags; >> - return blk_mq_init_bitmap_tags(tags, node, alloc_policy); >> + if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) { >> + kfree(tags); >> + tags = NULL; >> + } >> + return tags; >> } >> void blk_mq_free_tags(struct blk_mq_tags *tags) >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index 6b39cf0efdcd..91950d3e436a 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -2581,8 +2581,8 @@ static void queue_set_hctx_shared(struct >> request_queue *q, bool shared) >> } >> } >> -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, >> - bool shared) >> +static void blk_mq_update_tag_set_shared(struct blk_mq_tag_set *set, >> + bool shared) >> { >> struct request_queue *q; >> @@ -2605,7 +2605,7 @@ static void blk_mq_del_queue_tag_set(struct >> request_queue *q) >> /* just transitioned to unshared */ >> set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED; >> /* update existing queue */ >> - blk_mq_update_tag_set_depth(set, false); >> + blk_mq_update_tag_set_shared(set, false); >> } >> mutex_unlock(&set->tag_list_lock); >> INIT_LIST_HEAD(&q->tag_set_list); >> @@ -2623,7 +2623,7 @@ static void blk_mq_add_queue_tag_set(struct >> blk_mq_tag_set *set, >> !(set->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { >> set->flags |= BLK_MQ_F_TAG_QUEUE_SHARED; >> /* update existing queue */ >> - blk_mq_update_tag_set_depth(set, true); >> + blk_mq_update_tag_set_shared(set, true); >> } >> if (set->flags & BLK_MQ_F_TAG_QUEUE_SHARED) >> queue_set_hctx_shared(q, true); >> > Hmm. That shouldn't have happened; I'm sure I've had it in two patches originally. Oh well. But I'd rather wait for feedback to my shared scheduler tag patch; guess that'll meet with some raise eyebrows ... Cheers, Hannes
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index d7aa23c82dbf..f5009587e1b5 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -440,24 +440,22 @@ static int bt_alloc(struct sbitmap_queue *bt, unsigned int depth, node); } -static struct blk_mq_tags *blk_mq_init_bitmap_tags(struct blk_mq_tags *tags, - int node, int alloc_policy) +static int blk_mq_init_bitmap_tags(struct blk_mq_tags *tags, + int node, int alloc_policy) { unsigned int depth = tags->nr_tags - tags->nr_reserved_tags; bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR; if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node)) - goto free_tags; + return -ENOMEM; if (bt_alloc(&tags->breserved_tags, tags->nr_reserved_tags, round_robin, node)) goto free_bitmap_tags; - return tags; + return 0; free_bitmap_tags: sbitmap_queue_free(&tags->bitmap_tags); -free_tags: - kfree(tags); - return NULL; + return -ENOMEM; } struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags, @@ -478,7 +476,11 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags, tags->nr_tags = total_tags; tags->nr_reserved_tags = reserved_tags; - return blk_mq_init_bitmap_tags(tags, node, alloc_policy); + if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) { + kfree(tags); + tags = NULL; + } + return tags; } void blk_mq_free_tags(struct blk_mq_tags *tags) diff --git a/block/blk-mq.c b/block/blk-mq.c index 6b39cf0efdcd..91950d3e436a 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2581,8 +2581,8 @@ static void queue_set_hctx_shared(struct request_queue *q, bool shared) } } -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, - bool shared) +static void blk_mq_update_tag_set_shared(struct blk_mq_tag_set *set, + bool shared) { struct request_queue *q; @@ -2605,7 +2605,7 @@ static void blk_mq_del_queue_tag_set(struct request_queue *q) /* just transitioned to unshared */ set->flags &= ~BLK_MQ_F_TAG_QUEUE_SHARED; /* update existing queue */ - blk_mq_update_tag_set_depth(set, false); + blk_mq_update_tag_set_shared(set, false); } mutex_unlock(&set->tag_list_lock); INIT_LIST_HEAD(&q->tag_set_list); @@ -2623,7 +2623,7 @@ static void blk_mq_add_queue_tag_set(struct blk_mq_tag_set *set, !(set->flags & BLK_MQ_F_TAG_QUEUE_SHARED)) { set->flags |= BLK_MQ_F_TAG_QUEUE_SHARED; /* update existing queue */ - blk_mq_update_tag_set_depth(set, true); + blk_mq_update_tag_set_shared(set, true); } if (set->flags & BLK_MQ_F_TAG_QUEUE_SHARED) queue_set_hctx_shared(q, true);
The function does not set the depth, but rather transitions from shared to non-shared queues and vice versa. So rename it to blk_mq_update_tag_set_shared() to better reflect its purpose. Signed-off-by: Hannes Reinecke <hare@suse.de> --- block/blk-mq-tag.c | 18 ++++++++++-------- block/blk-mq.c | 8 ++++---- 2 files changed, 14 insertions(+), 12 deletions(-)