From: Xiao Ni <xni@redhat.com>
To: Song Liu <song@kernel.org>
Cc: linux-raid@vger.kernel.org, ming.lei@redhat.com,
ncroxon@redhat.com, heinzm@redhat.com
Subject: Re: [PATCH V3 1/1] md/raid0: Add mddev->io_acct_cnt for raid0_quiesce
Date: Thu, 2 Feb 2023 08:23:34 +0800 [thread overview]
Message-ID: <CALTww2_cLaULw4+QwwkjhhmBwjcP9GBTxNOR=WsZXAnPJaUakg@mail.gmail.com> (raw)
In-Reply-To: <CAPhsuW7MCSVREMp48CoO-qE-HfMonxhJn-+HfRUxvHfBXL0Nug@mail.gmail.com>
On Thu, Feb 2, 2023 at 2:00 AM Song Liu <song@kernel.org> wrote:
>
> On Wed, Feb 1, 2023 at 4:46 AM Xiao Ni <xni@redhat.com> wrote:
> >
> > It has added io_acct_set for raid0/raid5 io accounting and it needs to
> > alloc md_io_acct in the i/o path. They are free when the bios come back
> > from member disks. Now we don't have a method to monitor if those bios
> > are all come back. In the takeover process, it needs to free the raid0
> > memory resource including the memory pool for md_io_acct. But maybe some
> > bios are still not returned. When those bios are returned, it can cause
> > panic bcause of introducing NULL pointer or invalid address. Something
> > like this:
>
> Can we use mddev->active_io for this? If not, please explain the reason
> in the comments (in the code).
Hi Song
At first, we thought this way. Now ->acitve_io is used to wait all
submit processes to exit.
If we use ->active_io to count acct_bio, it means we change the usage
of ->active_io.
In mddev_suspend, first it waits for all submit processes to finish,
then it calls ->quiesce
to wait all inflight io to come back. For raid0, it's ok to use
->acitve_io to count acct_bio.
But for raid5, not sure if it's ok. What's your opinion?
>
> [...]
>
> > + } else
>
> Please add { } for the else clause.
ok
Regards
Xiao
>
> Thanks,
> Song
>
> > + if (percpu_ref_is_dying(&mddev->io_acct_cnt))
> > + percpu_ref_resurrect(&mddev->io_acct_cnt);
> > }
> >
> > static struct md_personality raid0_personality=
> > --
> > 2.32.0 (Apple Git-132)
> >
>
next prev parent reply other threads:[~2023-02-02 0:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-01 12:46 [PATCH V3 1/1] md/raid0: Add mddev->io_acct_cnt for raid0_quiesce Xiao Ni
2023-02-01 18:00 ` Song Liu
2023-02-02 0:23 ` Xiao Ni [this message]
2023-02-02 0:40 ` Xiao Ni
2023-02-02 6:57 ` Song Liu
2023-02-02 13:52 ` Xiao Ni
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='CALTww2_cLaULw4+QwwkjhhmBwjcP9GBTxNOR=WsZXAnPJaUakg@mail.gmail.com' \
--to=xni@redhat.com \
--cc=heinzm@redhat.com \
--cc=linux-raid@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=ncroxon@redhat.com \
--cc=song@kernel.org \
/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 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.