Re: [driver-core PATCH v5 5/9] driver core: Establish clear order of operations for deferred probe and remove

From: Bart Van Assche
Date: Tue Nov 06 2018 - 18:48:40 EST

Next message: Andrew Morton: "Re: [PATCH] fs/proc: introduce /proc/stat2 file"
Previous message: Nicolin Chen: "Re: [PATCH] iommu/dma: Zero pages manually in a length of scatterlist"
In reply to: Alexander Duyck: "Re: [driver-core PATCH v5 5/9] driver core: Establish clear order of operations for deferred probe and remove"
Next in thread: Joe Perches: "Re: [driver-core PATCH v5 5/9] driver core: Establish clear order of operations for deferred probe and remove"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, 2018-11-05 at 13:12 -0800, Alexander Duyck wrote:
+AD4 One change I made in addition is I replaced the use of +ACI-bool X:1+ACI to define
+AD4 the bitfield to a +ACI-u8 X:1+ACI setup in order to resolve some checkpatch
+AD4 warnings.

Please use +ACI-bool X:1+ACI instead of +ACI-u8 X:1+ACI. I think it was a bad idea to make
checkpatch complain about +ACI-bool X:1+ACI since +ACI-bool X:1+ACI should only be avoided
in structures for which alignment must be architecture-independent. For struct
device it is fine if member alignment differs per architecture. Additionally,
changing +ACI-bool X:1+ACI into +ACI-u8 X:1+ACI will reduce performance on architectures that
cannot do byte addressing.

