On Sat, Feb 08, 2025 at 09:37:56AM +0100, Louis Chauvet wrote:
> On 08/02/25 - 08:12, Greg Kroah-Hartman wrote:
> > On Fri, Feb 07, 2025 at 05:59:04PM +0100, Louis Chauvet wrote:
> > > On 06/02/25 - 18:38, Greg Kroah-Hartman wrote:
> > > > The vkms driver does not need to create a platform device, as there is
> > > > no real platform resources associated it, it only did so because it was
> > > > simple to do that in order to get a device to use for resource
> > > > management of drm resources. Change the driver to use the faux device
> > > > instead as this is NOT a real platform device.
> > > >
> > > > Cc: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> > > > Cc: Haneen Mohammed <hamohammed.sa@xxxxxxxxx>
> > > > Cc: Simona Vetter <simona@xxxxxxxx>
> > > > Cc: Melissa Wen <melissa.srw@xxxxxxxxx>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > > > Cc: Maxime Ripard <mripard@xxxxxxxxxx>
> > > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > > Cc: David Airlie <airlied@xxxxxxxxx>
> > > > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > ---
> > > > v3: new patch in the series. For an example of the api working, does
> > > > not have to be merged at this time, but I can take it if the
> > > > maintainers give an ack.
> > >
> > > Hi,
> > >
> > > This patch cannot be merged into drm-misc-next because we modified the
> > > vkms_device structure in commit 49a167c393b0 ("drm/vkms: Switch to dynamic
> > > allocation for CRTC"), which is present in linux-next.
> > >
> > > Once this conflict is resolved, I agree with changing from platform_device
> > > to faux_device.
> > >
> > > Apart from this minor conflict, I believe your patch has revealed an issue
> > > in VKMS:
> > >
> > > Without your patch:
> > >
> > > bash-5.2# modprobe vkms
> > > [drm] Initialized vkms 1.0.0 for vkms on minor 0
> > > bash-5.2#
> > >
> > > With your patch:
> > >
> > > bash-5.2# modprobe vkms
> > > faux vkms: Resources present before probing
> > > [drm] Initialized vkms 1.0.0 for vkms on minor 0
> > > bash-5.2#
> > >
> > > After some investigation, I found that the issue is not caused by your
> > > patch but by VKMS itself:
> > >
> > > During faux_device_create, the device core postpones the device probe to
> > > run it later [0]. This probe checks if the devres list is empty [1] and
> > > fails if it is not.
> > >
> > > [0]:https://elixir.bootlin.com/linux/v6.13.1/source/drivers/base/bus.c#L534
> > > [1]:https://elixir.bootlin.com/linux/v6.13.1/source/drivers/base/dd.c#L626
> > >
> > > With a platform driver, the order of execution was:
> > >
> > > platform_device_register_simple();
> > > device_add();
> > > *async* device_probe(); /* no issue, the devres is untouched */
> > > devres_open_group();
> > >
> > > But with faux-device, the order is:
> > >
> > > faux_device_create();
> > > device_add();
> > > devres_open_group();
> > > *async* device_probe(); /* issue here, because of the previous
> > > devres_open_group */
> >
> > Wait, what? It shouuld be the exact same codepath, as faux_device() is
> > not doing anything different from platform here. You might just be
> > hitting a race condition as the async probing is the same here.
>
> Yes, this is the same codepath, and this is a race condition. VKMS was
> just lucky it never happend before.
>
> > > How do you think this should be solved? I would like to keep a simple
> > > solution, given that:
> > > - we want to have multiple vkms devices (configfs [2])
> > > - we need to ensure that device_probe is called before devres_open_group
> > > and devm_drm_dev_alloc to avoid this error
> >
> > How about we take out the async probe? You are getting lucky that it's
> > not hit on the platform device code today. Faux really doesn't need
> > async, I was just trying to make the system work the same way that
> > platform devices did.
>
> I think this should be sufficient, and allows for a very simple interface:
> once faux_device_create returns, you can use the device "as-is", no
> need to wait for the probe.
>
> What change can I do to disable async probe and test?
Try this patch:
diff --git a/drivers/base/faux.c b/drivers/base/faux.c
index 27879ae78f53..0b9d17cd41f2 100644
--- a/drivers/base/faux.c
+++ b/drivers/base/faux.c
@@ -73,7 +73,7 @@ static const struct bus_type faux_bus_type = {
static struct device_driver faux_driver = {
.name = "faux_driver",
.bus = &faux_bus_type,
- .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .probe_type = PROBE_FORCE_SYNCHRONOUS,
};
static void faux_device_release(struct device *dev)
