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