+AD4 static void +AF8AXw-device+AF8-release+AF8-driver(struct device +ACo-dev, struct device +ACo-parent)
+AD4 +AHs
+AD4 - struct device+AF8-driver +ACo-drv+ADs
+AD4 +- struct device+AF8-driver +ACo-drv +AD0 dev-+AD4-driver+ADs
+AD4
+AD4 - drv +AD0 dev-+AD4-driver+ADs
+AD4 - if (drv) +AHs
+AD4 - while (device+AF8-links+AF8-busy(dev)) +AHs
+AD4 - +AF8AXw-device+AF8-driver+AF8-unlock(dev, parent)+ADs
+AD4 +- /+ACo
+AD4 +- +ACo In the event that we are asked to release the driver on an
+AD4 +- +ACo interface that is still waiting on a probe we can just terminate
+AD4 +- +ACo the probe by setting async+AF8-probe to false. When the async call
+AD4 +- +ACo is finally completed it will see this state and just exit.
+AD4 +- +ACo-/
+AD4 +- dev-+AD4-async+AF8-probe +AD0 false+ADs
+AD4 +- if (+ACE-drv)
+AD4 +- return+ADs
+AD4
+AD4 - device+AF8-links+AF8-unbind+AF8-consumers(dev)+ADs
+AD4 +- while (device+AF8-links+AF8-busy(dev)) +AHs
+AD4 +- +AF8AXw-device+AF8-driver+AF8-unlock(dev, parent)+ADs
+AD4
+AD4 - +AF8AXw-device+AF8-driver+AF8-lock(dev, parent)+ADs
+AD4 - /+ACo
+AD4 - +ACo A concurrent invocation of the same function might
+AD4 - +ACo have released the driver successfully while this one
+AD4 - +ACo was waiting, so check for that.
+AD4 - +ACo-/
+AD4 - if (dev-+AD4-driver +ACEAPQ drv)
+AD4 - return+ADs
+AD4 - +AH0
+AD4 +- device+AF8-links+AF8-unbind+AF8-consumers(dev)+ADs
+AD4
+AD4 - pm+AF8-runtime+AF8-get+AF8-sync(dev)+ADs
+AD4 - pm+AF8-runtime+AF8-clean+AF8-up+AF8-links(dev)+ADs
+AD4 +- +AF8AXw-device+AF8-driver+AF8-lock(dev, parent)+ADs
+AD4 +- /+ACo
+AD4 +- +ACo A concurrent invocation of the same function might
+AD4 +- +ACo have released the driver successfully while this one
+AD4 +- +ACo was waiting, so check for that.
+AD4 +- +ACo-/
+AD4 +- if (dev-+AD4-driver +ACEAPQ drv)
+AD4 +- return+ADs
+AD4 +- +AH0
+AD4
+AD4 - driver+AF8-sysfs+AF8-remove(dev)+ADs
+AD4 +- pm+AF8-runtime+AF8-get+AF8-sync(dev)+ADs
+AD4 +- pm+AF8-runtime+AF8-clean+AF8-up+AF8-links(dev)+ADs
+AD4
+AD4 - if (dev-+AD4-bus)
+AD4 - blocking+AF8-notifier+AF8-call+AF8-chain(+ACY-dev-+AD4-bus-+AD4-p-+AD4-bus+AF8-notifier,
+AD4 - BUS+AF8-NOTIFY+AF8-UNBIND+AF8-DRIVER,
+AD4 - dev)+ADs
+AD4 +- driver+AF8-sysfs+AF8-remove(dev)+ADs
+AD4
+AD4 - pm+AF8-runtime+AF8-put+AF8-sync(dev)+ADs
+AD4 +- if (dev-+AD4-bus)
+AD4 +- blocking+AF8-notifier+AF8-call+AF8-chain(+ACY-dev-+AD4-bus-+AD4-p-+AD4-bus+AF8-notifier,
+AD4 +- BUS+AF8-NOTIFY+AF8-UNBIND+AF8-DRIVER,
+AD4 +- dev)+ADs
+AD4
+AD4 - if (dev-+AD4-bus +ACYAJg dev-+AD4-bus-+AD4-remove)
+AD4 - dev-+AD4-bus-+AD4-remove(dev)+ADs
+AD4 - else if (drv-+AD4-remove)
+AD4 - drv-+AD4-remove(dev)+ADs
+AD4 +- pm+AF8-runtime+AF8-put+AF8-sync(dev)+ADs
+AD4
+AD4 - device+AF8-links+AF8-driver+AF8-cleanup(dev)+ADs
+AD4 - arch+AF8-teardown+AF8-dma+AF8-ops(dev)+ADs
+AD4 +- if (dev-+AD4-bus +ACYAJg dev-+AD4-bus-+AD4-remove)
+AD4 +- dev-+AD4-bus-+AD4-remove(dev)+ADs
+AD4 +- else if (drv-+AD4-remove)
+AD4 +- drv-+AD4-remove(dev)+ADs
+AD4
+AD4 - devres+AF8-release+AF8-all(dev)+ADs
+AD4 - dev-+AD4-driver +AD0 NULL+ADs
+AD4 - dev+AF8-set+AF8-drvdata(dev, NULL)+ADs
+AD4 - if (dev-+AD4-pm+AF8-domain +ACYAJg dev-+AD4-pm+AF8-domain-+AD4-dismiss)
+AD4 - dev-+AD4-pm+AF8-domain-+AD4-dismiss(dev)+ADs
+AD4 - pm+AF8-runtime+AF8-reinit(dev)+ADs
+AD4 - dev+AF8-pm+AF8-set+AF8-driver+AF8-flags(dev, 0)+ADs
+AD4 +- device+AF8-links+AF8-driver+AF8-cleanup(dev)+ADs
+AD4 +- arch+AF8-teardown+AF8-dma+AF8-ops(dev)+ADs
+AD4 +-
+AD4 +- devres+AF8-release+AF8-all(dev)+ADs
+AD4 +- dev-+AD4-driver +AD0 NULL+ADs
+AD4 +- dev+AF8-set+AF8-drvdata(dev, NULL)+ADs
+AD4 +- if (dev-+AD4-pm+AF8-domain +ACYAJg dev-+AD4-pm+AF8-domain-+AD4-dismiss)
+AD4 +- dev-+AD4-pm+AF8-domain-+AD4-dismiss(dev)+ADs
+AD4 +- pm+AF8-runtime+AF8-reinit(dev)+ADs
+AD4 +- dev+AF8-pm+AF8-set+AF8-driver+AF8-flags(dev, 0)+ADs
+AD4
+AD4 - klist+AF8-remove(+ACY-dev-+AD4-p-+AD4-knode+AF8-driver)+ADs
+AD4 - device+AF8-pm+AF8-check+AF8-callbacks(dev)+ADs
+AD4 - if (dev-+AD4-bus)
+AD4 - blocking+AF8-notifier+AF8-call+AF8-chain(+ACY-dev-+AD4-bus-+AD4-p-+AD4-bus+AF8-notifier,
+AD4 - BUS+AF8-NOTIFY+AF8-UNBOUND+AF8-DRIVER,
+AD4 - dev)+ADs
+AD4 +- klist+AF8-remove(+ACY-dev-+AD4-p-+AD4-knode+AF8-driver)+ADs
+AD4 +- device+AF8-pm+AF8-check+AF8-callbacks(dev)+ADs
+AD4 +- if (dev-+AD4-bus)
+AD4 +- blocking+AF8-notifier+AF8-call+AF8-chain(+ACY-dev-+AD4-bus-+AD4-p-+AD4-bus+AF8-notifier,
+AD4 +- BUS+AF8-NOTIFY+AF8-UNBOUND+AF8-DRIVER,
+AD4 +- dev)+ADs
+AD4
+AD4 - kobject+AF8-uevent(+ACY-dev-+AD4-kobj, KOBJ+AF8-UNBIND)+ADs
+AD4 - +AH0
+AD4 +- kobject+AF8-uevent(+ACY-dev-+AD4-kobj, KOBJ+AF8-UNBIND)+ADs
+AD4 +AH0

