Re: [PATCH 4/5] acpi/processor: Fix the return value of acpi_processor_ids_walk()

[Dou Liyang]

At 05/22/2018 09:47 AM, Dou Liyang wrote:
> At 05/19/2018 11:06 PM, Thomas Gleixner wrote:
> > On Tue, 20 Mar 2018, Dou Liyang wrote:
> >
> > > ACPI driver should make sure all the processor IDs in their ACPI 
> > > Namespace
> > > are unique for CPU hotplug. the driver performs a depth-first walk of 
> > > the
> > > namespace tree and calls the acpi_processor_ids_walk().
> > >
> > > But, the acpi_processor_ids_walk() will return true if one processor is
> > > checked, that cause the walk break after walking pass the first 
> > > processor.
> > >
> > > Repace the value with AE_OK which is the standard acpi_status value.
> > >
> > > Fixes 8c8cb30f49b8 ("acpi/processor: Implement DEVICE operator for 
> > > processor enumeration")
> > >
> > > Signed-off-by: Dou Liyang <douly.fnst@xxxxxxxxxxxxxx>
> > > ---
> > > Â drivers/acpi/acpi_processor.c | 4 ++--
> > > Â 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/acpi/acpi_processor.c 
> > > b/drivers/acpi/acpi_processor.c
> > > index 449d86d39965..db5bdb59639c 100644
> > > --- a/drivers/acpi/acpi_processor.c
> > > +++ b/drivers/acpi/acpi_processor.c
> > > @@ -663,11 +663,11 @@ static acpi_status __init (acpi_handle handle,
> > > ÂÂÂÂÂ }
> > > ÂÂÂÂÂ processor_validated_ids_update(uid);
> > > -ÂÂÂ return true;
> > > +ÂÂÂ return AE_OK;
> > > Â err:
> > > ÂÂÂÂÂ acpi_handle_info(handle, "Invalid processor object\n");
> > > -ÂÂÂ return false;
> > > +ÂÂÂ return AE_OK;
> >
> > I'm not sure whether this is the right return value here. Rafael?

+Cc Rafael's common used email address.

I am sorry, I created the cc list using ./script/get_maintainers.pl ...
and didn't check it.

Thanks,
	dou

> Hi, Thomas, Rafael,
>
> Yes, I used AE_OK to make sure it can skip the invalid objects and
> continue to do the following other objects, I'm also not sure.
>
> For this bug, recently, I sent another patch to remove this check code
> away.
>
>  Â Âhttps://lkml.org/lkml/2018/5/17/320
>
> IMO, the duplicate IDs can be avoid by the other code
>
>  ÂÂ if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) ---- 1)
>
> As the mapping of cpu_id(pr->id) and processor_id is fixed, when
> hot-plugging a physical CPU, if its processor_id is duplicated with the
> present, the above condition 1) will be 0, and Linux will do not add
> this CPU.
>
> And, when every time the system starts, this code will be executed, it
> will waste more time with the increase in the number of CPU.
>
> So I prefer to remove this code.
>
> Thanks,
>  ÂÂÂÂdou