[PATCH 0/3] Fix issues for device probe

[PATCH 0/3] Fix issues for device probe

[Wang Wensheng]

The sbsa-gwdt device sometimes doesn't probe on my board after the
kernel updated from 5.10 to 6.6. And even when the watchdog probe
eventually, it a bit later than that in 5.10 kernel, approximately
10 seconds after sbsa_gwdt.ko loaded.

The first issue is fixed in the first patch.
The second issue is fixed in the following two patches, but I'm not
sure whether this is properly.

Wang Wensheng (3):
  driver core: Fix concurrent problem of deferred_probe_extend_timeout()
  driver core: Introduce fw_devlink_relax_consumers helper
  irqchip/mbigen: Use fw_devlink_relax_consumers() helper

 drivers/base/core.c          | 22 ++++++++++++++++++++++
 drivers/base/dd.c            |  2 ++
 drivers/irqchip/irq-mbigen.c |  2 ++
 include/linux/device.h       |  1 +
 4 files changed, 27 insertions(+)

-- 
2.22.0

[PATCH 1/3] driver core: Fix concurrent problem of deferred_probe_extend_timeout()

[Wang Wensheng]

The deferred_probe_timeout_work may be canceled forever unexpected when
deferred_probe_extend_timeout() executes concurrently. Start with
deferred_probe_timeout_work pending, and the problem would
occur after the following sequence.

         CPU0                                 CPU1
deferred_probe_extend_timeout
  -> cancel_delayed_work => true
                                     deferred_probe_extend_timeout
                                       -> cancel_delayed_wrok
                                         -> __cancel_work
                                           -> try_grab_pending
  -> schedule_delayed_work
   -> queue_delayed_work_on
since pending bit is grabbed,
just return without doing anything
                                        -> set_work_pool_and_clear_pending
                                     this __cancel_work return false and
                                     the work would never be queued again

The root cause is that the PENDING_BIT of the work_struct would be set
temporaily in __cancel_work and this bit could prevent the work_struct
to be queued in another CPU.

Use deferred_probe_mutex to protect the cancel and queue operations for
the deferred_probe_timeout_work to fix this problem.

