diff mbox series

[04/14] bfq: use bfq_bic_lookup in bfq_limit_depth

Message ID 20211126115817.2087431-5-hch@lst.de (mailing list archive)
State Not Applicable
Headers show
Series [01/14] RDMA/qib: rename copy_io to qib_copy_io | expand

Commit Message

Christoph Hellwig Nov. 26, 2021, 11:58 a.m. UTC
No need to create a new I/O context if there is none present yet in
->limit_depth.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Kara Nov. 29, 2021, 4:09 p.m. UTC | #1
On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> No need to create a new I/O context if there is none present yet in
> ->limit_depth.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bfq-iosched.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index c990c6409c119..ecc2e57e68630 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
>  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
>  {
>  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);

Maybe I'm missing something but bfq_limit_depth() needs to know to which
BFQ queue (and consequently blkcg) this IO is going to be added. And to be
able to lookup this queue we are using IO context. So AFAICT we need the
IO context allocated already in bfq_limit_depth()?

								Honza
Christoph Hellwig Nov. 30, 2021, 6:39 a.m. UTC | #2
On Mon, Nov 29, 2021 at 05:09:25PM +0100, Jan Kara wrote:
> On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> > No need to create a new I/O context if there is none present yet in
> > ->limit_depth.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  block/bfq-iosched.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> > index c990c6409c119..ecc2e57e68630 100644
> > --- a/block/bfq-iosched.c
> > +++ b/block/bfq-iosched.c
> > @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> >  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
> >  {
> >  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> > -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> > +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
> 
> Maybe I'm missing something but bfq_limit_depth() needs to know to which
> BFQ queue (and consequently blkcg) this IO is going to be added. And to be
> able to lookup this queue we are using IO context. So AFAICT we need the
> IO context allocated already in bfq_limit_depth()?

But by allocating it you won't now anything, as it will still be empty.
Jan Kara Nov. 30, 2021, 10:26 a.m. UTC | #3
On Tue 30-11-21 07:39:55, Christoph Hellwig wrote:
> On Mon, Nov 29, 2021 at 05:09:25PM +0100, Jan Kara wrote:
> > On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> > > No need to create a new I/O context if there is none present yet in
> > > ->limit_depth.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  block/bfq-iosched.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> > > index c990c6409c119..ecc2e57e68630 100644
> > > --- a/block/bfq-iosched.c
> > > +++ b/block/bfq-iosched.c
> > > @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> > >  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
> > >  {
> > >  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> > > -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> > > +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
> > 
> > Maybe I'm missing something but bfq_limit_depth() needs to know to which
> > BFQ queue (and consequently blkcg) this IO is going to be added. And to be
> > able to lookup this queue we are using IO context. So AFAICT we need the
> > IO context allocated already in bfq_limit_depth()?
> 
> But by allocating it you won't now anything, as it will still be empty.

You're right, we'll create only IO context and corresponding bfq_io_cq but
we won't actually create bfqq in bfq_limit_depth() anyway and without that
bfq_limit_depth() isn't going to do more. So your patch indeed does not
change anything in that regard.

								Honza
diff mbox series

Patch

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index c990c6409c119..ecc2e57e68630 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -663,7 +663,7 @@  static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
 static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
 {
 	struct bfq_data *bfqd = data->q->elevator->elevator_data;
-	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
+	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
 	struct bfq_queue *bfqq = bic ? bic_to_bfqq(bic, op_is_sync(op)) : NULL;
 	int depth;
 	unsigned limit = data->q->nr_requests;