[PATCH V4 15/16] block, bfq: remove all get and put of I/O contexts

[Paolo Valente]

When a bfq queue is set in service and when it is merged, a reference
to the I/O context associated with the queue is taken. This reference
is then released when the queue is deselected from service or
split. More precisely, the release of the reference is postponed to
when the scheduler lock is released, to avoid nesting between the
scheduler and the I/O-context lock. In fact, such nesting would lead
to deadlocks, because of other code paths that take the same locks in
the opposite order. This postponing of I/O-context releases does
complicate code.

This commit addresses these issue by modifying involved operations in
such a way to not need to get the above I/O-context references any
more. Then it also removes any get and release of these references.

Signed-off-by: Paolo Valente 
---
 block/bfq-iosched.c | 143 +---
 1 file changed, 23 insertions(+), 120 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index b7e3c86..30bb8f9 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -538,8 +538,6 @@ struct bfq_data {
 
/* bfq_queue in service */
struct bfq_queue *in_service_queue;
-   /* bfq_io_cq (bic) associated with the @in_service_queue */
-   struct bfq_io_cq *in_service_bic;
 
/* on-disk position of the last served request */
sector_t last_position;
@@ -704,15 +702,6 @@ struct bfq_data {
struct bfq_io_cq *bio_bic;
/* bfqq associated with the task issuing current bio for merging */
struct bfq_queue *bio_bfqq;
-
-   /*
-* io context to put right after bfqd->lock is released. This
-* filed is used to perform put_io_context, when needed, to
-* after the scheduler lock has been released, and thus
-* prevent an ioc->lock from being possibly taken while the
-* scheduler lock is being held.
-*/
-   struct io_context *ioc_to_put;
 };
 
 enum bfqq_state_flags {
@@ -1148,34 +1137,6 @@ static void bfq_schedule_dispatch(struct bfq_data *bfqd)
}
 }
 
-/*
- * Next two functions release bfqd->lock and put the io context
- * pointed by bfqd->ioc_to_put. This delayed put is used to not risk
- * to take an ioc->lock while the scheduler lock is being held.
- */
-static void bfq_unlock_put_ioc(struct bfq_data *bfqd)
-{
-   struct io_context *ioc_to_put = bfqd->ioc_to_put;
-
-   bfqd->ioc_to_put = NULL;
-   spin_unlock_irq(>lock);
-
-   if (ioc_to_put)
-   put_io_context(ioc_to_put);
-}
-
-static void bfq_unlock_put_ioc_restore(struct bfq_data *bfqd,
-  unsigned long flags)
-{
-   struct io_context *ioc_to_put = bfqd->ioc_to_put;
-
-   bfqd->ioc_to_put = NULL;
-   spin_unlock_irqrestore(>lock, flags);
-
-   if (ioc_to_put)
-   put_io_context(ioc_to_put);
-}
-
 /**
  * bfq_gt - compare two timestamps.
  * @a: first ts.
@@ -2684,18 +2645,6 @@ static void __bfq_bfqd_reset_in_service(struct bfq_data 
*bfqd)
struct bfq_entity *in_serv_entity = _serv_bfqq->entity;
struct bfq_entity *entity = in_serv_entity;
 
-   if (bfqd->in_service_bic) {
-   /*
-* Schedule the release of a reference to
-* bfqd->in_service_bic->icq.ioc to right after the
-* scheduler lock is released. This ioc is not
-* released immediately, to not risk to possibly take
-* an ioc->lock while holding the scheduler lock.
-*/
-   bfqd->ioc_to_put = bfqd->in_service_bic->icq.ioc;
-   bfqd->in_service_bic = NULL;
-   }
-
bfq_clear_bfqq_wait_request(in_serv_bfqq);
hrtimer_try_to_cancel(>idle_slice_timer);
bfqd->in_service_queue = NULL;
@@ -3495,7 +3444,7 @@ static void bfq_pd_offline(struct blkg_policy_data *pd)
__bfq_deactivate_entity(entity, false);
bfq_put_async_queues(bfqd, bfqg);
 