Fixes: 2b28a1a84a0e ("driver core: Extend deferred probe timeout on driver registration")
Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
---
 drivers/base/dd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 13ab98e033ea..1983919917e0 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -323,6 +323,7 @@ static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_
 
 void deferred_probe_extend_timeout(void)
 {
+	mutex_lock(&deferred_probe_mutex);
 	/*
 	 * If the work hasn't been queued yet or if the work expired, don't
 	 * start a new one.
@@ -333,6 +334,7 @@ void deferred_probe_extend_timeout(void)
 		pr_debug("Extended deferred probe timeout by %d secs\n",
 					driver_deferred_probe_timeout);
 	}
+	mutex_unlock(&deferred_probe_mutex);
 }
 
 /**
-- 
2.22.0

Re: [PATCH 1/3] driver core: Fix concurrent problem of deferred_probe_extend_timeout()

[Greg KH]

On Thu, Aug 14, 2025 at 07:10:21PM +0800, Wang Wensheng wrote:
> The deferred_probe_timeout_work may be canceled forever unexpected when
> deferred_probe_extend_timeout() executes concurrently. Start with
> deferred_probe_timeout_work pending, and the problem would
> occur after the following sequence.
> 
>          CPU0                                 CPU1
> deferred_probe_extend_timeout
>   -> cancel_delayed_work => true
>                                      deferred_probe_extend_timeout
>                                        -> cancel_delayed_wrok
>                                          -> __cancel_work
>                                            -> try_grab_pending
>   -> schedule_delayed_work
>    -> queue_delayed_work_on
> since pending bit is grabbed,
> just return without doing anything
>                                         -> set_work_pool_and_clear_pending
>                                      this __cancel_work return false and
>                                      the work would never be queued again
> 
> The root cause is that the PENDING_BIT of the work_struct would be set
> temporaily in __cancel_work and this bit could prevent the work_struct
> to be queued in another CPU.
> 
> Use deferred_probe_mutex to protect the cancel and queue operations for
> the deferred_probe_timeout_work to fix this problem.
> 
> Fixes: 2b28a1a84a0e ("driver core: Extend deferred probe timeout on driver registration")
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> ---
>  drivers/base/dd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 13ab98e033ea..1983919917e0 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -323,6 +323,7 @@ static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_
>  
>  void deferred_probe_extend_timeout(void)
>  {
> +	mutex_lock(&deferred_probe_mutex);

Perhaps use a guard() instead?

thanks,

greg k-h

Re: [PATCH 1/3] driver core: Fix concurrent problem of deferred_probe_extend_timeout()

[wangwensheng (C)]

在 2025/8/14 19:37, Greg KH 写道:
> On Thu, Aug 14, 2025 at 07:10:21PM +0800, Wang Wensheng wrote:
>> The deferred_probe_timeout_work may be canceled forever unexpected when
>> deferred_probe_extend_timeout() executes concurrently. Start with
>> deferred_probe_timeout_work pending, and the problem would
>> occur after the following sequence.
>>
>>           CPU0                                 CPU1
>> deferred_probe_extend_timeout
>>    -> cancel_delayed_work => true
>>                                       deferred_probe_extend_timeout
>>                                         -> cancel_delayed_wrok
>>                                           -> __cancel_work
>>                                             -> try_grab_pending
>>    -> schedule_delayed_work
>>     -> queue_delayed_work_on
>> since pending bit is grabbed,
>> just return without doing anything
>>                                          -> set_work_pool_and_clear_pending
>>                                       this __cancel_work return false and
>>                                       the work would never be queued again
>>
>> The root cause is that the PENDING_BIT of the work_struct would be set
>> temporaily in __cancel_work and this bit could prevent the work_struct
>> to be queued in another CPU.
>>
>> Use deferred_probe_mutex to protect the cancel and queue operations for
>> the deferred_probe_timeout_work to fix this problem.
>>
>> Fixes: 2b28a1a84a0e ("driver core: Extend deferred probe timeout on driver registration")
>> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
>> ---
>>   drivers/base/dd.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index 13ab98e033ea..1983919917e0 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -323,6 +323,7 @@ static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_
>>   
>>   void deferred_probe_extend_timeout(void)
>>   {
>> +	mutex_lock(&deferred_probe_mutex);
> 
> Perhaps use a guard() instead?
> 
> thanks,
> 
> greg k-h
> 

Thanks for your suggestion. I have sent a v2 for this signle patch, 
because the other issue is not strongly related to this and need
more discussion.

Re: [PATCH 1/3] driver core: Fix concurrent problem of deferred_probe_extend_timeout()

[Greg KH]

On Thu, Aug 14, 2025 at 07:10:21PM +0800, Wang Wensheng wrote:
> The deferred_probe_timeout_work may be canceled forever unexpected when
> deferred_probe_extend_timeout() executes concurrently. Start with
> deferred_probe_timeout_work pending, and the problem would
> occur after the following sequence.
> 
>          CPU0                                 CPU1
> deferred_probe_extend_timeout
>   -> cancel_delayed_work => true
>                                      deferred_probe_extend_timeout
>                                        -> cancel_delayed_wrok
>                                          -> __cancel_work
>                                            -> try_grab_pending
>   -> schedule_delayed_work
>    -> queue_delayed_work_on
> since pending bit is grabbed,
> just return without doing anything
>                                         -> set_work_pool_and_clear_pending
>                                      this __cancel_work return false and
>                                      the work would never be queued again
> 
> The root cause is that the PENDING_BIT of the work_struct would be set
> temporaily in __cancel_work and this bit could prevent the work_struct
> to be queued in another CPU.
> 
> Use deferred_probe_mutex to protect the cancel and queue operations for
> the deferred_probe_timeout_work to fix this problem.
> 
> Fixes: 2b28a1a84a0e ("driver core: Extend deferred probe timeout on driver registration")
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> ---
>  drivers/base/dd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 13ab98e033ea..1983919917e0 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -323,6 +323,7 @@ static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_
>  
>  void deferred_probe_extend_timeout(void)
>  {
> +	mutex_lock(&deferred_probe_mutex);
>  	/*
>  	 * If the work hasn't been queued yet or if the work expired, don't
>  	 * start a new one.
> @@ -333,6 +334,7 @@ void deferred_probe_extend_timeout(void)
>  		pr_debug("Extended deferred probe timeout by %d secs\n",
>  					driver_deferred_probe_timeout);
>  	}
> +	mutex_unlock(&deferred_probe_mutex);
>  }
>  
>  /**
> -- 
> 2.22.0
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH 2/3] driver core: Introduce fw_devlink_relax_consumers helper

[Wang Wensheng]

Some devices are added during its parent's probe and will never get
bound to a driver. In this case, with fw_devlink set to "rpm",
which is the default value, its consumers will be deferred probe
until deferred_probe_timeout when fw_devlink_drivers_done() would
relax the devlinks to the suplier.

Use this function to relax the consumer devlinks, just like what we
do for the unmatched devices in fw_devlink_drivers_done(), so that
the consumer devices would be probed not that later.

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
---
 drivers/base/core.c    | 22 ++++++++++++++++++++++
 include/linux/device.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d22d6b23e758..2f7101ad9d11 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1754,6 +1754,28 @@ static void fw_devlink_relax_link(struct device_link *link)
 		dev_name(link->supplier));
 }
 
+/**
+ * fw_devlink_relax_consumers - Relax the devlinks with all its consumers
+ * @dev: Device whose consumer devlinks will be relaxed
+ *
+ * Some devices are added during its parent's probe and will never get bound
+ * to a driver. In this case its consumers will be deferred probe until
+ * deferred_probe_timeout.
+ *
+ * Use this function to relax the consumer devlinks so that the consumers
+ * device would be probed not that later.
+ */
+void fw_devlink_relax_consumers(struct device *dev)
+{
+	struct device_link *link;
+
+	device_links_write_lock();
+	list_for_each_entry(link, &dev->links.consumers, s_node)
+		fw_devlink_relax_link(link);
+	device_links_write_unlock();
+}
+EXPORT_SYMBOL_GPL(fw_devlink_relax_consumers);
+
 static int fw_devlink_no_driver(struct device *dev, void *data)
 {
 	struct device_link *link = to_devlink(dev);
diff --git a/include/linux/device.h b/include/linux/device.h
index 0470d19da7f2..a451c0eb2ffa 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1201,6 +1201,7 @@ void device_link_remove(void *consumer, struct device *supplier);
 void device_links_supplier_sync_state_pause(void);
 void device_links_supplier_sync_state_resume(void);
 void device_link_wait_removal(void);
+void fw_devlink_relax_consumers(struct device *dev);
 
 static inline bool device_link_test(const struct device_link *link, u32 flags)
 {
-- 
2.22.0

Re: [PATCH 2/3] driver core: Introduce fw_devlink_relax_consumers helper

[Greg KH]

On Thu, Aug 14, 2025 at 07:10:22PM +0800, Wang Wensheng wrote:
> Some devices are added during its parent's probe and will never get
> bound to a driver. In this case, with fw_devlink set to "rpm",
> which is the default value, its consumers will be deferred probe
> until deferred_probe_timeout when fw_devlink_drivers_done() would
> relax the devlinks to the suplier.
> 
> Use this function to relax the consumer devlinks, just like what we
> do for the unmatched devices in fw_devlink_drivers_done(), so that
> the consumer devices would be probed not that later.
> 
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> ---
>  drivers/base/core.c    | 22 ++++++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d22d6b23e758..2f7101ad9d11 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1754,6 +1754,28 @@ static void fw_devlink_relax_link(struct device_link *link)
>  		dev_name(link->supplier));
>  }
>  
> +/**
> + * fw_devlink_relax_consumers - Relax the devlinks with all its consumers
> + * @dev: Device whose consumer devlinks will be relaxed
> + *
> + * Some devices are added during its parent's probe and will never get bound
> + * to a driver. In this case its consumers will be deferred probe until
> + * deferred_probe_timeout.
> + *
> + * Use this function to relax the consumer devlinks so that the consumers
> + * device would be probed not that later.
> + */
> +void fw_devlink_relax_consumers(struct device *dev)
> +{
> +	struct device_link *link;
> +
> +	device_links_write_lock();
> +	list_for_each_entry(link, &dev->links.consumers, s_node)
> +		fw_devlink_relax_link(link);
> +	device_links_write_unlock();
> +}
> +EXPORT_SYMBOL_GPL(fw_devlink_relax_consumers);

We currently do not export any "fw_" functions from the driver core, why
do that now?  This feels wrong as this should all be "internal" to the
driver core, no driver should be calling this.

thanks,

greg k-h

Re: [PATCH 2/3] driver core: Introduce fw_devlink_relax_consumers helper

[Saravana Kannan]

On Thu, Aug 14, 2025 at 4:39 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 14, 2025 at 07:10:22PM +0800, Wang Wensheng wrote:
> > Some devices are added during its parent's probe and will never get
> > bound to a driver.

Hi Wang,

I'm guessing you are adding these "will never probe" devices to a bus?
Why are you adding devices to a bus if you'll never probe them? You
should use a "class" if these are devices of a type and don't need to
be probed.

> > In this case, with fw_devlink set to "rpm",
> > which is the default value, its consumers will be deferred probe
> > until deferred_probe_timeout when fw_devlink_drivers_done() would
> > relax the devlinks to the suplier.
> >
> > Use this function to relax the consumer devlinks, just like what we
> > do for the unmatched devices in fw_devlink_drivers_done(), so that
> > the consumer devices would be probed not that later.

This function is not meant for use outside fw_devlink core code.

Write a stub driver or move those devices to a class.

> >
> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> > ---
> >  drivers/base/core.c    | 22 ++++++++++++++++++++++
> >  include/linux/device.h |  1 +
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index d22d6b23e758..2f7101ad9d11 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -1754,6 +1754,28 @@ static void fw_devlink_relax_link(struct device_link *link)
> >               dev_name(link->supplier));
> >  }
> >
> > +/**
> > + * fw_devlink_relax_consumers - Relax the devlinks with all its consumers
> > + * @dev: Device whose consumer devlinks will be relaxed
> > + *
> > + * Some devices are added during its parent's probe and will never get bound
> > + * to a driver. In this case its consumers will be deferred probe until
> > + * deferred_probe_timeout.
> > + *
> > + * Use this function to relax the consumer devlinks so that the consumers
> > + * device would be probed not that later.
> > + */
> > +void fw_devlink_relax_consumers(struct device *dev)
> > +{
> > +     struct device_link *link;
> > +
> > +     device_links_write_lock();
> > +     list_for_each_entry(link, &dev->links.consumers, s_node)
> > +             fw_devlink_relax_link(link);
> > +     device_links_write_unlock();
> > +}
> > +EXPORT_SYMBOL_GPL(fw_devlink_relax_consumers);
>
> We currently do not export any "fw_" functions from the driver core, why
> do that now?  This feels wrong as this should all be "internal" to the
> driver core, no driver should be calling this.

