On Thu, Feb 2, 2023 at 8:23 AM Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> On Thu, Feb 2, 2023 at 2:00 AM Song Liu <song@xxxxxxxxxx> wrote:
> >
> > On Wed, Feb 1, 2023 at 4:46 AM Xiao Ni <xni@xxxxxxxxxx> 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?
Hi Song
I've sent V4. If you think ->active_io is a better way to count acct_io,
I'll re-write the patch to use ->active_io
Regards
Xiao
>
> >
> > [...]
> >
> > > + } 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)
> > >
> >
