On Sat, Apr 4, 2026 at 2:07 AM Douglas Anderson <[email protected]> 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) <[email protected]>