-   bfq_unlock_put_ioc_restore(bfqd, flags);
+   spin_unlock_irqrestore(>lock, flags);
/*
 * @blkg is going offline and will be ignored by
 * blkg_[rw]stat_recursive_sum().  Transfer stats to the parent so
@@ -5472,20 +5421,18 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct 
bfq_queue *new_bfqq)
 * first time that the requests of some process are redirected to
 * it.
 *
-* We redirect bfqq to new_bfqq and not the opposite, because we
-* are in the context of the process owning bfqq, hence we have
-* the io_cq of this process. So we can immediately configure this
-* io_cq to redirect the requests of the process to new_bfqq.
+* We redirect bfqq to new_bfqq and not the opposite, because
+* we are in the context of the process owning bfqq, thus we
+* have the io_cq of this process. So we can immediately
+* configure this io_cq to redirect the requests of the
+  

[PATCH V4 15/16] block, bfq: remove all get and put of I/O contexts

[Paolo Valente]

When a bfq queue is set in service and when it is merged, a reference
to the I/O context associated with the queue is taken. This reference
is then released when the queue is deselected from service or
split. More precisely, the release of the reference is postponed to
when the scheduler lock is released, to avoid nesting between the
scheduler and the I/O-context lock. In fact, such nesting would lead
to deadlocks, because of other code paths that take the same locks in
the opposite order. This postponing of I/O-context releases does
complicate code.

This commit addresses these issue by modifying involved operations in
such a way to not need to get the above I/O-context references any
more. Then it also removes any get and release of these references.

Signed-off-by: Paolo Valente 
---
 block/bfq-iosched.c | 143 +---
 1 file changed, 23 insertions(+), 120 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index b7e3c86..30bb8f9 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -538,8 +538,6 @@ struct bfq_data {
 
/* bfq_queue in service */
struct bfq_queue *in_service_queue;
-   /* bfq_io_cq (bic) associated with the @in_service_queue */
-   struct bfq_io_cq *in_service_bic;
 
/* on-disk position of the last served request */
sector_t last_position;
@@ -704,15 +702,6 @@ struct bfq_data {
struct bfq_io_cq *bio_bic;
/* bfqq associated with the task issuing current bio for merging */
struct bfq_queue *bio_bfqq;
-
-   /*
-* io context to put right after bfqd->lock is released. This
-* filed is used to perform put_io_context, when needed, to
-* after the scheduler lock has been released, and thus
-* prevent an ioc->lock from being possibly taken while the
-* scheduler lock is being held.
-*/
-   struct io_context *ioc_to_put;
 };
 
 enum bfqq_state_flags {
@@ -1148,34 +1137,6 @@ static void bfq_schedule_dispatch(struct bfq_data *bfqd)
}
 }
 
-/*
- * Next two functions release bfqd->lock and put the io context
- * pointed by bfqd->ioc_to_put. This delayed put is used to not risk
- * to take an ioc->lock while the scheduler lock is being held.
- */
-static void bfq_unlock_put_ioc(struct bfq_data *bfqd)
-{
-   struct io_context *ioc_to_put = bfqd->ioc_to_put;
-
-   bfqd->ioc_to_put = NULL;
-   spin_unlock_irq(>lock);
-
-   if (ioc_to_put)
-   put_io_context(ioc_to_put);
-}
-
-static void bfq_unlock_put_ioc_restore(struct bfq_data *bfqd,
-  unsigned long flags)
-{
-   struct io_context *ioc_to_put = bfqd->ioc_to_put;
-
-   bfqd->ioc_to_put = NULL;
-   spin_unlock_irqrestore(>lock, flags);
-
-   if (ioc_to_put)
-   put_io_context(ioc_to_put);
-}
-
 /**
  * bfq_gt - compare two timestamps.
  * @a: first ts.
@@ -2684,18 +2645,6 @@ static void __bfq_bfqd_reset_in_service(struct bfq_data 
*bfqd)
struct bfq_entity *in_serv_entity = _serv_bfqq->entity;
struct bfq_entity *entity = in_serv_entity;
 
-   if (bfqd->in_service_bic) {
-   /*
-* Schedule the release of a reference to
-* bfqd->in_service_bic->icq.ioc to right after the
-* scheduler lock is released. This ioc is not
-* released immediately, to not risk to possibly take
-* an ioc->lock while holding the scheduler lock.
-*/
-   bfqd->ioc_to_put = bfqd->in_service_bic->icq.ioc;
-   bfqd->in_service_bic = NULL;
-   }
-
bfq_clear_bfqq_wait_request(in_serv_bfqq);
hrtimer_try_to_cancel(>idle_slice_timer);
bfqd->in_service_queue = NULL;
@@ -3495,7 +3444,7 @@ static void bfq_pd_offline(struct blkg_policy_data *pd)
__bfq_deactivate_entity(entity, false);
bfq_put_async_queues(bfqd, bfqg);
 
-   bfq_unlock_put_ioc_restore(bfqd, flags);
+   spin_unlock_irqrestore(>lock, flags);
/*
 * @blkg is going offline and will be ignored by
 * blkg_[rw]stat_recursive_sum().  Transfer stats to the parent so
@@ -5472,20 +5421,18 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct 
bfq_queue *new_bfqq)
 * first time that the requests of some process are redirected to
 * it.
 *
-* We redirect bfqq to new_bfqq and not the opposite, because we
-* are in the context of the process owning bfqq, hence we have
-* the io_cq of this process. So we can immediately configure this
-* io_cq to redirect the requests of the process to new_bfqq.
+* We redirect bfqq to new_bfqq and not the opposite, because
+* we are in the context of the process owning bfqq, thus we
+* have the io_cq of this process. So we can immediately
+* configure this io_cq to redirect the requests of the
+* process to