stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] bfq: Split shared queues on move between cgroups" failed to apply to 4.19-stable tree
@ 2022-06-06 11:57 gregkh
  2022-06-06 13:02 ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2022-06-06 11:57 UTC (permalink / raw)
  To: jack, axboe, hch, yukuai3; +Cc: stable


The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 3bc5e683c67d94bd839a1da2e796c15847b51b69 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Fri, 1 Apr 2022 12:27:44 +0200
Subject: [PATCH] bfq: Split shared queues on move between cgroups

When bfqq is shared by multiple processes it can happen that one of the
processes gets moved to a different cgroup (or just starts submitting IO
for different cgroup). In case that happens we need to split the merged
bfqq as otherwise we will have IO for multiple cgroups in one bfqq and
we will just account IO time to wrong entities etc.

Similarly if the bfqq is scheduled to merge with another bfqq but the
merge didn't happen yet, cancel the merge as it need not be valid
anymore.

CC: stable@vger.kernel.org
Fixes: e21b7a0b9887 ("block, bfq: add full hierarchical scheduling and cgroups support")
Tested-by: "yukuai (C)" <yukuai3@huawei.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20220401102752.8599-3-jack@suse.cz
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 420eda2589c0..9352f3cc2377 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -743,9 +743,39 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
 	}
 
 	if (sync_bfqq) {
-		entity = &sync_bfqq->entity;
-		if (entity->sched_data != &bfqg->sched_data)
-			bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
+		if (!sync_bfqq->new_bfqq && !bfq_bfqq_coop(sync_bfqq)) {
+			/* We are the only user of this bfqq, just move it */
+			if (sync_bfqq->entity.sched_data != &bfqg->sched_data)
+				bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
+		} else {
+			struct bfq_queue *bfqq;
+
+			/*
+			 * The queue was merged to a different queue. Check
+			 * that the merge chain still belongs to the same
+			 * cgroup.
+			 */
+			for (bfqq = sync_bfqq; bfqq; bfqq = bfqq->new_bfqq)
+				if (bfqq->entity.sched_data !=
+				    &bfqg->sched_data)
+					break;
+			if (bfqq) {
+				/*
+				 * Some queue changed cgroup so the merge is
+				 * not valid anymore. We cannot easily just
+				 * cancel the merge (by clearing new_bfqq) as
+				 * there may be other processes using this
+				 * queue and holding refs to all queues below
+				 * sync_bfqq->new_bfqq. Similarly if the merge
+				 * already happened, we need to detach from
+				 * bfqq now so that we cannot merge bio to a
+				 * request from the old cgroup.
+				 */
+				bfq_put_cooperator(sync_bfqq);
+				bfq_release_process_ref(bfqd, sync_bfqq);
+				bic_set_bfqq(bic, NULL, 1);
+			}
+		}
 	}
 
 	return bfqg;
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 7d00b21ebe5d..89fe3f85eb3c 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5315,7 +5315,7 @@ static void bfq_put_stable_ref(struct bfq_queue *bfqq)
 	bfq_put_queue(bfqq);
 }
 
-static void bfq_put_cooperator(struct bfq_queue *bfqq)
+void bfq_put_cooperator(struct bfq_queue *bfqq)
 {
 	struct bfq_queue *__bfqq, *next;
 
diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index 3b83e3d1c2e5..a56763045d19 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -979,6 +979,7 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
 void bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 		     bool compensate, enum bfqq_expiration reason);
 void bfq_put_queue(struct bfq_queue *bfqq);
+void bfq_put_cooperator(struct bfq_queue *bfqq);
 void bfq_end_wr_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg);
 void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq);
 void bfq_schedule_dispatch(struct bfq_data *bfqd);


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] bfq: Split shared queues on move between cgroups" failed to apply to 4.19-stable tree
  2022-06-06 11:57 FAILED: patch "[PATCH] bfq: Split shared queues on move between cgroups" failed to apply to 4.19-stable tree gregkh
