From: Yu Kuai <yukuai3@huawei.com>

daemon_work() will be called by daemon thread, on the one hand, daemon
thread doesn't have strict wake-up time; on the other hand, too much
work are put to daemon thread, like handle sync IO, handle failed
or specail normal IO, handle recovery, and so on. Hence daemon thread
may be too busy to clear dirty bits in time.

Make bitmap_ops->daemon_work() optional and following patches will use
separate async work to clear dirty bits for the new bitmap.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/md.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 737fdb6474bd..c7f7914b7452 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9853,7 +9853,7 @@ static void unregister_sync_thread(struct mddev *mddev)
  */
 void md_check_recovery(struct mddev *mddev)
 {
-	if (md_bitmap_enabled(mddev))
+	if (md_bitmap_enabled(mddev) && mddev->bitmap_ops->daemon_work)
 		mddev->bitmap_ops->daemon_work(mddev);
 
 	if (signal_pending(current)) {
-- 
2.39.2

On 5/24/25 08:13, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> daemon_work() will be called by daemon thread, on the one hand, daemon
> thread doesn't have strict wake-up time; on the other hand, too much
> work are put to daemon thread, like handle sync IO, handle failed
> or specail normal IO, handle recovery, and so on. Hence daemon thread
> may be too busy to clear dirty bits in time.
> 
> Make bitmap_ops->daemon_work() optional and following patches will use
> separate async work to clear dirty bits for the new bitmap.
> 
Why not move it to a workqueue in general?
The above argument is valid even for the current implementation, no?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Hi,

在 2025/05/27 14:19, Hannes Reinecke 写道:
> On 5/24/25 08:13, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> daemon_work() will be called by daemon thread, on the one hand, daemon
>> thread doesn't have strict wake-up time; on the other hand, too much
>> work are put to daemon thread, like handle sync IO, handle failed
>> or specail normal IO, handle recovery, and so on. Hence daemon thread
>> may be too busy to clear dirty bits in time.
>>
>> Make bitmap_ops->daemon_work() optional and following patches will use
>> separate async work to clear dirty bits for the new bitmap.
>>
> Why not move it to a workqueue in general?
> The above argument is valid even for the current implementation, no?

Yes, and however, I'll prefer not to touch current implementaion :(
This is trivial comparing to other flaws like global spinlock.

Thanks,
Kuai

> 
> Cheers,
> 
> Hannes

On 5/27/25 10:03, Yu Kuai wrote:
> Hi,
> 
> 在 2025/05/27 14:19, Hannes Reinecke 写道:
>> On 5/24/25 08:13, Yu Kuai wrote:
>>> From: Yu Kuai <yukuai3@huawei.com>
>>>
>>> daemon_work() will be called by daemon thread, on the one hand, daemon
>>> thread doesn't have strict wake-up time; on the other hand, too much
>>> work are put to daemon thread, like handle sync IO, handle failed
>>> or specail normal IO, handle recovery, and so on. Hence daemon thread
>>> may be too busy to clear dirty bits in time.
>>>
>>> Make bitmap_ops->daemon_work() optional and following patches will use
>>> separate async work to clear dirty bits for the new bitmap.
>>>
>> Why not move it to a workqueue in general?
>> The above argument is valid even for the current implementation, no?
> 
> Yes, and however, I'll prefer not to touch current implementaion :(
> This is trivial comparing to other flaws like global spinlock.
> 
Fair enough.

You can add:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>