Re: [PATCH] md: fix soft lockup during resync when sync is repeatedly skipped

Hi,

在 2026/7/20 14:20, Yunye Zhao 写道:
> Hi Kuai,
>
> On 2026/7/17 14:27, Yu Kuai wrote:
>>> md_do_sync()'s main loop advances io_sectors only when I/O is actually
>>> issued (skipped == 0).  When sync_request() keeps returning skipped == 1,
>>> io_sectors never increases, [...]
>> That's not expected, io_sectors should always increase in the skip case.
> Sorry, my description was not accurate. The problem is not that io_sectors
> stays 0. md_do_sync()'s loop exit condition is j == max_sectors, and in
> raid10_sync_request()'s recovery path sectors is always 128, so for a very
> large max_sectors the loop iterates a huge number of times.
>
> raid10_sync_request(), recovery branch:
>
>      max_sync = RESYNC_PAGES << (PAGE_SHIFT-9);      /* 128 */
>      must_sync = md_bitmap_start_sync(mddev, sect, &sync_blocks, true);
>      if (sync_blocks < max_sync)   /* sync_blocks huge, never true */
>          max_sync = sync_blocks;   /* so max_sync stays 128 */
>      ...
>      if (biolist == NULL) {
>          *skipped = 1;
>          return max_sync;          /* 128; the large span is discarded */
>      }
>
> Back in md_do_sync()'s main loop, j then crawls forward 128 sectors per
> call until it reaches max_sectors:
>
>      while (j < max_sectors) {
>          sectors = mddev->pers->sync_request(mddev, j, max_sectors, &skipped);
>          if (!skipped)             /* skipped == 1 -> io_sectors stays 0 */
>              io_sectors += sectors;
>          j += sectors;             /* j += 128 only */
>          if (last_check + window > io_sectors || j == max_sectors)
>              continue;
>      }
>
>  From the vmcore:
>
>      sync_blocks returned = 0x5ED03680 (~1.59e8 sectors), clamped to 128
>      recovery max_sectors = dev_sectors = 2^40
>      iterations = 2^40 / 128 = 2^33 (~8.6e9)

Please update and fix the commit message "io_sectors never increases", this
really is misleading.

>
>> What kernel version you're testing?
> 6.6.102. I also tested mainline and hit the same problem.
>
>> If this is latest kernel, bitmap_start_sync() need to be fixed. It can't
>> return skip while setting skipping sectors to 0.
> bitmap_start_sync() is actually fine -- it returns a large clean span
> (0x5ED03680 above). The recovery path just discards it: max_sync is only
> ever clamped down, so it returns 128 regardless.
>
>> And since this is dead loop, a cond_resched() will not fix anything.
> Agreed -- cond_resched() only stops the watchdog; the thread still spins
> ~8.6e9 no-op iterations and pins a CPU for ~313s.
>
> For v2 I'd fix this at the source: make the recovery path honour the clean
> span reported by the bitmap instead of capping the skip at 128. Does that
> direction look right to you?

This patch is still valuable, even a 5s lockup is not acceptable.

And I think this is not a problem for llbitmap, it skips one bit at a time and
bits are limited to at most 4k*128. Perhaps bitmap can do this as well, it can
definably return much bigger skipping sectors from bitmap_start_sync().

>
> Thanks,
> Yunye

-- 
Thanks,
Kuai