Re: [PATCH v2 2/7] acpi: utils: Add function to fetch dependent acpi_devices

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Feb 02, 2021 at 09:58:17AM +0000, Daniel Scally wrote:
> On 21/01/2021 21:06, Daniel Scally wrote:
> > On 21/01/2021 18:08, Rafael J. Wysocki wrote:

...

> > No problem;  I'll tweak that then
> 
> Slightly walking back my "No problem" here; as I understand this there's
> kinda two options:
> 
> 1. Walk over the (locked) list, when a match is found unlock, run the
> callback and re-lock.
> 
> The problem with that idea is unless I'm mistaken there's no guarantee
> that the .next pointer is still valid then (even using the *_safe()
> methods) because either the next or the next + 1 entry could have been
> removed whilst the list was unlocked and the callback was being ran, so
> this seems a little unsafe.

It's easy to solve.
See an example in deferred_probe_work_func().

https://elixir.bootlin.com/linux/latest/source/drivers/base/dd.c#L75

> 2. Walk over the (locked) list twice, the first time counting matching
> entries and using that to allocate a temporary buffer, then walk again
> to store the matching entries into the buffer. Finally, run the callback
> for everything in the buffer, free it and return.
> 
> Obviously that's a lot less efficient than the current function, which
> isn't particularly palatable.
> 
> Apologies if I've missed a better option that would work fine; but
> failing that do you still want me to go ahead and change
> acpi_walk_dep_device_list() to do this (I'd choose #2 of the above), or
> fallback to using acpi_dev_get_next_dependent_dev() described above? If
> the latter, does acpi_walk_dep_device_list() maybe need re-naming to
> make clear it's not a generalised function?

-- 
With Best Regards,
Andy Shevchenko