All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
@ 2020-06-08  9:39 Giuliano Procida
  2020-06-08 13:01 ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-08  9:39 UTC (permalink / raw)
  To: greg; +Cc: stable, Jianchao Wang, Ming Lei, Jens Axboe, Giuliano Procida

From: Jianchao Wang <jianchao.w.wang@oracle.com>

commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.

For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
account the inflight requests. It will access the queue_hw_ctx and
nr_hw_queues w/o any protection. When updating nr_hw_queues and
blk_mq_in_flight/rw occur concurrently, panic comes up.

Before update nr_hw_queues, the q will be frozen. So we could use
q_usage_counter to avoid the race. percpu_ref_is_zero is used here
so that we will not miss any in-flight request. The access to
nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
under rcu critical section, __blk_mq_update_nr_hw_queues could use
synchronize_rcu to ensure the zeroed q_usage_counter to be globally
visible.

Backporting Notes

This is a re-backport, landing synchronize_rcu in the right place.

Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 block/blk-mq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9d53f476c517..cf56bdad2e06 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2738,6 +2738,10 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list)
 		blk_mq_freeze_queue(q);
+	/*
+	 * Sync with blk_mq_queue_tag_busy_iter.
+	 */
+	synchronize_rcu();
 
 	set->nr_hw_queues = nr_hw_queues;
 	blk_mq_update_queue_map(set);
@@ -2748,10 +2752,6 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list)
 		blk_mq_unfreeze_queue(q);
