On 06/12/18 at 11:29am, Andy Shevchenko wrote:
> On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He <b...@redhat.com> wrote:
> > reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> > and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> > so that it's shared. Later its code also need be updated using list_head
> > to replace singly linked list.
>
> While this is a good deduplication of the code, some requirements for
> public functions would be good to satisfy.
>
> > +/*
> > + * Reparent resource children of pr that conflict with res
> > + * under res, and make res replace those children.
> > + */
>
> kernel doc format, though...
>
> > +static int reparent_resources(struct resource *parent,
> > + struct resource *res)
>
> ...is it really public with static keyword?!
Thanks for looking into this. This is a code bug, I copied and changed,
but forgot merging the changing to local commit. And the error reported
by test robot in patch 2 was changed too locally, forgot merging it to
patch. Will repost to address this.
>
>
>
> > +{
>
> > + for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> > + if (p->end < res->start)
> > + continue;
> > + if (res->end < p->start)
> > + break;
>
> > + if (p->start < res->start || p->end > res->end)
> > + return -1; /* not completely contained */
>
> Usually we are expecting real eeror codes.
Hmm, I just copied it from arch/powerpc/kernel/pci-common.c. The
function interface expects an integer returned value, not sure what a
real error codes look like, could you give more hints? Will change
accordingly.
>
> > + if (firstpp == NULL)
> > + firstpp = pp;
> > + }
>
> > + if (firstpp == NULL)
> > + return -1; /* didn't find any conflicting entries? */
>
> Ditto.
>
> > +}
> > +EXPORT_SYMBOL(reparent_resources);
>
> --
> With Best Regards,
> Andy Shevchenko
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
