* [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
@ 2015-10-28 8:52 Roman Gushchin
2015-10-29 0:34 ` Neil Brown
0 siblings, 1 reply; 6+ messages in thread
From: Roman Gushchin @ 2015-10-28 8:52 UTC (permalink / raw)
To: linux-kernel; +Cc: Roman Gushchin, Neil Brown, Shaohua Li, linux-raid, stable
After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
__find_stripe() is called under conf->hash_locks + hash.
But handle_stripe_clean_event() calls remove_hash() under
conf->device_lock.
Under some cirscumstances the hash chain can be circuited,
and we get an infinite loop with disabled interrupts and locked hash
lock in __find_stripe(). This leads to hard lockup on multiple CPUs
and following system crash.
I was able to reproduce this behavior on raid6 over 6 ssd disks.
The devices_handle_discard_safely option should be set to enable trim
support. The following script was used:
for i in `seq 1 32`; do
dd if=/dev/zero of=large$i bs=10M count=100 &
done
Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
Cc: Neil Brown <neilb@suse.de>
Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Cc: <stable@vger.kernel.org> # 3.10 - 3.19
---
drivers/md/raid5.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e421016..5fa7549 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3060,6 +3060,8 @@ static void handle_stripe_clean_event(struct r5conf *conf,
}
if (!discard_pending &&
test_bit(R5_Discard, &sh->dev[sh->pd_idx].flags)) {
+ int hash = sh->hash_lock_index;
+
clear_bit(R5_Discard, &sh->dev[sh->pd_idx].flags);
clear_bit(R5_UPTODATE, &sh->dev[sh->pd_idx].flags);
if (sh->qd_idx >= 0) {
@@ -3073,9 +3075,9 @@ static void handle_stripe_clean_event(struct r5conf *conf,
* no updated data, so remove it from hash list and the stripe
* will be reinitialized
*/
- spin_lock_irq(&conf->device_lock);
+ spin_lock_irq(conf->hash_locks + hash);
remove_hash(sh);
- spin_unlock_irq(&conf->device_lock);
+ spin_unlock_irq(conf->hash_locks + hash);
if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state))
set_bit(STRIPE_HANDLE, &sh->state);
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
2015-10-28 8:52 [PATCH] md/raid5: fix locking in handle_stripe_clean_event() Roman Gushchin
@ 2015-10-29 0:34 ` Neil Brown
2015-10-29 14:15 ` Roman Gushchin
0 siblings, 1 reply; 6+ messages in thread
From: Neil Brown @ 2015-10-29 0:34 UTC (permalink / raw)
To: Roman Gushchin, linux-kernel
Cc: Roman Gushchin, Shaohua Li, linux-raid, stable
[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]
On Wed, Oct 28 2015, Roman Gushchin wrote:
> After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
> __find_stripe() is called under conf->hash_locks + hash.
> But handle_stripe_clean_event() calls remove_hash() under
> conf->device_lock.
>
> Under some cirscumstances the hash chain can be circuited,
> and we get an infinite loop with disabled interrupts and locked hash
> lock in __find_stripe(). This leads to hard lockup on multiple CPUs
> and following system crash.
>
> I was able to reproduce this behavior on raid6 over 6 ssd disks.
> The devices_handle_discard_safely option should be set to enable trim
> support. The following script was used:
>
> for i in `seq 1 32`; do
> dd if=/dev/zero of=large$i bs=10M count=100 &
> done
>
> Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
> Cc: Neil Brown <neilb@suse.de>
> Cc: Shaohua Li <shli@kernel.org>
> Cc: linux-raid@vger.kernel.org
> Cc: <stable@vger.kernel.org> # 3.10 - 3.19
Hi Roman,
thanks for reporting this and providing a fix.
I'm a bit confused by that stable range: 3.10 - 3.19
The commit you identify as introducing the bug was added in 3.13, so
presumably 3.10, 3.11, 3.12 are not affected.
Also the bug is still present in mainline, so 4.0, 4.1, 4.2 are also
affected, though the patch needs to be revised a bit for 4.1 and later.
Does that match your understanding? Or is there something that I am
missing?
Thanks,
NeilBrown
> ---
> drivers/md/raid5.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index e421016..5fa7549 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3060,6 +3060,8 @@ static void handle_stripe_clean_event(struct r5conf *conf,
> }
> if (!discard_pending &&
> test_bit(R5_Discard, &sh->dev[sh->pd_idx].flags)) {
> + int hash = sh->hash_lock_index;
> +
> clear_bit(R5_Discard, &sh->dev[sh->pd_idx].flags);
> clear_bit(R5_UPTODATE, &sh->dev[sh->pd_idx].flags);
> if (sh->qd_idx >= 0) {
> @@ -3073,9 +3075,9 @@ static void handle_stripe_clean_event(struct r5conf *conf,
> * no updated data, so remove it from hash list and the stripe
> * will be reinitialized
> */
> - spin_lock_irq(&conf->device_lock);
> + spin_lock_irq(conf->hash_locks + hash);
> remove_hash(sh);
> - spin_unlock_irq(&conf->device_lock);
> + spin_unlock_irq(conf->hash_locks + hash);
> if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state))
> set_bit(STRIPE_HANDLE, &sh->state);
>
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
2015-10-29 0:34 ` Neil Brown
@ 2015-10-29 14:15 ` Roman Gushchin
2015-10-29 21:22 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Roman Gushchin @ 2015-10-29 14:15 UTC (permalink / raw)
To: Neil Brown, linux-kernel@vger.kernel.org
Cc: Shaohua Li, linux-raid@vger.kernel.org, stable@vger.kernel.org
29.10.2015, 03:35, "Neil Brown" <neilb@suse.de>:
> On Wed, Oct 28 2015, Roman Gushchin wrote:
>
>> О©╫After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
>> О©╫__find_stripe() is called under conf->hash_locks + hash.
>> О©╫But handle_stripe_clean_event() calls remove_hash() under
>> О©╫conf->device_lock.
>>
>> О©╫Under some cirscumstances the hash chain can be circuited,
>> О©╫and we get an infinite loop with disabled interrupts and locked hash
>> О©╫lock in __find_stripe(). This leads to hard lockup on multiple CPUs
>> О©╫and following system crash.
>>
>> О©╫I was able to reproduce this behavior on raid6 over 6 ssd disks.
>> О©╫The devices_handle_discard_safely option should be set to enable trim
>> О©╫support. The following script was used:
>>
>> О©╫for i in `seq 1 32`; do
>> О©╫О©╫О©╫О©╫О©╫dd if=/dev/zero of=large$i bs=10M count=100 &
>> О©╫done
>>
>> О©╫Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
>> О©╫Cc: Neil Brown <neilb@suse.de>
>> О©╫Cc: Shaohua Li <shli@kernel.org>
>> О©╫Cc: linux-raid@vger.kernel.org
>> О©╫Cc: <stable@vger.kernel.org> # 3.10 - 3.19
>
> Hi Roman,
> О©╫thanks for reporting this and providing a fix.
>
> I'm a bit confused by that stable range: 3.10 - 3.19
>
> The commit you identify as introducing the bug was added in 3.13, so
> presumably 3.10, 3.11, 3.12 are not affected.
Sure, it's my mistake. Correct range is 3.13 - 3.19. Sorry.
> Also the bug is still present in mainline, so 4.0, 4.1, 4.2 are also
> affected, though the patch needs to be revised a bit for 4.1 and later.
Yes, exactly, but things are a bit more complicated in mainline.
I'll try to prepare a patch for mainline in a couple of days.
Thanks,
Roman
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
2015-10-29 14:15 ` Roman Gushchin
@ 2015-10-29 21:22 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2015-10-29 21:22 UTC (permalink / raw)
To: Roman Gushchin
Cc: Neil Brown, linux-kernel@vger.kernel.org, Shaohua Li,
linux-raid@vger.kernel.org, stable@vger.kernel.org
On Thu, Oct 29, 2015 at 05:15:48PM +0300, Roman Gushchin wrote:
> 29.10.2015, 03:35, "Neil Brown" <neilb@suse.de>:
> > On Wed, Oct 28 2015, Roman Gushchin wrote:
> >
> >> �After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
> >> �__find_stripe() is called under conf->hash_locks + hash.
> >> �But handle_stripe_clean_event() calls remove_hash() under
> >> �conf->device_lock.
> >>
> >> �Under some cirscumstances the hash chain can be circuited,
> >> �and we get an infinite loop with disabled interrupts and locked hash
> >> �lock in __find_stripe(). This leads to hard lockup on multiple CPUs
> >> �and following system crash.
> >>
> >> �I was able to reproduce this behavior on raid6 over 6 ssd disks.
> >> �The devices_handle_discard_safely option should be set to enable trim
> >> �support. The following script was used:
> >>
> >> �for i in `seq 1 32`; do
> >> �����dd if=/dev/zero of=large$i bs=10M count=100 &
> >> �done
> >>
> >> �Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
> >> �Cc: Neil Brown <neilb@suse.de>
> >> �Cc: Shaohua Li <shli@kernel.org>
> >> �Cc: linux-raid@vger.kernel.org
> >> �Cc: <stable@vger.kernel.org> # 3.10 - 3.19
> >
> > Hi Roman,
> > �thanks for reporting this and providing a fix.
> >
> > I'm a bit confused by that stable range: 3.10 - 3.19
> >
> > The commit you identify as introducing the bug was added in 3.13, so
> > presumably 3.10, 3.11, 3.12 are not affected.
>
> Sure, it's my mistake. Correct range is 3.13 - 3.19. Sorry.
>
> > Also the bug is still present in mainline, so 4.0, 4.1, 4.2 are also
> > affected, though the patch needs to be revised a bit for 4.1 and later.
>
> Yes, exactly, but things are a bit more complicated in mainline.
> I'll try to prepare a patch for mainline in a couple of days.
We can't do anything with a patch that is not already in Linus's tree,
which is why this isn't even in my patch queue anymore. Please resend
this once the fix is in Linus's tree, with the git commit id of what it
is there and we will be glad to queue it up.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
2015-11-06 6:41 FAILED: patch "[PATCH] md/raid5: fix locking in handle_stripe_clean_event()" failed to apply to 3.14-stable tree gregkh
@ 2015-11-06 9:54 ` Roman Gushchin
2015-11-11 15:58 ` Luis Henriques
0 siblings, 1 reply; 6+ messages in thread
From: Roman Gushchin @ 2015-11-06 9:54 UTC (permalink / raw)
To: stable, gregkh; +Cc: Roman Gushchin, NeilBrown, Shaohua Li
commit b8a9d66d043ffac116100775a469f05f5158c16f upstream.
After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
__find_stripe() is called under conf->hash_locks + hash.
But handle_stripe_clean_event() calls remove_hash() under
conf->device_lock.
Under some cirscumstances the hash chain can be circuited,
and we get an infinite loop with disabled interrupts and locked hash
lock in __find_stripe(). This leads to hard lockup on multiple CPUs
and following system crash.
I was able to reproduce this behavior on raid6 over 6 ssd disks.
The devices_handle_discard_safely option should be set to enable trim
support. The following script was used:
for i in `seq 1 32`; do
dd if=/dev/zero of=large$i bs=10M count=100 &
done
Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
Fixes: 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
Signed-off-by: NeilBrown <neilb@suse.com>
Cc: Shaohua Li <shli@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org> # 3.13 - 4.2
---
drivers/md/raid5.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b98c70e..1c829a0 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3029,6 +3029,8 @@ static void handle_stripe_clean_event(struct r5conf *conf,
}
if (!discard_pending &&
test_bit(R5_Discard, &sh->dev[sh->pd_idx].flags)) {
+ int hash = sh->hash_lock_index;
+
clear_bit(R5_Discard, &sh->dev[sh->pd_idx].flags);
clear_bit(R5_UPTODATE, &sh->dev[sh->pd_idx].flags);
if (sh->qd_idx >= 0) {
@@ -3042,9 +3044,9 @@ static void handle_stripe_clean_event(struct r5conf *conf,
* no updated data, so remove it from hash list and the stripe
* will be reinitialized
*/
- spin_lock_irq(&conf->device_lock);
+ spin_lock_irq(conf->hash_locks + hash);
remove_hash(sh);
- spin_unlock_irq(&conf->device_lock);
+ spin_unlock_irq(conf->hash_locks + hash);
if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state))
set_bit(STRIPE_HANDLE, &sh->state);
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] md/raid5: fix locking in handle_stripe_clean_event()
2015-11-06 9:54 ` [PATCH] md/raid5: fix locking in handle_stripe_clean_event() Roman Gushchin
@ 2015-11-11 15:58 ` Luis Henriques
0 siblings, 0 replies; 6+ messages in thread
From: Luis Henriques @ 2015-11-11 15:58 UTC (permalink / raw)
To: Roman Gushchin; +Cc: stable, gregkh, NeilBrown, Shaohua Li
On Fri, Nov 06, 2015 at 12:54:47PM +0300, Roman Gushchin wrote:
> commit b8a9d66d043ffac116100775a469f05f5158c16f upstream.
>
Thanks, I'll use this backport for the 3.16 kernel as well.
Cheers,
--
Lu�s
> After commit 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
> __find_stripe() is called under conf->hash_locks + hash.
> But handle_stripe_clean_event() calls remove_hash() under
> conf->device_lock.
>
> Under some cirscumstances the hash chain can be circuited,
> and we get an infinite loop with disabled interrupts and locked hash
> lock in __find_stripe(). This leads to hard lockup on multiple CPUs
> and following system crash.
>
> I was able to reproduce this behavior on raid6 over 6 ssd disks.
> The devices_handle_discard_safely option should be set to enable trim
> support. The following script was used:
>
> for i in `seq 1 32`; do
> dd if=/dev/zero of=large$i bs=10M count=100 &
> done
>
> Signed-off-by: Roman Gushchin <klamm@yandex-team.ru>
> Fixes: 566c09c53455 ("raid5: relieve lock contention in get_active_stripe()")
> Signed-off-by: NeilBrown <neilb@suse.com>
> Cc: Shaohua Li <shli@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: <stable@vger.kernel.org> # 3.13 - 4.2
> ---
> drivers/md/raid5.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b98c70e..1c829a0 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3029,6 +3029,8 @@ static void handle_stripe_clean_event(struct r5conf *conf,
> }
> if (!discard_pending &&
> test_bit(R5_Discard, &sh->dev[sh->pd_idx].flags)) {
> + int hash = sh->hash_lock_index;
> +
> clear_bit(R5_Discard, &sh->dev[sh->pd_idx].flags);
> clear_bit(R5_UPTODATE, &sh->dev[sh->pd_idx].flags);
> if (sh->qd_idx >= 0) {
> @@ -3042,9 +3044,9 @@ static void handle_stripe_clean_event(struct r5conf *conf,
> * no updated data, so remove it from hash list and the stripe
> * will be reinitialized
> */
> - spin_lock_irq(&conf->device_lock);
> + spin_lock_irq(conf->hash_locks + hash);
> remove_hash(sh);
> - spin_unlock_irq(&conf->device_lock);
> + spin_unlock_irq(conf->hash_locks + hash);
> if (test_bit(STRIPE_SYNC_REQUESTED, &sh->state))
> set_bit(STRIPE_HANDLE, &sh->state);
>
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-11 15:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-28 8:52 [PATCH] md/raid5: fix locking in handle_stripe_clean_event() Roman Gushchin
2015-10-29 0:34 ` Neil Brown
2015-10-29 14:15 ` Roman Gushchin
2015-10-29 21:22 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2015-11-06 6:41 FAILED: patch "[PATCH] md/raid5: fix locking in handle_stripe_clean_event()" failed to apply to 3.14-stable tree gregkh
2015-11-06 9:54 ` [PATCH] md/raid5: fix locking in handle_stripe_clean_event() Roman Gushchin
2015-11-11 15:58 ` Luis Henriques
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).