Re: [PATCH 2/3] md/raid10: factor out get_rdev_repl_from_mirror()

[Li Nan <linan666@xxxxxxxxxxxxxxx>]

在 2023/7/1 7:53, Song Liu 写道:
> On Tue, Jun 27, 2023 at 6:58 PM <linan666@xxxxxxxxxxxxxxx> wrote:
> >
> [...]
>
> > +static void get_rdev_repl_from_mirror(struct raid10_info *mirror,
> > +                                struct md_rdev **prdev,
> > +                                struct md_rdev **prrdev)
> > +{
> > +       struct md_rdev *rdev, *rrdev;
> > +
> > +       rrdev = rcu_dereference(mirror->replacement);
> > +       /*
> > +        * Read replacement first to prevent reading both rdev and
> > +        * replacement as NULL during replacement replace rdev.
> > +        */
> > +       smp_mb();
> > +       rdev = rcu_dereference(mirror->rdev);
> > +       if (rdev == rrdev)
> > +               rrdev = NULL;
> > +
> > +       *prrdev = rrdev;
> > +       *prdev = rdev;
>
> I don't think the reduction in duplicated code justifies two output arguments.
>
> How about
>
> static struct md_rdev *dereference_rdev_and_rrdev(struct raid10_info *mirror,
>                                 struct md_rdev **prrdev)
> {
>      ...
>      *prrdev = xxx;
>      return rdev;
> }
>
> So we only have one argument for output.
>
> Also, "from_mirror" in the function name doesn't really add more value.
>
> Thanks,
> Song
> .

I agree. Let me improve this.

--
Thanks,
Nan