@ 2022-06-06 13:02 ` Jan Kara
  2022-06-06 14:48   ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2022-06-06 13:02 UTC (permalink / raw)
  To: gregkh; +Cc: jack, axboe, hch, yukuai3, stable

Hello Greg!

I've seen this patch failed to apply to 4.19 and 4.14 stable trees but
you've still merged (some) of the patches following this one from the same
series. Honestly, I would not consider the result very trustworthy. The
code involved is rather complex with subtle interactions across subsystems.
So please remove also upstream commits:

ea591cd4eb27 ("bfq: Update cgroup information before merging bio")
fc84e1f941b9 ("bfq: Drop pointless unlock-lock pair")
5f550ede5edf ("bfq: Remove pointless bfq_init_rq() calls")
09f871868080 ("bfq: Track whether bfq_group is still online")
4e54a2493e58 ("bfq: Get rid of __bio_blkcg() usage")
075a53b78b81 ("bfq: Make sure bfqg for which we are queueing requests is online")

from 4.14 and 4.19 stable queues. If someone will need these fixes there,
he'll need to do proper backport with targetted testing etc... Thanks!

								Honza

On Mon 06-06-22 13:57:29, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 4.19-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 3bc5e683c67d94bd839a1da2e796c15847b51b69 Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Fri, 1 Apr 2022 12:27:44 +0200
> Subject: [PATCH] bfq: Split shared queues on move between cgroups
> 
> When bfqq is shared by multiple processes it can happen that one of the
> processes gets moved to a different cgroup (or just starts submitting IO
> for different cgroup). In case that happens we need to split the merged
> bfqq as otherwise we will have IO for multiple cgroups in one bfqq and
> we will just account IO time to wrong entities etc.
> 
> Similarly if the bfqq is scheduled to merge with another bfqq but the
> merge didn't happen yet, cancel the merge as it need not be valid
> anymore.
> 
> CC: stable@vger.kernel.org
> Fixes: e21b7a0b9887 ("block, bfq: add full hierarchical scheduling and cgroups support")
> Tested-by: "yukuai (C)" <yukuai3@huawei.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Link: https://lore.kernel.org/r/20220401102752.8599-3-jack@suse.cz
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index 420eda2589c0..9352f3cc2377 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -743,9 +743,39 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
>  	}
>  
>  	if (sync_bfqq) {
> -		entity = &sync_bfqq->entity;
> -		if (entity->sched_data != &bfqg->sched_data)
> -			bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
> +		if (!sync_bfqq->new_bfqq && !bfq_bfqq_coop(sync_bfqq)) {
> +			/* We are the only user of this bfqq, just move it */
> +			if (sync_bfqq->entity.sched_data != &bfqg->sched_data)
> +				bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
> +		} else {
> +			struct bfq_queue *bfqq;
> +
> +			/*
> +			 * The queue was merged to a different queue. Check
> +			 * that the merge chain still belongs to the same
> +			 * cgroup.
> +			 */
> +			for (bfqq = sync_bfqq; bfqq; bfqq = bfqq->new_bfqq)
> +				if (bfqq->entity.sched_data !=
> +				    &bfqg->sched_data)
> +					break;
> +			if (bfqq) {
> +				/*
> +				 * Some queue changed cgroup so the merge is
> +				 * not valid anymore. We cannot easily just
> +				 * cancel the merge (by clearing new_bfqq) as
> +				 * there may be other processes using this
> +				 * queue and holding refs to all queues below
> +				 * sync_bfqq->new_bfqq. Similarly if the merge
> +				 * already happened, we need to detach from
> +				 * bfqq now so that we cannot merge bio to a
> +				 * request from the old cgroup.
> +				 */
> +				bfq_put_cooperator(sync_bfqq);
> +				bfq_release_process_ref(bfqd, sync_bfqq);
> +				bic_set_bfqq(bic, NULL, 1);
> +			}
> +		}
>  	}
>  
>  	return bfqg;
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 7d00b21ebe5d..89fe3f85eb3c 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -5315,7 +5315,7 @@ static void bfq_put_stable_ref(struct bfq_queue *bfqq)
>  	bfq_put_queue(bfqq);
>  }
>  
> -static void bfq_put_cooperator(struct bfq_queue *bfqq)
> +void bfq_put_cooperator(struct bfq_queue *bfqq)
>  {
>  	struct bfq_queue *__bfqq, *next;
>  
> diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
> index 3b83e3d1c2e5..a56763045d19 100644
> --- a/block/bfq-iosched.h
> +++ b/block/bfq-iosched.h
> @@ -979,6 +979,7 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
>  void bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>  		     bool compensate, enum bfqq_expiration reason);
>  void bfq_put_queue(struct bfq_queue *bfqq);
> +void bfq_put_cooperator(struct bfq_queue *bfqq);
>  void bfq_end_wr_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg);
>  void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq);
>  void bfq_schedule_dispatch(struct bfq_data *bfqd);
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: FAILED: patch "[PATCH] bfq: Split shared queues on move between cgroups" failed to apply to 4.19-stable tree
  2022-06-06 13:02 ` Jan Kara
@ 2022-06-06 14:48   ` Greg KH
  0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2022-06-06 14:48 UTC (permalink / raw)
  To: Jan Kara; +Cc: axboe, hch, yukuai3, stable

On Mon, Jun 06, 2022 at 03:02:53PM +0200, Jan Kara wrote:
> Hello Greg!
> 
> I've seen this patch failed to apply to 4.19 and 4.14 stable trees but
> you've still merged (some) of the patches following this one from the same
> series. Honestly, I would not consider the result very trustworthy. The
> code involved is rather complex with subtle interactions across subsystems.
> So please remove also upstream commits:
> 
> ea591cd4eb27 ("bfq: Update cgroup information before merging bio")
> fc84e1f941b9 ("bfq: Drop pointless unlock-lock pair")
> 5f550ede5edf ("bfq: Remove pointless bfq_init_rq() calls")
> 09f871868080 ("bfq: Track whether bfq_group is still online")
> 4e54a2493e58 ("bfq: Get rid of __bio_blkcg() usage")
> 075a53b78b81 ("bfq: Make sure bfqg for which we are queueing requests is online")
> 
> from 4.14 and 4.19 stable queues. If someone will need these fixes there,
> he'll need to do proper backport with targetted testing etc... Thanks!

Only 2 ended up applying there, so I've dropped those two now, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-06-06 14:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 11:57 FAILED: patch "[PATCH] bfq: Split shared queues on move between cgroups" failed to apply to 4.19-stable tree gregkh
2022-06-06 13:02 ` Jan Kara
2022-06-06 14:48   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).