Re: [PATCH 04/10] driver core: Don't return EPROBE_DEFER to userspace during sysfs bind
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-06-14 22:43:05
Also in:
dri-devel, intel-gfx, kvm, linux-s390
On Mon, Jun 14, 2021 at 05:08:40PM +0200, Christoph Hellwig wrote:
quoted hunk ↗ jump to hunk
@@ -679,8 +666,6 @@ static int really_probe(struct device *dev, struct device_driver *drv) dev->pm_domain->dismiss(dev); pm_runtime_reinit(dev); dev_pm_set_driver_flags(dev, 0); - if (probe_ret == -EPROBE_DEFER) - driver_deferred_probe_add_trigger(dev, local_trigger_count); done:
I like the new arrangement - however I'm looking at the ordering relative to this:
atomic_dec(&probe_count); wake_up_all(&probe_waitqueue);
And wondering if the idea is that driver_deferred_probe_add_trigger() is supposed to be enclosed by the atomic, so that the device_block_probing() / wait_for_device_probe() sequence is actually a fence against queuing new work? Which is suggesting that the other driver_deferred_probe_add_trigger() at the top of really_probe is already ordered wrong? Although, if that is the idea the wait_for_device_probe() doesn't look entirely sequenced right.. It looks easy enough to fix by moving the probe_count up:
+static int driver_probe_device(struct device_driver *drv, struct device *dev)
+{
+ int trigger_count = atomic_read(&deferred_trigger_count);
+ int ret;
+
+ ret = __driver_probe_device(drv, dev);
+ if (ret == -EPROBE_DEFER || ret == EPROBE_DEFER) {
+ driver_deferred_probe_add(dev);
+
+ /*
+ * Did a trigger occur while probing? Need to re-trigger if yes
+ */
+ if (trigger_count != atomic_read(&deferred_trigger_count) &&
+ !defer_all_probes)
+ driver_deferred_probe_trigger();
+ }into here? I didn't see a reason why it couldn't enclose the pm stuff too.. Jason