Yup, you are right. Definite NACK.

-Saravana

[PATCH 3/3] irqchip/mbigen: Use fw_devlink_relax_consumers() helper

[Wang Wensheng]

Use this to prevernt the consumer devices of mbigen to be probed too
later.

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
---
 drivers/irqchip/irq-mbigen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index 6f69f4e5dbac..4e96eb9b6a6a 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -252,6 +252,8 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
 
 		if (!mbigen_create_device_domain(&child->dev, num_pins, mgn_chip))
 			return -ENOMEM;
+
+		fw_devlink_relax_consumers(&child->dev);
 	}
 
 	return 0;
-- 
2.22.0

Re: [PATCH 3/3] irqchip/mbigen: Use fw_devlink_relax_consumers() helper

[Greg KH]

On Thu, Aug 14, 2025 at 07:10:23PM +0800, Wang Wensheng wrote:
> Use this to prevernt the consumer devices of mbigen to be probed too
> later.
> 
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> ---
>  drivers/irqchip/irq-mbigen.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> index 6f69f4e5dbac..4e96eb9b6a6a 100644
> --- a/drivers/irqchip/irq-mbigen.c
> +++ b/drivers/irqchip/irq-mbigen.c
> @@ -252,6 +252,8 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
>  
>  		if (!mbigen_create_device_domain(&child->dev, num_pins, mgn_chip))
>  			return -ENOMEM;
> +
> +		fw_devlink_relax_consumers(&child->dev);

