Re: [PATCH v4 0/9] driver core: Fix some race conditions

Rafael J. Wysocki
2026-04-04T17:12:13Z

On Sat, Apr 4, 2026 at 2:07 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> The main goal of this series is to fix the observed bug talked about
> in the first patch ("driver core: Don't let a device probe until it's
> ready"). That patch fixes a problem that has been observed in the real
> world and could land even if the rest of the patches are found
> unacceptable or need to be spun.
>
> That said, during patch review Danilo correctly pointed out that many
> of the bitfield accesses in "struct device" are unsafe. I added a
> bunch of patches in the series to address each one.
>
> Danilo said he's most worried about "can_match", so I put that one
> first. After that, I tried to transition bitfields to flags in reverse
> order to when the bitfield was added.
>
> Even if transitioning from bitfields to flags isn't truly needed for
> correctness, it seems silly (and wasteful of space in struct device)
> to have some in bitfields and some as flags. Thus I didn't spend time
> for each bitfield showing that it's truly needed for correctness.
>
> Transition was done semi manually. Presumably someone skilled at
> coccinelle could do a better job, but I just used sed in a heavy-
> handed manner and then reviewed/fixed the results, undoing anything my
> script got wrong. My terrible/ugly script was:
>
> var=can_match
> caps="${var^^}"
> for f in $(git grep -l "[>\.]${var}[^1-9_a-zA-Z\[]"); do
> echo $f
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = true/set_bit(DEV_FLAG_${caps}, \&\\1->flags)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = true/dev_set_${caps}(\&\\1)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = false/clear_bit(DEV_FLAG_${caps}, \&\\1->flags)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = false/dev_clear_${caps}(\&\\1)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var} = \([^;]*\)/assign_bit(DEV_FLAG_${caps}, \&\\1->flags, \\2)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var} = \([^;]*\)/dev_assign_${caps}(\&\\1, \\2)/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)->${var}\([^1-9_a-zA-Z\[]\)/test_bit(DEV_FLAG_${caps}, \&\\1->flags)\\2/" "$f"
> sed -i~ -e "s/\([a-zA-Z_0-9\.>()-][a-zA-Z_0-9\.>()-]*\)\.${var}\([^1-9_a-zA-Z\[]\)/dev_${caps}(\&\\1)\\2/" "$f"
> done
>
> From v3 to v4, I transitioned to accessor functions with another ugly
> sed script. I had git format the old patches, then transformed them
> with:
>
> for f in *.patch; do
> echo $f
> sed -i~ -e "s/test_and_set_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_test_and_set_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/test_and_set_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_test_and_set_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/test_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/test_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/set_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_set_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/set_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_set_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/clear_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags)/dev_clear_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/clear_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags)/dev_clear_\\L\\1(\\2)/" "$f"
> sed -i~ -e "s/assign_bit(DEV_FLAG_\([^,]*\), \&\(.*\)->flags, \(.*\))/dev_assign_\\L\\1(\\2, \\3)/" "$f"
> sed -i~ -e "s/assign_bit(DEV_FLAG_\([^,]*\), \(.*\)\.flags, \(.*\))/dev_assign_\\L\\1(\\2, \\3)/" "$f"
> done
>
> ...and then did a few manual touchups for spacing.
>
> NOTE: one potentially "controversial" choice I made in some patches
> was to always reserve a flag ID even if a flag is only used under
> certain CONFIG_ settings. This is a change from how things were
> before. Keeping the numbering consistent and allowing easy
> compile-testing of both CONFIG settings seemed worth it, especially
> since it won't take up any extra space until we've added a lot more
> flags.
>
> I only marked the first patch as a "Fix" since it is the only one
> fixing observed problems. Other patches could be considered fixes too
> if folks want.
>
> I tested the first patch in the series backported to kernel 6.6 on the
> Pixel phone that was experiencing the race. I added extra printouts to
> make sure that the problem was hitting / addressed. The rest of the
> patches are tested with allmodconfig with arm32, arm64, ppc, and
> x86. I boot tested on an arm64 Chromebook running mainline.
>
> Changes in v4:
> - Use accessor functions for flags
>
> Changes in v3:
> - Use a new "flags" bitfield
> - Add missing \n in probe error message
>
> Changes in v2:
> - Instead of adjusting the ordering, use "ready_to_probe" flag
>
> Douglas Anderson (9):
> driver core: Don't let a device probe until it's ready
> driver core: Replace dev->can_match with dev_can_match()
> driver core: Replace dev->dma_iommu with dev_dma_iommu()
> driver core: Replace dev->dma_skip_sync with dev_dma_skip_sync()
> driver core: Replace dev->dma_ops_bypass with dev_dma_ops_bypass()
> driver core: Replace dev->state_synced with dev_state_synced()
> driver core: Replace dev->dma_coherent with dev_dma_coherent()
> driver core: Replace dev->of_node_reused with dev_of_node_reused()
> driver core: Replace dev->offline + ->offline_disabled with accessors
>
> arch/arc/mm/dma.c | 4 +-
> arch/arm/mach-highbank/highbank.c | 2 +-
> arch/arm/mach-mvebu/coherency.c | 2 +-
> arch/arm/mm/dma-mapping-nommu.c | 4 +-
> arch/arm/mm/dma-mapping.c | 28 ++--
> arch/arm64/kernel/cpufeature.c | 2 +-
> arch/arm64/mm/dma-mapping.c | 2 +-
> arch/mips/mm/dma-noncoherent.c | 2 +-
> arch/powerpc/kernel/dma-iommu.c | 8 +-
> .../platforms/pseries/hotplug-memory.c | 4 +-
> arch/riscv/mm/dma-noncoherent.c | 2 +-
> drivers/acpi/scan.c | 2 +-
> drivers/base/core.c | 53 +++++---
> drivers/base/cpu.c | 4 +-
> drivers/base/dd.c | 28 ++--
> drivers/base/memory.c | 2 +-
> drivers/base/pinctrl.c | 2 +-
> drivers/base/platform.c | 2 +-
> drivers/dma/ti/k3-udma-glue.c | 6 +-
> drivers/dma/ti/k3-udma.c | 6 +-
> drivers/iommu/dma-iommu.c | 9 +-
> drivers/iommu/iommu.c | 5 +-
> drivers/net/pcs/pcs-xpcs-plat.c | 2 +-
> drivers/of/device.c | 6 +-
> drivers/pci/of.c | 2 +-
> drivers/pci/pwrctrl/core.c | 2 +-
> drivers/regulator/bq257xx-regulator.c | 2 +-
> drivers/regulator/rk808-regulator.c | 2 +-
> drivers/tty/serial/serial_base_bus.c | 2 +-
> drivers/usb/gadget/udc/aspeed-vhub/dev.c | 2 +-
> include/linux/device.h | 120 ++++++++++++------
> include/linux/dma-map-ops.h | 6 +-
> include/linux/dma-mapping.h | 2 +-
> include/linux/iommu-dma.h | 3 +-
> kernel/cpu.c | 4 +-
> kernel/dma/mapping.c | 12 +-
> mm/hmm.c | 2 +-
> 37 files changed, 206 insertions(+), 142 deletions(-)
>
> --

For the whole set

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>