Re: [PATCH 2/7] block: Remove bio->bi_ioc

From: Adam Manzanares <Adam.Manzanares@wdc.com>
To: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	"axboe@kernel.dk" <axboe@kernel.dk>
Cc: "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 2/7] block: Remove bio->bi_ioc
Date: Mon, 19 Nov 2018 19:07:18 +0000	[thread overview]
Message-ID: <cd28e72b1a597f1915948e280fe3e2468e44f524.camel@wdc.com> (raw)
In-Reply-To: <20181119035131.11255-3-damien.lemoal@wdc.com>

On Mon, 2018-11-19 at 12:51 +0900, Damien Le Moal wrote:
> bio->bi_ioc is never set so always NULL. Remove references to it in
> bio_disassociate_task() and in rq_ioc() and delete this field from
> struct bio. With this change, rq_ioc() always returns
> current->io_context without the need for a bio argument. Further
> simplify the code and make it more readable by also removing this
> helper, which also allows to simplify blk_mq_sched_assign_ioc() by
> removing its bio argument.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Looks good,

Reviewed-by: Adam Manzanares <adam.manzanares@wdc.com>

Reviewed by: 
> ---
>  block/bio.c               |  4 ----
>  block/blk-core.c          |  2 +-
>  block/blk-mq-sched.c      |  4 ++--
>  block/blk-mq-sched.h      |  2 +-
>  block/blk-mq.c            |  4 ++--
>  block/blk.h               | 16 ----------------
>  include/linux/blk_types.h |  3 +--
>  7 files changed, 7 insertions(+), 28 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 4f4d9884443b..03895cc0d74a 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -2027,10 +2027,6 @@ int bio_associate_blkg(struct bio *bio, struct
> blkcg_gq *blkg)
>   */
>  void bio_disassociate_task(struct bio *bio)
>  {
> -	if (bio->bi_ioc) {
> -		put_io_context(bio->bi_ioc);
> -		bio->bi_ioc = NULL;
> -	}
>  	if (bio->bi_css) {
>  		css_put(bio->bi_css);
>  		bio->bi_css = NULL;
> diff --git a/block/blk-core.c b/block/blk-core.c
> index d6e8ab9ca99d..492648c96992 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -813,7 +813,7 @@ unsigned int blk_plug_queued_count(struct
> request_queue *q)
>  
>  void blk_init_request_from_bio(struct request *req, struct bio *bio)
>  {
> -	struct io_context *ioc = rq_ioc(bio);
> +	struct io_context *ioc = current->io_context;
>  
>  	if (bio->bi_opf & REQ_RAHEAD)
>  		req->cmd_flags |= REQ_FAILFAST_MASK;
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index d084f731d104..13b8dc332541 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -31,10 +31,10 @@ void blk_mq_sched_free_hctx_data(struct
> request_queue *q,
>  }
>  EXPORT_SYMBOL_GPL(blk_mq_sched_free_hctx_data);
>  
> -void blk_mq_sched_assign_ioc(struct request *rq, struct bio *bio)
> +void blk_mq_sched_assign_ioc(struct request *rq)
>  {
>  	struct request_queue *q = rq->q;
> -	struct io_context *ioc = rq_ioc(bio);
> +	struct io_context *ioc = current->io_context;
>  	struct io_cq *icq;
>  
>  	spin_lock_irq(&q->queue_lock);
> diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
> index 7ff5671bf128..0f719c8532ae 100644
> --- a/block/blk-mq-sched.h
> +++ b/block/blk-mq-sched.h
> @@ -8,7 +8,7 @@
>  void blk_mq_sched_free_hctx_data(struct request_queue *q,
>  				 void (*exit)(struct blk_mq_hw_ctx *));
>  
> -void blk_mq_sched_assign_ioc(struct request *rq, struct bio *bio);
> +void blk_mq_sched_assign_ioc(struct request *rq);
>  
>  void blk_mq_sched_request_inserted(struct request *rq);
>  bool blk_mq_sched_try_merge(struct request_queue *q, struct bio
> *bio,
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 32b246ed44c0..636f80b96fa6 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -389,8 +389,8 @@ static struct request *blk_mq_get_request(struct
> request_queue *q,
>  	if (!op_is_flush(data->cmd_flags)) {
>  		rq->elv.icq = NULL;
>  		if (e && e->type->ops.prepare_request) {
> -			if (e->type->icq_cache && rq_ioc(bio))
> -				blk_mq_sched_assign_ioc(rq, bio);
> +			if (e->type->icq_cache)
> +				blk_mq_sched_assign_ioc(rq);
>  
>  			e->type->ops.prepare_request(rq, bio);
>  			rq->rq_flags |= RQF_ELVPRIV;
> diff --git a/block/blk.h b/block/blk.h
> index 816a9abb87cd..610948157a5b 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -254,22 +254,6 @@ void ioc_clear_queue(struct request_queue *q);
>  
>  int create_task_io_context(struct task_struct *task, gfp_t gfp_mask,
> int node);
>  
> -/**
> - * rq_ioc - determine io_context for request allocation
> - * @bio: request being allocated is for this bio (can be %NULL)
> - *
> - * Determine io_context to use for request allocation for @bio.  May
> return
> - * %NULL if %current->io_context doesn't exist.
> - */
> -static inline struct io_context *rq_ioc(struct bio *bio)
> -{
> -#ifdef CONFIG_BLK_CGROUP
> -	if (bio && bio->bi_ioc)
> -		return bio->bi_ioc;
> -#endif
> -	return current->io_context;
> -}
> -
>  /**
>   * create_io_context - try to create task->io_context
>   * @gfp_mask: allocation mask
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index dbdbfbd6a987..c0ba1a038ff3 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -174,10 +174,9 @@ struct bio {
>  	void			*bi_private;
>  #ifdef CONFIG_BLK_CGROUP
>  	/*
> -	 * Optional ioc and css associated with this bio.  Put on bio
> +	 * Optional css associated with this bio.  Put on bio
>  	 * release.  Read comment on top of bio_associate_current().
>  	 */
> -	struct io_context	*bi_ioc;
>  	struct cgroup_subsys_state *bi_css;
>  	struct blkcg_gq		*bi_blkg;
>  	struct bio_issue	bi_issue;