Ick, no, individual drivers should not be doing this.  Saravana, any
ideas?

greg k-h

Re: [PATCH 3/3] irqchip/mbigen: Use fw_devlink_relax_consumers() helper

[Saravana Kannan]

On Thu, Aug 14, 2025 at 4:39 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 14, 2025 at 07:10:23PM +0800, Wang Wensheng wrote:
> > Use this to prevernt the consumer devices of mbigen to be probed too
> > later.
> >
> > Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> > ---
> >  drivers/irqchip/irq-mbigen.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
> > index 6f69f4e5dbac..4e96eb9b6a6a 100644
> > --- a/drivers/irqchip/irq-mbigen.c
> > +++ b/drivers/irqchip/irq-mbigen.c
> > @@ -252,6 +252,8 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
> >
> >               if (!mbigen_create_device_domain(&child->dev, num_pins, mgn_chip))
> >                       return -ENOMEM;
> > +
> > +             fw_devlink_relax_consumers(&child->dev);
>
> Ick, no, individual drivers should not be doing this.  Saravana, any
> ideas?

Yeah, I responded to patch 2/3.

To give a more specific answer, this driver is just adding platform
devices to the platform bus that it never probes. They should create a
class for these and add these devices to their own class. fw_devlink
is smart enough about not waiting on class devices to probe. If for
whatever reason, switching it to a class is impossible, then they
should write a stub driver to probe these devices.

