Re: [PATCH V3 1/1] md/raid0: Add mddev->io_acct_cnt for raid0_quiesce

[Xiao Ni <xni@redhat.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)
> >
>