-	/*
-	 * Sync with blk_mq_queue_tag_busy_iter.
-	 */
-	synchronize_rcu();
 }
 
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
-- 
2.27.0.278.ge193c7cf3a9-goog


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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-08  9:39 [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter Giuliano Procida
@ 2020-06-08 13:01 ` Giuliano Procida
  2020-06-18  7:27   ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-08 13:01 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

-bouncing mail.

This one is for 4.14. Apologies.

Giuliano.

On Mon, 8 Jun 2020 at 10:39, Giuliano Procida <gprocida@google.com> wrote:
>
> From: Jianchao Wang <jianchao.w.wang@oracle.com>
>
> commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.
>
> For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
> account the inflight requests. It will access the queue_hw_ctx and
> nr_hw_queues w/o any protection. When updating nr_hw_queues and
> blk_mq_in_flight/rw occur concurrently, panic comes up.
>
> Before update nr_hw_queues, the q will be frozen. So we could use
> q_usage_counter to avoid the race. percpu_ref_is_zero is used here
> so that we will not miss any in-flight request. The access to
> nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
> under rcu critical section, __blk_mq_update_nr_hw_queues could use
> synchronize_rcu to ensure the zeroed q_usage_counter to be globally
> visible.
>
> Backporting Notes
>
> This is a re-backport, landing synchronize_rcu in the right place.
>
> Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  block/blk-mq.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 9d53f476c517..cf56bdad2e06 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2738,6 +2738,10 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>
>         list_for_each_entry(q, &set->tag_list, tag_set_list)
>                 blk_mq_freeze_queue(q);
> +       /*
> +        * Sync with blk_mq_queue_tag_busy_iter.
> +        */
> +       synchronize_rcu();
>
>         set->nr_hw_queues = nr_hw_queues;
>         blk_mq_update_queue_map(set);
> @@ -2748,10 +2752,6 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>
>         list_for_each_entry(q, &set->tag_list, tag_set_list)
>                 blk_mq_unfreeze_queue(q);
> -       /*
> -        * Sync with blk_mq_queue_tag_busy_iter.
> -        */
> -       synchronize_rcu();
>  }
>
>  void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
> --
> 2.27.0.278.ge193c7cf3a9-goog
>

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-08 13:01 ` Giuliano Procida
@ 2020-06-18  7:27   ` Giuliano Procida
  2020-06-18  7:32     ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-18  7:27 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

Hi Greg.

Is this patch (and the similar one for 4.9) queued?

Giuliano.

On Mon, 8 Jun 2020 at 14:01, Giuliano Procida <gprocida@google.com> wrote:
>
> -bouncing mail.
>
> This one is for 4.14. Apologies.
>
> Giuliano.
>
> On Mon, 8 Jun 2020 at 10:39, Giuliano Procida <gprocida@google.com> wrote:
> >
> > From: Jianchao Wang <jianchao.w.wang@oracle.com>
> >
> > commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.
> >
> > For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
> > account the inflight requests. It will access the queue_hw_ctx and
> > nr_hw_queues w/o any protection. When updating nr_hw_queues and
> > blk_mq_in_flight/rw occur concurrently, panic comes up.
> >
> > Before update nr_hw_queues, the q will be frozen. So we could use
> > q_usage_counter to avoid the race. percpu_ref_is_zero is used here
> > so that we will not miss any in-flight request. The access to
> > nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
> > under rcu critical section, __blk_mq_update_nr_hw_queues could use
> > synchronize_rcu to ensure the zeroed q_usage_counter to be globally
> > visible.
> >
> > Backporting Notes
> >
> > This is a re-backport, landing synchronize_rcu in the right place.
> >
> > Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
> > Reviewed-by: Ming Lei <ming.lei@redhat.com>
> > Signed-off-by: Jens Axboe <axboe@kernel.dk>
> > Signed-off-by: Giuliano Procida <gprocida@google.com>
> > ---
> >  block/blk-mq.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 9d53f476c517..cf56bdad2e06 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -2738,6 +2738,10 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
> >
> >         list_for_each_entry(q, &set->tag_list, tag_set_list)
> >                 blk_mq_freeze_queue(q);
> > +       /*
> > +        * Sync with blk_mq_queue_tag_busy_iter.
> > +        */
> > +       synchronize_rcu();
> >
> >         set->nr_hw_queues = nr_hw_queues;
> >         blk_mq_update_queue_map(set);
> > @@ -2748,10 +2752,6 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
> >
> >         list_for_each_entry(q, &set->tag_list, tag_set_list)
> >                 blk_mq_unfreeze_queue(q);
> > -       /*
> > -        * Sync with blk_mq_queue_tag_busy_iter.
> > -        */
> > -       synchronize_rcu();
> >  }
> >
> >  void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
> > --
> > 2.27.0.278.ge193c7cf3a9-goog
> >

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18  7:27   ` Giuliano Procida
@ 2020-06-18  7:32     ` Greg KH
  2020-06-18  9:16       ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2020-06-18  7:32 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: stable, Ming Lei, Jens Axboe


A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

:)

On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> Hi Greg.
> 
> Is this patch (and the similar one for 4.9) queued?

f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
blk_mq_queue_tag_busy_iter") is in the following stable releases:
	4.4.224 4.9.219 4.14.176 4.19

Do you not see it there?

Or is there some other upstream commit you are referring to here?

thanks,

greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18  7:32     ` Greg KH
@ 2020-06-18  9:16       ` Giuliano Procida
  2020-06-18 14:59         ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-18  9:16 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

Hi.

On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
>
>
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
>
> :)

I'm well aware of the above.
Alas, I haven't used mutt properly in about 15 years and I'm still
doing everything with Gmail.
Given that I was referring to the entire email thread, I punted on
finding a place to insert a comment.
BTW, there's a typo in the Q&A above. s/Were/Where/
:)

> On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > Hi Greg.
> >
> > Is this patch (and the similar one for 4.9) queued?
>
> f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> blk_mq_queue_tag_busy_iter") is in the following stable releases:
>         4.4.224 4.9.219 4.14.176 4.19
>
> Do you not see it there?

We are referring to different "it"s.

Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
which you back-ported to at least some of these kernels. This is
clearly there.

Mine: the commit sent earlier in this email thread - it's a
re-back-port, as I think the original back-port for 4.14 (and
similarly for 4.9) is incorrect. This has clearly not reached public
git, hence my question about whether the change was queued.

These are the ids of messages containing my commits:

4.14: 20200608093950.86293-1-gprocida@google.com
4.9: 20200608094030.87031-1-gprocida@google.com

> Or is there some other upstream commit you are referring to here?

No.

> thanks,
>
> greg k-h

I hope this clears things up.

Regards,
Giuliano.

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18  9:16       ` Giuliano Procida
@ 2020-06-18 14:59         ` Greg KH
  2020-06-18 15:35           ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2020-06-18 14:59 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: stable, Ming Lei, Jens Axboe

On Thu, Jun 18, 2020 at 10:16:45AM +0100, Giuliano Procida wrote:
> Hi.
> 
> On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
> >
> >
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> >
> > A: No.
> > Q: Should I include quotations after my reply?
> >
> > http://daringfireball.net/2007/07/on_top
> >
> > :)
> 
> I'm well aware of the above.
> Alas, I haven't used mutt properly in about 15 years and I'm still
> doing everything with Gmail.

gmail can handle proper quoting, if you are stuck with that :)

> Given that I was referring to the entire email thread, I punted on
> finding a place to insert a comment.
> BTW, there's a typo in the Q&A above. s/Were/Where/

Ah, nice catch, first one to notice that in years!

> > On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > > Hi Greg.
> > >
> > > Is this patch (and the similar one for 4.9) queued?
> >
> > f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> > blk_mq_queue_tag_busy_iter") is in the following stable releases:
> >         4.4.224 4.9.219 4.14.176 4.19
> >
> > Do you not see it there?
> 
> We are referring to different "it"s.
> 
> Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
> which you back-ported to at least some of these kernels. This is
> clearly there.

Great.

> Mine: the commit sent earlier in this email thread - it's a
> re-back-port, as I think the original back-port for 4.14 (and
> similarly for 4.9) is incorrect. This has clearly not reached public
> git, hence my question about whether the change was queued.

I don't know what the git commit id you are looking for here, sorry.  I
don't have the whole thread anywhere.

> These are the ids of messages containing my commits:
> 
> 4.14: 20200608093950.86293-1-gprocida@google.com
> 4.9: 20200608094030.87031-1-gprocida@google.com

Pointers to this on lore.kernel.org perhaps?

Remember, some of us get thousands of patches a week to handle,
remembering old email thread, or even keeping them around, is
impossible...

thanks,

greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18 14:59         ` Greg KH
@ 2020-06-18 15:35           ` Giuliano Procida
  2020-06-18 16:05             ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-18 15:35 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

Hi.

On Thu, 18 Jun 2020 at 15:59, Greg KH <greg@kroah.com> wrote:
>
> On Thu, Jun 18, 2020 at 10:16:45AM +0100, Giuliano Procida wrote:
> > Hi.
> >
> > On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
> > >
> > >
> > > A: http://en.wikipedia.org/wiki/Top_post
> > > Q: Were do I find info about this thing called top-posting?
> > > A: Because it messes up the order in which people normally read text.
> > > Q: Why is top-posting such a bad thing?
> > > A: Top-posting.
> > > Q: What is the most annoying thing in e-mail?
> > >
> > > A: No.
> > > Q: Should I include quotations after my reply?
> > >
> > > http://daringfireball.net/2007/07/on_top
> > >
> > > :)
> >
> > I'm well aware of the above.
> > Alas, I haven't used mutt properly in about 15 years and I'm still
> > doing everything with Gmail.
>
> gmail can handle proper quoting, if you are stuck with that :)
>
> > Given that I was referring to the entire email thread, I punted on
> > finding a place to insert a comment.
> > BTW, there's a typo in the Q&A above. s/Were/Where/
>
> Ah, nice catch, first one to notice that in years!
>
> > > On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > > > Hi Greg.
> > > >
> > > > Is this patch (and the similar one for 4.9) queued?
> > >
> > > f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> > > blk_mq_queue_tag_busy_iter") is in the following stable releases:
> > >         4.4.224 4.9.219 4.14.176 4.19
> > >
> > > Do you not see it there?
> >
> > We are referring to different "it"s.
> >
> > Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
> > which you back-ported to at least some of these kernels. This is
> > clearly there.
>
> Great.
>
> > Mine: the commit sent earlier in this email thread - it's a
> > re-back-port, as I think the original back-port for 4.14 (and
> > similarly for 4.9) is incorrect. This has clearly not reached public
> > git, hence my question about whether the change was queued.
>
> I don't know what the git commit id you are looking for here, sorry.  I
> don't have the whole thread anywhere.
>
> > These are the ids of messages containing my commits:
> >
> > 4.14: 20200608093950.86293-1-gprocida@google.com
> > 4.9: 20200608094030.87031-1-gprocida@google.com
>
> Pointers to this on lore.kernel.org perhaps?
>

I wasn't aware stable was publicly archived. Here you go:

4.14: https://lore.kernel.org/stable/20200608093950.86293-1-gprocida@google.com/
4.9: https://lore.kernel.org/stable/20200608094030.87031-1-gprocida@google.com/

Regards,
Giuliano.

> thanks

> Remember, some of us get thousands of patches a week to handle,
> remembering old email thread, or even keeping them around, is
> impossible...
>
> greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18 15:35           ` Giuliano Procida
@ 2020-06-18 16:05             ` Greg KH
  2020-06-18 16:14               ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2020-06-18 16:05 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: stable, Ming Lei, Jens Axboe

On Thu, Jun 18, 2020 at 04:35:03PM +0100, Giuliano Procida wrote:
> Hi.
> 
> On Thu, 18 Jun 2020 at 15:59, Greg KH <greg@kroah.com> wrote:
> >
> > On Thu, Jun 18, 2020 at 10:16:45AM +0100, Giuliano Procida wrote:
> > > Hi.
> > >
> > > On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
> > > >
> > > >
> > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > Q: Were do I find info about this thing called top-posting?
> > > > A: Because it messes up the order in which people normally read text.
> > > > Q: Why is top-posting such a bad thing?
> > > > A: Top-posting.
> > > > Q: What is the most annoying thing in e-mail?
> > > >
> > > > A: No.
> > > > Q: Should I include quotations after my reply?
> > > >
> > > > http://daringfireball.net/2007/07/on_top
> > > >
> > > > :)
> > >
> > > I'm well aware of the above.
> > > Alas, I haven't used mutt properly in about 15 years and I'm still
> > > doing everything with Gmail.
> >
> > gmail can handle proper quoting, if you are stuck with that :)
> >
> > > Given that I was referring to the entire email thread, I punted on
> > > finding a place to insert a comment.
> > > BTW, there's a typo in the Q&A above. s/Were/Where/
> >
> > Ah, nice catch, first one to notice that in years!
> >
> > > > On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > > > > Hi Greg.
> > > > >
> > > > > Is this patch (and the similar one for 4.9) queued?
> > > >
> > > > f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> > > > blk_mq_queue_tag_busy_iter") is in the following stable releases:
> > > >         4.4.224 4.9.219 4.14.176 4.19
> > > >
> > > > Do you not see it there?
> > >
> > > We are referring to different "it"s.
> > >
> > > Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
> > > which you back-ported to at least some of these kernels. This is
> > > clearly there.
> >
> > Great.
> >
> > > Mine: the commit sent earlier in this email thread - it's a
> > > re-back-port, as I think the original back-port for 4.14 (and
> > > similarly for 4.9) is incorrect. This has clearly not reached public
> > > git, hence my question about whether the change was queued.
> >
> > I don't know what the git commit id you are looking for here, sorry.  I
> > don't have the whole thread anywhere.
> >
> > > These are the ids of messages containing my commits:
> > >
> > > 4.14: 20200608093950.86293-1-gprocida@google.com
> > > 4.9: 20200608094030.87031-1-gprocida@google.com
> >
> > Pointers to this on lore.kernel.org perhaps?
> >
> 
> I wasn't aware stable was publicly archived. Here you go:
> 
> 4.14: https://lore.kernel.org/stable/20200608093950.86293-1-gprocida@google.com/
> 4.9: https://lore.kernel.org/stable/20200608094030.87031-1-gprocida@google.com/

As per the git commit id in those emails, you should be able to
determine the same thing that I can, namely that that patch is in the
following stable releases:
	4.4.224 4.9.219 4.14.176 4.19

thanks,

greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18 16:05             ` Greg KH
@ 2020-06-18 16:14               ` Giuliano Procida
  2020-06-18 16:20                 ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-18 16:14 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

Hi.

On Thu, 18 Jun 2020 at 17:05, Greg KH <greg@kroah.com> wrote:
>
> On Thu, Jun 18, 2020 at 04:35:03PM +0100, Giuliano Procida wrote:
> > Hi.
> >
> > On Thu, 18 Jun 2020 at 15:59, Greg KH <greg@kroah.com> wrote:
> > >
> > > On Thu, Jun 18, 2020 at 10:16:45AM +0100, Giuliano Procida wrote:
> > > > Hi.
> > > >
> > > > On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
> > > > >
> > > > >
> > > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > > Q: Were do I find info about this thing called top-posting?
> > > > > A: Because it messes up the order in which people normally read text.
> > > > > Q: Why is top-posting such a bad thing?
> > > > > A: Top-posting.
> > > > > Q: What is the most annoying thing in e-mail?
> > > > >
> > > > > A: No.
> > > > > Q: Should I include quotations after my reply?
> > > > >
> > > > > http://daringfireball.net/2007/07/on_top
> > > > >
> > > > > :)
> > > >
> > > > I'm well aware of the above.
> > > > Alas, I haven't used mutt properly in about 15 years and I'm still
> > > > doing everything with Gmail.
> > >
> > > gmail can handle proper quoting, if you are stuck with that :)
> > >
> > > > Given that I was referring to the entire email thread, I punted on
> > > > finding a place to insert a comment.
> > > > BTW, there's a typo in the Q&A above. s/Were/Where/
> > >
> > > Ah, nice catch, first one to notice that in years!
> > >
> > > > > On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > > > > > Hi Greg.
> > > > > >
> > > > > > Is this patch (and the similar one for 4.9) queued?
> > > > >
> > > > > f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> > > > > blk_mq_queue_tag_busy_iter") is in the following stable releases:
> > > > >         4.4.224 4.9.219 4.14.176 4.19
> > > > >
> > > > > Do you not see it there?
> > > >
> > > > We are referring to different "it"s.
> > > >
> > > > Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
> > > > which you back-ported to at least some of these kernels. This is
> > > > clearly there.
> > >
> > > Great.
> > >
> > > > Mine: the commit sent earlier in this email thread - it's a
> > > > re-back-port, as I think the original back-port for 4.14 (and
> > > > similarly for 4.9) is incorrect. This has clearly not reached public
> > > > git, hence my question about whether the change was queued.
> > >
> > > I don't know what the git commit id you are looking for here, sorry.  I
> > > don't have the whole thread anywhere.
> > >
> > > > These are the ids of messages containing my commits:
> > > >
> > > > 4.14: 20200608093950.86293-1-gprocida@google.com
> > > > 4.9: 20200608094030.87031-1-gprocida@google.com
> > >
> > > Pointers to this on lore.kernel.org perhaps?
> > >
> >
> > I wasn't aware stable was publicly archived. Here you go:
> >
> > 4.14: https://lore.kernel.org/stable/20200608093950.86293-1-gprocida@google.com/
> > 4.9: https://lore.kernel.org/stable/20200608094030.87031-1-gprocida@google.com/
>
> As per the git commit id in those emails, you should be able to
> determine the same thing that I can, namely that that patch is in the
> following stable releases:
>         4.4.224 4.9.219 4.14.176 4.19

Both those commits contain the text:

"Backporting Notes

This is a re-backport, landing synchronize_rcu in the right place."

If there's a special procedure for fixing back-ports, let me know,
'cos I haven't found one!

Cheers,
Giuliano.

> thanks,
>
> greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-18 16:14               ` Giuliano Procida
@ 2020-06-18 16:20                 ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2020-06-18 16:20 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: stable, Ming Lei, Jens Axboe

On Thu, Jun 18, 2020 at 05:14:23PM +0100, Giuliano Procida wrote:
> Hi.
> 
> On Thu, 18 Jun 2020 at 17:05, Greg KH <greg@kroah.com> wrote:
> >
> > On Thu, Jun 18, 2020 at 04:35:03PM +0100, Giuliano Procida wrote:
> > > Hi.
> > >
> > > On Thu, 18 Jun 2020 at 15:59, Greg KH <greg@kroah.com> wrote:
> > > >
> > > > On Thu, Jun 18, 2020 at 10:16:45AM +0100, Giuliano Procida wrote:
> > > > > Hi.
> > > > >
> > > > > On Thu, 18 Jun 2020 at 08:33, Greg KH <greg@kroah.com> wrote:
> > > > > >
> > > > > >
> > > > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > > > Q: Were do I find info about this thing called top-posting?
> > > > > > A: Because it messes up the order in which people normally read text.
> > > > > > Q: Why is top-posting such a bad thing?
> > > > > > A: Top-posting.
> > > > > > Q: What is the most annoying thing in e-mail?
> > > > > >
> > > > > > A: No.
> > > > > > Q: Should I include quotations after my reply?
> > > > > >
> > > > > > http://daringfireball.net/2007/07/on_top
> > > > > >
> > > > > > :)
> > > > >
> > > > > I'm well aware of the above.
> > > > > Alas, I haven't used mutt properly in about 15 years and I'm still
> > > > > doing everything with Gmail.
> > > >
> > > > gmail can handle proper quoting, if you are stuck with that :)
> > > >
> > > > > Given that I was referring to the entire email thread, I punted on
> > > > > finding a place to insert a comment.
> > > > > BTW, there's a typo in the Q&A above. s/Were/Where/
> > > >
> > > > Ah, nice catch, first one to notice that in years!
> > > >
> > > > > > On Thu, Jun 18, 2020 at 08:27:55AM +0100, Giuliano Procida wrote:
> > > > > > > Hi Greg.
> > > > > > >
> > > > > > > Is this patch (and the similar one for 4.9) queued?
> > > > > >
> > > > > > f5bbbbe4d635 ("blk-mq: sync the update nr_hw_queues with
> > > > > > blk_mq_queue_tag_busy_iter") is in the following stable releases:
> > > > > >         4.4.224 4.9.219 4.14.176 4.19
> > > > > >
> > > > > > Do you not see it there?
> > > > >
> > > > > We are referring to different "it"s.
> > > > >
> > > > > Yours: f5bbbbe4d635 is the upstream patch that went into v4.19-rc1 and
> > > > > which you back-ported to at least some of these kernels. This is
> > > > > clearly there.
> > > >
> > > > Great.
> > > >
> > > > > Mine: the commit sent earlier in this email thread - it's a
> > > > > re-back-port, as I think the original back-port for 4.14 (and
> > > > > similarly for 4.9) is incorrect. This has clearly not reached public
> > > > > git, hence my question about whether the change was queued.
> > > >
> > > > I don't know what the git commit id you are looking for here, sorry.  I
> > > > don't have the whole thread anywhere.
> > > >
> > > > > These are the ids of messages containing my commits:
> > > > >
> > > > > 4.14: 20200608093950.86293-1-gprocida@google.com
> > > > > 4.9: 20200608094030.87031-1-gprocida@google.com
> > > >
> > > > Pointers to this on lore.kernel.org perhaps?
> > > >
> > >
> > > I wasn't aware stable was publicly archived. Here you go:
> > >
> > > 4.14: https://lore.kernel.org/stable/20200608093950.86293-1-gprocida@google.com/
> > > 4.9: https://lore.kernel.org/stable/20200608094030.87031-1-gprocida@google.com/
> >
> > As per the git commit id in those emails, you should be able to
> > determine the same thing that I can, namely that that patch is in the
> > following stable releases:
> >         4.4.224 4.9.219 4.14.176 4.19
> 
> Both those commits contain the text:
> 
> "Backporting Notes
> 
> This is a re-backport, landing synchronize_rcu in the right place."
> 
> If there's a special procedure for fixing back-ports, let me know,
> 'cos I haven't found one!

Ah, I missed that, as it was not obvious.

I can't take a patch that is a redo of a previous patch as it will not
apply, right?

Send a patch saying, specifically, something like "This fixes a broken
backport that was commit XXX which was commit YYY upstream..." and so
on.

otherwise it gets lost in the noise as you are seeing here...

thansk,

greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-08 10:56 ` Greg KH
@ 2020-06-08 12:59   ` Giuliano Procida
  0 siblings, 0 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-06-08 12:59 UTC (permalink / raw)
  To: Greg KH; +Cc: stable, Ming Lei, Jens Axboe

-bouncing mail.

That wasn't clever. Apologies.

This one is for 4.9.

Giuliano.

On Mon, 8 Jun 2020 at 11:56, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 08, 2020 at 10:40:30AM +0100, Giuliano Procida wrote:
> > From: Jianchao Wang <jianchao.w.wang@oracle.com>
> >
> > commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.
> >
> > For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
> > account the inflight requests. It will access the queue_hw_ctx and
> > nr_hw_queues w/o any protection. When updating nr_hw_queues and
> > blk_mq_in_flight/rw occur concurrently, panic comes up.
> >
> > Before update nr_hw_queues, the q will be frozen. So we could use
> > q_usage_counter to avoid the race. percpu_ref_is_zero is used here
> > so that we will not miss any in-flight request. The access to
> > nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
> > under rcu critical section, __blk_mq_update_nr_hw_queues could use
> > synchronize_rcu to ensure the zeroed q_usage_counter to be globally
> > visible.
> >
> > Backporting Notes
> >
> > This is a re-backport, landing synchronize_rcu in the right place.
>
> You sent this twice?
>
> And what stable kernel(s) does it go to?
>
> thanks,
>
> greg k-h

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

* Re: [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
  2020-06-08  9:40 Giuliano Procida
@ 2020-06-08 10:56 ` Greg KH
  2020-06-08 12:59   ` Giuliano Procida
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2020-06-08 10:56 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: stable, Jianchao Wang, Ming Lei, Jens Axboe

On Mon, Jun 08, 2020 at 10:40:30AM +0100, Giuliano Procida wrote:
> From: Jianchao Wang <jianchao.w.wang@oracle.com>
> 
> commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.
> 
> For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
> account the inflight requests. It will access the queue_hw_ctx and
> nr_hw_queues w/o any protection. When updating nr_hw_queues and
> blk_mq_in_flight/rw occur concurrently, panic comes up.
> 
> Before update nr_hw_queues, the q will be frozen. So we could use
> q_usage_counter to avoid the race. percpu_ref_is_zero is used here
> so that we will not miss any in-flight request. The access to
> nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
> under rcu critical section, __blk_mq_update_nr_hw_queues could use
> synchronize_rcu to ensure the zeroed q_usage_counter to be globally
> visible.
> 
> Backporting Notes
> 
> This is a re-backport, landing synchronize_rcu in the right place.

You sent this twice?

And what stable kernel(s) does it go to?

thanks,

greg k-h

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

* [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter
@ 2020-06-08  9:40 Giuliano Procida
  2020-06-08 10:56 ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-06-08  9:40 UTC (permalink / raw)
  To: greg; +Cc: stable, Jianchao Wang, Ming Lei, Jens Axboe, Giuliano Procida

From: Jianchao Wang <jianchao.w.wang@oracle.com>

commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream.

For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to
account the inflight requests. It will access the queue_hw_ctx and
nr_hw_queues w/o any protection. When updating nr_hw_queues and
blk_mq_in_flight/rw occur concurrently, panic comes up.

Before update nr_hw_queues, the q will be frozen. So we could use
q_usage_counter to avoid the race. percpu_ref_is_zero is used here
so that we will not miss any in-flight request. The access to
nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are
under rcu critical section, __blk_mq_update_nr_hw_queues could use
synchronize_rcu to ensure the zeroed q_usage_counter to be globally
visible.

Backporting Notes

This is a re-backport, landing synchronize_rcu in the right place.

Signed-off-by: Jianchao Wang <jianchao.w.wang@oracle.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 block/blk-mq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 58be2eaa5aaa..e0ed7317e98c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2331,6 +2331,10 @@ void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list)
 		blk_mq_freeze_queue(q);
+	/*
+	 * Sync with blk_mq_queue_tag_busy_iter.
+	 */
+	synchronize_rcu();
 
 	set->nr_hw_queues = nr_hw_queues;
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
@@ -2346,10 +2350,6 @@ void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues)
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list)
 		blk_mq_unfreeze_queue(q);
-	/*
-	 * Sync with blk_mq_queue_tag_busy_iter.
-	 */
-	synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(blk_mq_update_nr_hw_queues);
 
-- 
2.27.0.278.ge193c7cf3a9-goog


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

end of thread, other threads:[~2020-06-18 16:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-08  9:39 [PATCH] blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter Giuliano Procida
2020-06-08 13:01 ` Giuliano Procida
2020-06-18  7:27   ` Giuliano Procida
2020-06-18  7:32     ` Greg KH
2020-06-18  9:16       ` Giuliano Procida
2020-06-18 14:59         ` Greg KH
2020-06-18 15:35           ` Giuliano Procida
2020-06-18 16:05             ` Greg KH
2020-06-18 16:14               ` Giuliano Procida
2020-06-18 16:20                 ` Greg KH
2020-06-08  9:40 Giuliano Procida
2020-06-08 10:56 ` Greg KH
2020-06-08 12:59   ` Giuliano Procida

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.