-Saravana

Re: [PATCH 3/3] irqchip/mbigen: Use fw_devlink_relax_consumers() helper

[wangwensheng (C)]

在 2025/8/15 2:05, Saravana Kannan 写道:
> On Thu, Aug 14, 2025 at 4:39 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Thu, Aug 14, 2025 at 07:10:23PM +0800, Wang Wensheng wrote:
>>> Use this to prevernt the consumer devices of mbigen to be probed too
>>> later.
>>>
>>> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
>>> ---
>>>   drivers/irqchip/irq-mbigen.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>>> index 6f69f4e5dbac..4e96eb9b6a6a 100644
>>> --- a/drivers/irqchip/irq-mbigen.c
>>> +++ b/drivers/irqchip/irq-mbigen.c
>>> @@ -252,6 +252,8 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
>>>
>>>                if (!mbigen_create_device_domain(&child->dev, num_pins, mgn_chip))
>>>                        return -ENOMEM;
>>> +
>>> +             fw_devlink_relax_consumers(&child->dev);
>>
>> Ick, no, individual drivers should not be doing this.  Saravana, any
>> ideas?
> 
> Yeah, I responded to patch 2/3.
> 
> To give a more specific answer, this driver is just adding platform
> devices to the platform bus that it never probes. They should create a
> class for these and add these devices to their own class. fw_devlink
> is smart enough about not waiting on class devices to probe. If for
> whatever reason, switching it to a class is impossible, then they
> should write a stub driver to probe these devices.
> 
> -Saravana

Thanks for your suggestion. I will try this way later.