From: Mike Hartman <mike@hartmanipulation.com>
To: Neil Brown <neilb@suse.de>
Cc: linux-raid@vger.kernel.org
Subject: Re: New RAID causing system lockups
Date: Mon, 13 Sep 2010 22:50:10 -0400 [thread overview]
Message-ID: <AANLkTimeeecqZX1Bn23=_41sBYO+P8h_b5Z6Gf_ACuA1@mail.gmail.com> (raw)
In-Reply-To: <20100914113516.63c883c4@notabene>
>>
>> > Hi Mike,
>> > thanks for the updates.
>> >
>> > I'm not entirely clear what is happening (in fact, due to a cold that I am
>> > still fighting off, nothing is entirely clear at the moment), but it looks
>> > very likely that the problem is due to an interplay between barrier handling,
>> > and the multi-level structure of your array (a raid0 being a member of a
>> > raid5).
>> >
>> > When a barrier request is processed, both arrays will schedule 'work' to be
>> > done by the 'event' thread and I'm guess that you can get into a situation
>> > where one work time is wait for the other, but the other is behind the one on
>> > the single queue (I wonder if that make sense...)
>> >
>> > Anyway, this patch might make a difference, It reduced the number of work
>> > items schedule in a way that could conceivably fix the problem.
>> >
>> > If you can test this, please report the results. I cannot easily reproduce
>> > the problem so there is limited testing that I can do.
>> >
>> > Thanks,
>> > NeilBrown
>> >
>> >
>> > diff --git a/drivers/md/md.c b/drivers/md/md.c
>> > index f20d13e..7f2785c 100644
>> > --- a/drivers/md/md.c
>> > +++ b/drivers/md/md.c
>> > @@ -294,6 +294,23 @@ EXPORT_SYMBOL(mddev_congested);
>> >
>> > #define POST_REQUEST_BARRIER ((void*)1)
>> >
>> > +static void md_barrier_done(mddev_t *mddev)
>> > +{
>> > + struct bio *bio = mddev->barrier;
>> > +
>> > + if (test_bit(BIO_EOPNOTSUPP, &bio->bi_flags))
>> > + bio_endio(bio, -EOPNOTSUPP);
>> > + else if (bio->bi_size == 0)
>> > + bio_endio(bio, 0);
>> > + else {
>> > + /* other options need to be handled from process context */
>> > + schedule_work(&mddev->barrier_work);
>> > + return;
>> > + }
>> > + mddev->barrier = NULL;
>> > + wake_up(&mddev->sb_wait);
>> > +}
>> > +
>> > static void md_end_barrier(struct bio *bio, int err)
>> > {
>> > mdk_rdev_t *rdev = bio->bi_private;
>> > @@ -310,7 +327,7 @@ static void md_end_barrier(struct bio *bio, int err)
>> > wake_up(&mddev->sb_wait);
>> > } else
>> > /* The pre-request barrier has finished */
>> > - schedule_work(&mddev->barrier_work);
>> > + md_barrier_done(mddev);
>> > }
>> > bio_put(bio);
>> > }
>> > @@ -350,18 +367,12 @@ static void md_submit_barrier(struct work_struct *ws)
>> >
>> > atomic_set(&mddev->flush_pending, 1);
>> >
>> > - if (test_bit(BIO_EOPNOTSUPP, &bio->bi_flags))
>> > - bio_endio(bio, -EOPNOTSUPP);
>> > - else if (bio->bi_size == 0)
>> > - /* an empty barrier - all done */
>> > - bio_endio(bio, 0);
>> > - else {
>> > - bio->bi_rw &= ~REQ_HARDBARRIER;
>> > - if (mddev->pers->make_request(mddev, bio))
>> > - generic_make_request(bio);
>> > - mddev->barrier = POST_REQUEST_BARRIER;
>> > - submit_barriers(mddev);
>> > - }
>> > + bio->bi_rw &= ~REQ_HARDBARRIER;
>> > + if (mddev->pers->make_request(mddev, bio))
>> > + generic_make_request(bio);
>> > + mddev->barrier = POST_REQUEST_BARRIER;
>> > + submit_barriers(mddev);
>> > +
>> > if (atomic_dec_and_test(&mddev->flush_pending)) {
>> > mddev->barrier = NULL;
>> > wake_up(&mddev->sb_wait);
>> > @@ -383,7 +394,7 @@ void md_barrier_request(mddev_t *mddev, struct bio *bio)
>> > submit_barriers(mddev);
>> >
>> > if (atomic_dec_and_test(&mddev->flush_pending))
>> > - schedule_work(&mddev->barrier_work);
>> > + md_barrier_done(mddev);
>> > }
>> > EXPORT_SYMBOL(md_barrier_request);
>> >
>> >
>> >
>>
>> Neil, thanks for the patch. I experienced the lockup for the 5th time
>> an hour ago (about 3 hours after the last hard reboot) so I thought it
>> would be a good time to try your patch. Unfortunately I'm getting an
>> error:
>>
>> patching file drivers/md/md.c
>> Hunk #1 succeeded at 291 with fuzz 1 (offset -3 lines).
>> Hunk #2 FAILED at 324.
>> Hunk #3 FAILED at 364.
>> Hunk #4 FAILED at 391.
>> 3 out of 4 hunks FAILED -- saving rejects to file drivers/md/md.c.rej
>
> That is odd.
> I took the md.c that you posted on the web site, use "patch" to apply my
> patch to it, and only Hunk #3 failed.
>
> I used 'wiggle' to apply the patch and it applied perfectly, properly
> replacing (1<<BIO_RW_BARRIER) with REQ_HARDBARRIER (or the other way around).
>
> Try this version. You will need to be in drivers/md/, or use
>
> patch drivers/md/md.c < this-patch
>
>
> NeilBrown
>
> --- md.c.orig 2010-09-14 11:29:15.000000000 +1000
> +++ md.c 2010-09-14 11:29:50.000000000 +1000
> @@ -291,6 +291,23 @@
>
> #define POST_REQUEST_BARRIER ((void*)1)
>
> +static void md_barrier_done(mddev_t *mddev)
> +{
> + struct bio *bio = mddev->barrier;
> +
> + if (test_bit(BIO_EOPNOTSUPP, &bio->bi_flags))
> + bio_endio(bio, -EOPNOTSUPP);
> + else if (bio->bi_size == 0)
> + bio_endio(bio, 0);
> + else {
> + /* other options need to be handled from process context */
> + schedule_work(&mddev->barrier_work);
> + return;
> + }
> + mddev->barrier = NULL;
> + wake_up(&mddev->sb_wait);
> +}
> +
> static void md_end_barrier(struct bio *bio, int err)
> {
> mdk_rdev_t *rdev = bio->bi_private;
> @@ -307,7 +324,7 @@
> wake_up(&mddev->sb_wait);
> } else
> /* The pre-request barrier has finished */
> - schedule_work(&mddev->barrier_work);
> + md_barrier_done(mddev);
> }
> bio_put(bio);
> }
> @@ -347,18 +364,12 @@
>
> atomic_set(&mddev->flush_pending, 1);
>
> - if (test_bit(BIO_EOPNOTSUPP, &bio->bi_flags))
> - bio_endio(bio, -EOPNOTSUPP);
> - else if (bio->bi_size == 0)
> - /* an empty barrier - all done */
> - bio_endio(bio, 0);
> - else {
> - bio->bi_rw &= ~(1<<BIO_RW_BARRIER);
> - if (mddev->pers->make_request(mddev, bio))
> - generic_make_request(bio);
> - mddev->barrier = POST_REQUEST_BARRIER;
> - submit_barriers(mddev);
> - }
> + bio->bi_rw &= ~(1<<BIO_RW_BARRIER);
> + if (mddev->pers->make_request(mddev, bio))
> + generic_make_request(bio);
> + mddev->barrier = POST_REQUEST_BARRIER;
> + submit_barriers(mddev);
> +
> if (atomic_dec_and_test(&mddev->flush_pending)) {
> mddev->barrier = NULL;
> wake_up(&mddev->sb_wait);
> @@ -380,7 +391,7 @@
> submit_barriers(mddev);
>
> if (atomic_dec_and_test(&mddev->flush_pending))
> - schedule_work(&mddev->barrier_work);
> + md_barrier_done(mddev);
> }
> EXPORT_SYMBOL(md_barrier_request);
>
>
Sorry about that Neil, it was my fault. I copied your patch out of the
email and I think it picked up some unintended characters. I tried
copying it from the mailing list archive website instead and it
patched in fine. The kernel compiled with no trouble and I'm booted
into it now. No unexpected side-effects yet. I'll continue with my
copying and we'll see if it locks up again. Thanks for all your help!
Mike
--
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
next prev parent reply other threads:[~2010-09-14 2:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-11 18:20 New RAID causing system lockups Mike Hartman
2010-09-11 18:45 ` Mike Hartman
2010-09-11 20:43 ` Neil Brown
2010-09-11 20:56 ` Mike Hartman
2010-09-13 6:28 ` Mike Hartman
2010-09-13 15:57 ` Mike Hartman
2010-09-13 23:51 ` Neil Brown
[not found] ` <AANLkTin=jy=xJTtN5mQ6U=rYw3p+_4-nmkhO7zqR0KLP@mail.gmail.com>
2010-09-14 1:11 ` Mike Hartman
2010-09-14 1:35 ` Neil Brown
2010-09-14 2:50 ` Mike Hartman [this message]
2010-09-14 3:35 ` Mike Hartman
2010-09-14 3:48 ` Neil Brown
[not found] ` <AANLkTimXabL-TyjqJ81syrx-Oxn50qexbA8q9p22sxJt@mail.gmail.com>
2010-09-15 21:49 ` Mike Hartman
2010-09-21 2:26 ` Neil Brown
2010-09-21 11:28 ` Mike Hartman
-- strict thread matches above, loose matches on Subject: below --
2010-09-11 18:13 Mike Hartman
2010-09-11 18:12 Mike Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='AANLkTimeeecqZX1Bn23=_41sBYO+P8h_b5Z6Gf_ACuA1@mail.gmail.com' \
--to=mike@hartmanipulation.com \
--cc=linux-raid@vger.kernel.org \
--cc=neilb@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).