This patch mixes functional changes with whitespace changes. Please move the
whitespace changes into a separate patch such that this patch becomes easier
to read.

+AD4 void device+AF8-release+AF8-driver+AF8-internal(struct device +ACo-dev,
+AD4 diff --git a/include/linux/device.h b/include/linux/device.h
+AD4 index 1b25c7a43f4c..fc7091d436c2 100644
+AD4 --- a/include/linux/device.h
+AD4 +-+-+- b/include/linux/device.h
+AD4 +AEAAQA -1043,14 +-1043,15 +AEAAQA struct device +AHs
+AD4 struct iommu+AF8-group +ACo-iommu+AF8-group+ADs
+AD4 struct iommu+AF8-fwspec +ACo-iommu+AF8-fwspec+ADs
+AD4
+AD4 - bool offline+AF8-disabled:1+ADs
+AD4 - bool offline:1+ADs
+AD4 - bool of+AF8-node+AF8-reused:1+ADs
+AD4 +- u8 offline+AF8-disabled:1+ADs
+AD4 +- u8 offline:1+ADs
+AD4 +- u8 of+AF8-node+AF8-reused:1+ADs
+AD4 +ACM-if defined(CONFIG+AF8-ARCH+AF8-HAS+AF8-SYNC+AF8-DMA+AF8-FOR+AF8-DEVICE) +AHwAfA +AFw
+AD4 defined(CONFIG+AF8-ARCH+AF8-HAS+AF8-SYNC+AF8-DMA+AF8-FOR+AF8-CPU) +AHwAfA +AFw
+AD4 defined(CONFIG+AF8-ARCH+AF8-HAS+AF8-SYNC+AF8-DMA+AF8-FOR+AF8-CPU+AF8-ALL)
+AD4 - bool dma+AF8-coherent:1+ADs
+AD4 +- u8 dma+AF8-coherent:1+ADs
+AD4 +ACM-endif
+AD4 +- u8 async+AF8-probe:1+ADs

The new async+AF8-probe field can be changed from multiple threads. Concurrent
changes of a bitfield are only safe if these are serialized in some way.
Please document the locking requirements for the async+AF8-probe bitfield in
device.h.

Thanks,

Bart.

Next message: Andrew Morton: "Re: [PATCH] fs/proc: introduce /proc/stat2 file"
Previous message: Nicolin Chen: "Re: [PATCH] iommu/dma: Zero pages manually in a length of scatterlist"
In reply to: Alexander Duyck: "Re: [driver-core PATCH v5 5/9] driver core: Establish clear order of operations for deferred probe and remove"
Next in thread: Joe Perches: "Re: [driver-core PATCH v5 5/9] driver core: Establish clear order of operations for deferred probe and remove"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]